Fix for Issue #222: handle EOF and offset correctly on read #224
Conversation
The EOF flag on native file streams is only set after reading *past* the end of the buffer, never on open or seek. I believe all the failing tests here are asserting behaviour that doesn't match the native file handler.
Testing with normal files show that fread() does not progress the pointer beyond the end of the file, and a read *beyond* the file size is required before feof() returns true.
|
Alternative implementation that applies on top of #221: refactor/rename_and_restructure...IMSoP:issue-222-rebase Oddly, a bunch of the tests that my change caused to fail don't seem to exist on that branch. |
|
Thanks! I think we should merge this after #221 is finished, as it reduces the points where seek functionality exists back to one. The other question for me is whether to backport it to the v1.x branch and do a bugfix release. |
That's correct, as some functionality was removed/moved somewhere else the according specific tests aren't needed any more and have been removed. This is especially true for the seek functionality in the |
|
Yeah, that makes sense. I think to backport to 1.x, we'd need to also backport #220 (otherwise the streams all have the same offset anyway), so it needs a call on the risk and effort of that. If you think it is, I can rebase this branch onto 1.x, and open a new one for the master fix once #221 lands. Regarding tests, it seems like too many of them were testing internal details that changed when refactoring, rather than the desired behaviour of the wrapper itself. If I get time, I'll raise a separate PR with some more test cases for calls to |
Having thought about it I believe it's not worth the effort, it's probably better to concentrate on 2.0.
That would be highly appreciated! |
The EOF flag on native file streams is only set after reading past the end of the stream, never on open or seek. Similarly,
freadwill never push the file position past the current end of the stream.I have changed various tests which I believe were asserting behaviour that doesn't match the native implementation.