tenox7 / ttyplot

a realtime plotting utility for terminal/console with data input from stdin
Apache License 2.0
957 stars 42 forks source link

Fix over-eager buffer truncation (regression from #98) #129

Closed hartwork closed 7 months ago

hartwork commented 7 months ago

Regression from #98, may need coordination with #115.

Previously, with high input pressure where full sizeof(input_buf) - 1 bytes could be read, all content would be dropped right after reading, and it could happen with every single read.

Example:

# ./ttyplot < /dev/random
edgar-bonet commented 7 months ago

Since this section is completely rewritten in #115. Fixing this now may not be very useful. And I guess it could just add merge conflicts.

hartwork commented 7 months ago

Since this section is completely rewritten in #115. Fixing this now may not be very useful. And I guess it could just add merge conflicts.

@edgar-bonet it would give me a chance to clean-up the mess I made earlier regarding truncation, it would separate bugfixes from refactoring and get the fix into development sooner. Toyota would like it :smiley:

I was hoping that we could merge this (#129) and rebase #115 on top of it after. Conflict resolution is cheap here, it could even be automated, because/assuming that #115 does not have the same problem. Let me demo that with our very case on the shell:

# Start after the first big refactoring commit (that would show conflict)
git checkout -b demo-free-conflict-resolution edgar-bonet-readonly/refactor-input~1

# Merge but keep our side because the refactored version does not have the same issue
EDITOR=true git merge --strategy=ours hartwork-readwrite/fix-buffer-truncation 

# Extract a rebased patch
git diff github-readwrite/fix-buffer-truncation HEAD > rebased.patch

# Pretend to start all over
git reset --hard edgar-bonet-readonly/refactor-input~2

# Apply the rebased version, three commits:
git cherry-pick hartwork-readwrite/fix-buffer-truncation  # 1
patch < rebased.patch
git add ttyplot.c
git commit -C edgar-bonet-readonly/refactor-input~1  # 2
git cherry-pick edgar-bonet-readonly/refactor-input  # 3

# Done

EDIT: There's actually simpler versions to achieve the same. This just was the first version that came to mind that would work.

What do you think?

hartwork commented 7 months ago

@edgar-bonet PS: For a cleaner version using git commit-tree

# First commit (i.e. start right at <fix-buffer-truncation>)
git checkout -b demo-free-conflict-resolution-2 hartwork-readwrite/fix-buffer-truncation

# Second commit, fake a merge with startegy "theirs" plus remaking original commit metadata
git merge --ff "$(git commit-tree edgar-bonet-readonly/refactor-input~1^{tree} -p HEAD -m replaced-later | tee /dev/stderr)"
EDITOR=true git commit --amend -C edgar-bonet-readonly/refactor-input~1

# Third commit, plain cherry-pick
git cherry-pick edgar-bonet-readonly/refactor-input
hartwork commented 7 months ago

@edgar-bonet thank you, very kind! :pray: