Skip to content
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 EOF behavior in interactive mode with history #6

Merged
merged 1 commit into from Mar 16, 2019

Conversation

michaelforney
Copy link
Contributor

Previously, EOF would re-process the previous line, read one more
line, then exit. This is because bc_history_line left the buffer
untouched in case of EOF, and the bc_vm_stdin loop reads the next
line before checking the done condition of the previous iteration.

This fix also prevents reading past the end of the buffer if the
first user input is EOF, since in that case vec->len is still 0,
so (size_t)-1 is passed to bc_read_binary.

Previously, EOF would re-process the previous line, read one more
line, then exit. This is because bc_history_line left the buffer
untouched in case of EOF, and the bc_vm_stdin loop reads the next
line before checking the `done` condition of the previous iteration.

This fix also prevents reading past the end of the buffer if the
first user input is EOF, since in that case vec->len is still 0,
so (size_t)-1 is passed to bc_read_binary.
@codecov
Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #6 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #6      +/-   ##
=========================================
+ Coverage    98.7%   98.7%   +<.01%     
=========================================
  Files          16      16              
  Lines        3478    3479       +1     
=========================================
+ Hits         3433    3434       +1     
  Misses         45      45
Impacted Files Coverage Δ
src/vm.c 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63031c6...f2bb38c. Read the comment docs.

@gavinhoward gavinhoward merged commit 19f9d07 into gavinhoward:master Mar 16, 2019
@gavinhoward
Copy link
Owner

I am back from my vacation, so it's review time.

Let's see:

  • Style? Style is correct.
  • Is there a problem? Yes, there was, and it was one I didn't even know about.
  • Does it fix the problem? Yes, it does.
  • Does it pass the test suite? Yes, it does. (Actually, it doesn't pass when running the tests in parallel, but that is a bug in the test harnesses, not your changes.)

Result: Merged.

Thank you so much for the fix! I will add your name to the list of contributors.

@michaelforney
Copy link
Contributor Author

Thanks!

gavinhoward pushed a commit that referenced this pull request Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants