Fix Ref::new_slice_from_suffix()#506
Conversation
`new_slice_from_suffix()` has a bug and implemented exactly as `new_slice_from_prefix()`. Now part of buffer is chosen right. Tests couldn't have caught because buffer was just zeros. Now buffer is not just zeros so that tests can prove correctness of `new_slice_from_suffix()`.
dmitryash
force-pushed
the
fix_new_slice_from_suffix
branch
from
22f5e49 to
21f7974
Compare
joshlf
left a comment
joshlf
left a comment
There was a problem hiding this comment.
Awesome, thanks so much for catching this! One small change to request. I want to give some more thought to whether we should do more to beef up our tests in this PR to catch bugs like this in the future; I'll have time to do that and give more detailed feedback early this coming week.
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
|
Not sure what's up with those failing Miri test cases. Maybe try rebasing on |
In #506, a bug was discovered which `test_new_aligned_sized` did not previously catch. This commit improves that test to catch that bug.
|
Note that we plan to follow up on this consistent with #512. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
It appears that Miri is failing nondeterministically. I've reported the issue here: rust-lang/miri#3125. This appears to be an issue with Miri rather than with our code, so I'll try a few more times and maybe we'll get lucky with this making it through the merge queue. Obviously we'll need to fix this issue fairly soon, but there's no reason for resolving that issue to block this PR. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
@dmitryash Out of curiosity, how did you stumble upon this issue? We're always curious to hear how zerocopy is getting used in the wild! |
In #506, a bug was discovered which `test_new_aligned_sized` did not previously catch. This commit improves that test to catch that bug.
In #506, a bug was discovered which `test_new_aligned_sized` did not previously catch. This commit improves that test to catch that bug.
* Fix Ref::new_slice_from_suffix() `new_slice_from_suffix()` has a bug and implemented exactly as `new_slice_from_prefix()`. Now part of buffer is chosen right. Tests couldn't have caught because buffer was just zeros. Now buffer is not just zeros so that tests can prove correctness of `new_slice_from_suffix()`. * position -> split_at Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com> --------- Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
* Fix Ref::new_slice_from_suffix() `new_slice_from_suffix()` has a bug and implemented exactly as `new_slice_from_prefix()`. Now part of buffer is chosen right. Tests couldn't have caught because buffer was just zeros. Now buffer is not just zeros so that tests can prove correctness of `new_slice_from_suffix()`. * position -> split_at Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com> --------- Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
* Fix Ref::new_slice_from_suffix() `new_slice_from_suffix()` has a bug and implemented exactly as `new_slice_from_prefix()`. Now part of buffer is chosen right. Tests couldn't have caught because buffer was just zeros. Now buffer is not just zeros so that tests can prove correctness of `new_slice_from_suffix()`. * position -> split_at Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com> --------- Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
|
@joshlf I needed to decode some data that had "footer" and I tried from_suffix variant but found that it did not work correctly. |
Interesting, can you say more about why it didn't work? I'd like to be able to support your use case if possible. |
2 participants
new_slice_from_suffix()has a bug and implemented exactly asnew_slice_from_prefix(). Now part of buffer is chosen right.Tests couldn't have caught because buffer was just zeros. Now buffer is not just zeros so that tests can prove correctness of
new_slice_from_suffix().