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

Make freeze respect the start offset for BytesMuts in Vec mode #376

Merged
merged 1 commit into from Mar 24, 2020

Conversation

TimHambourger
Copy link
Contributor

Fixes #352. This one seemed pretty straightforward to me (advance the Bytes before returning). But I'm a Rust newbie, so let me know if I'm missing something.

Also, I guess this is technically a breaking change b/c consuming code might have been depending on the old behavior. E.g. something like

let mut b: BytesMut;
b.advance(2);
let b = b.freeze();
// Huh, weird, frozen version doesn't reflect my advance. Guess I'll advance again.
b.advance(2);

Y'all's call on how big of a concern that is. My thought is it seems small: The work around above seems less likely than the workaround of using split_to as mentioned in the original issue. And the old behavior definitely seemed inconsistent (behavior for Vec mode differed from the behavior for Arc mode, and behavior for freeze differed from the behavior for as_slice/as_slice_mut).

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

I get what you're saying about this being breaking, but that's the reality of any bug that people try to work around...

@seanmonstar seanmonstar merged commit 8bbe9dd into tokio-rs:master Mar 24, 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.

BytesMut::freeze ignores advance
2 participants