-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix over-eager buffer truncation (regression from #98) #129
Conversation
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
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 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? |
@edgar-bonet PS: For a cleaner version using # 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 |
@edgar-bonet thank you, very kind! 🙏 |
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