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.

Sign up for GitHub

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

Jump to bottom

Fix bugs in BytesMut::reserve_inner #544

Merged
merged 3 commits into from Apr 28, 2022
Merged

Fix bugs in BytesMut::reserve_inner #544

merged 3 commits into from Apr 28, 2022

Conversation

NobodyXu
Copy link
Contributor

@NobodyXu NobodyXu commented Apr 26, 2022 •
edited

Fixed bugs introduced in #529 , BytesMut::reserve_inner:

  • Fix calculation of offset

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

@NobodyXu
Fix calculation of offset in BytesMut::reserve_inner
4d65b11 
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@Darksonn
Copy link
Contributor

Would you be able to add a test case for this?

@NobodyXu
Copy link
Contributor Author

NobodyXu commented Apr 28, 2022 •
edited

Would you be able to add a test case for this?

Sure.

I should be able to using split_off

@NobodyXu NobodyXu changed the title Fix calculation of offset in BytesMut::reserve_inner Fix bugs in BytesMut::reserve_inner Apr 28, 2022
@NobodyXu
Add reserve_shared_reuse tests in tests/test_bytes.rs
a80c53d 
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu
Copy link
Contributor Author

The miri failed with leaking in the newly added test, however I cannot locate the problem.

@Darksonn
Copy link
Contributor

You're using mem::forget. It's not surprising that this would result in a leak. Perhaps you meant to use drop?

@NobodyXu
Copy link
Contributor Author

You're using mem::forget. It's not surprising that this would result in a leak. Perhaps you meant to use drop?

Oh right, thank you for reminding me that!

@NobodyXu
Fix reserve_shared_reuse
6e215ff 
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@Darksonn
Copy link
Contributor

I have verified that the test fails without your change.

Darksonn
Darksonn approved these changes Apr 28, 2022
View reviewed changes
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks!

I will merge this once CI passes.

@Darksonn Darksonn merged commit 0a2c43a into tokio-rs:master Apr 28, 2022
@NobodyXu NobodyXu deleted the fix/reserve_inner branch April 28, 2022 09:37
@Darksonn Darksonn mentioned this pull request Jul 19, 2022
Merged
lelongg pushed a commit to lelongg/bytes that referenced this pull request Jan 9, 2023
@NobodyXu @lelongg
Fix bugs in BytesMut::reserve_inner (tokio-rs#544)
7b39d05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Darksonn Darksonn Darksonn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@NobodyXu @Darksonn