reader: fix Skip(n) where n >= buffer size #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The (*Reader).Skip() code did not handle skip sizes above the buffer size correctly, because the inner reader loop only looped while 'r.buffered() < n', and that condition is obviously violated when the reader successfully buffers more bytes than are to be skipped. Refactor this method to be a little shorter and hopefully much clearer (and more correct) and add test coverage that exercises this code better.
klauspost
approved these changes
Nov 24, 2020
klauspost
added a commit
to klauspost/minio
that referenced
this pull request
Nov 24, 2020
The error in `github.com/philhofer/fwd` was quickly fixed through philhofer/fwd#22 - update the dependency and remove the workaround.
1 task
harshavardhana
pushed a commit
to minio/minio
that referenced
this pull request
Nov 24, 2020
The error in `github.com/philhofer/fwd` was quickly fixed through philhofer/fwd#22 - update the dependency and remove the workaround.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The (*Reader).Skip() code did not handle skip sizes
above the buffer size correctly, because the inner
reader loop only looped while 'r.buffered() < n', and
that condition is obviously violated when the reader
successfully buffers more bytes than are to be skipped.
Refactor this method to be a little shorter and hopefully
much clearer (and more correct) and add test coverage that
exercises this code better.
Fixes #20