Skip to content

Conversation

@lperlaki
Copy link

This would increase the MSRV to 1.51 since it uses const generics!

I am not sure on the MSRV policy of bytes, so if this is a problem we can just close this PR

Added Methods

  • Buf::try_get_array
    is a copy of the old implementation of buf_try_get_impl
  • Buf::get_array simply calls try_get_array and panic_advance on error

the second commit changes the implementation of the get_x methods to use try_get_array

this would also enable Buf implementors to optimise (like #622) the get_x methods by overriding try_get_array

@lperlaki
Copy link
Author

as related work there is also a std lib proposal to add a std::io::Read::read_array rust-lang/rust#148848

@Darksonn
Copy link
Contributor

I don't really get why we should add this. It seems pretty niche.

@lperlaki
Copy link
Author

I find the method useful for decoding data from a Buf

let mut key_buf = [0u8; 32];
buf.copy_to_slice(&mut key);
let key = key_buf;

compared to

let key: [u8; 32] = buf.get_array();
this can also be achieved with an extension trait
pub trait BufExtArray: Buf {
     fn get_array<const N: usize>(&mut self) -> [u8; N] {
        self.try_get_array()
            .unwrap_or_else(|error| panic_advance(&error))
    }

    fn try_get_array<const N: usize>(&mut self) -> Result<[u8; N], bytes::TryGetError> {
        if self.remaining() < N {
            return Err(bytes::TryGetError {
                requested: N,
                available: self.remaining(),
            });
        }

        let ret = self.chunk().first_chunk::<N>().copied();

        if let Some(ret) = ret {
            // if the direct get was possible, advance and return
            self.advance(N);
            Ok(ret)
        } else {
            // if not we copy the bytes in a temp buffer
            let mut buf = [0; N];
            self.copy_to_slice(&mut buf); // (do the advance)
            Ok(buf)
        }
    }
}

impl<B: Buf + ?Sized> BufExtArray for B {}

so if you find it to niche for including please close this PR.

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.

2 participants