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

Add method BoundedBuf::put_slice #198

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

mzabaluev
Copy link
Contributor

@mzabaluev mzabaluev commented Dec 4, 2022

Provide a generic, safe way to copy data into a buffer, possibly advancing the initialized length watermark.
This method is especially useful for FixedBuf, which lacks other population methods available for the foreign types.

This resolves a discussion started in #190 (comment)

A safe way to copy data into a buffer, possibly advancing the
initialized length watermark. This generic method is especially useful
for FixedBuf, which lacks other population methods available for
foreign types.
Copy link
Collaborator

@FrankReh FrankReh left a comment

Choose a reason for hiding this comment

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

Super.

Could you see the use cases for providing two other methods, one for zeroing to the init point and one for zeroing to the capacity and setting the init point to the capacity?

Seems code like

ary.iter_mut().for_each(|m| *m = 0)

is expected to be optimized to a memset call, per https://users.rust-lang.org/t/fastest-way-to-zero-an-array/39222
but that's from a few years ago.

One typo, and maybe wait for Noah's second commit today and this is good to go.

@@ -152,6 +153,29 @@ pub trait BoundedBufMut: BoundedBuf<Buf = Self::BufMut> {
/// The caller must ensure that all bytes starting at `stable_mut_ptr()` up
/// to `pos` are initialized and owned by the buffer.
unsafe fn set_init(&mut self, pos: usize);

/// Copies the given byte slice slice into the buffer, starting at
Copy link
Collaborator

Choose a reason for hiding this comment

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

'slice slice' -> 'slice'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FrankReh
Copy link
Collaborator

FrankReh commented Dec 5, 2022

Following the discussion further, I see there is a slice fill method since 1.50.
https://doc.rust-lang.org/std/primitive.slice.html#method.fill

And even if there weren't, no additional method is needed I just realized. The init slice can be zero filled by the app. There really is no need to fill to capacity.

@Noah-Kennedy
Copy link
Contributor

Noah-Kennedy commented Dec 5, 2022

Once the typo is fixed I'm in favor of merging unless @FrankReh has anything more to add.

Copy link
Collaborator

@FrankReh FrankReh left a comment

Choose a reason for hiding this comment

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

Thanks

@FrankReh FrankReh merged commit 4417d63 into tokio-rs:master Dec 5, 2022
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.

3 participants