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

write_to_prefix (and similar ones) take ownership of the buffer #195

Closed
SuperKenVery opened this issue Jul 9, 2023 · 2 comments
Closed
Labels
compatibility-breaking Changes that are (likely to be) breaking

Comments

@SuperKenVery
Copy link

Please correct me if I'm wrong, but by taking ownership of the buffer, it seems to release the buffer as soon as the function exits, and subsequent code cannot use the buffer.

The method is defined as follows:

    /// Writes a copy of `self` to the prefix of `bytes`.
    ///
    /// `write_to_prefix` writes `self` to the first `size_of_val(self)` bytes
    /// of `bytes`. If `bytes.len() < size_of_val(self)`, it returns `None`.
    fn write_to_prefix<B: ByteSliceMut>(&self, mut bytes: B) -> Option<()> {
        let size = mem::size_of_val(self);
        if bytes.len() < size {
            return None;
        }

        bytes[..size].copy_from_slice(self.as_bytes());
        Some(())
    }

I think the method should only borrow the buffer.

Again, correct me if I'm wrong please!

@joshlf
Copy link
Member

joshlf commented Jul 9, 2023

If your B type is &mut [u8], you can reborrow like this. That said, ByteSliceMut is also implemented for RefMut, so your suggestion is still a good one! I'll consider changing the signature of this and other methods/functions.

@joshlf
Copy link
Member

joshlf commented Aug 12, 2023

This was fixed in #204.

@joshlf joshlf closed this as completed Aug 12, 2023
@joshlf joshlf added the compatibility-breaking Changes that are (likely to be) breaking label Aug 12, 2023
@joshlf joshlf mentioned this issue Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Development

No branches or pull requests

2 participants