diff --git a/src/buf/buf.rs b/src/buf/buf.rs index e85b3211e563f72afe9c7d9ae346930318d82c40..a8268f7049799992a8157eca653bfc709439e3c7 100644 --- a/src/buf/buf.rs +++ b/src/buf/buf.rs @@ -49,6 +49,12 @@ pub trait Buf { /// /// assert_eq!(buf.remaining(), 10); /// ``` + /// + /// # Implementer notes + /// + /// Implementations of `remaining` should ensure that the return value does + /// not change unless a call is made to `advance` or any other function that + /// is documented to change the `Buf`'s current position. fn remaining(&self) -> usize; /// Returns a slice starting at the current position and of length between 0 @@ -71,6 +77,12 @@ pub trait Buf { /// /// assert_eq!(buf.bytes(), b"world"); /// ``` + /// + /// # Implementer notes + /// + /// This function should never panic. Once the end of the buffer is reached, + /// i.e., `Buf::remaining` returns 0, calls to `bytes` should return an + /// empty slice. fn bytes(&self) -> &[u8]; /// Fills `dst` with potentially multiple slices starting at `self`'s @@ -90,15 +102,27 @@ pub trait Buf { /// This is a lower level function. Most operations are done with other /// functions. /// + /// # Implementer notes + /// + /// This function should never panic. Once the end of the buffer is reached, + /// i.e., `Buf::remaining` returns 0, calls to `bytes_vec` must return 0 + /// without mutating `dst`. + /// + /// Implementations should also take care to properly handle being called + /// with `dst` being a zero length slice. + /// /// [`writev`]: http://man7.org/linux/man-pages/man2/readv.2.html fn bytes_vec<'a>(&'a self, dst: &mut [&'a IoVec]) -> usize { if dst.is_empty() { return 0; } - dst[0] = self.bytes().into(); - - 1 + if self.has_remaining() { + dst[0] = self.bytes().into(); + 1 + } else { + 0 + } } /// Advance the internal cursor of the Buf @@ -123,7 +147,15 @@ pub trait Buf { /// /// # Panics /// - /// This function can panic if `cnt > self.remaining()`. + /// This function **may** panic if `cnt > self.remaining()`. + /// + /// # Implementer notes + /// + /// It is recommended for implementations of `advance` to panic if `cnt > + /// self.remaining()`. If the implementation does not panic, the call must + /// behave as if `cnt == self.remaining()`. + /// + /// A call with `cnt == 0` should never panic and be a no-op. fn advance(&mut self, cnt: usize); /// Returns true if there are any more bytes to consume @@ -669,13 +701,22 @@ impl<T: AsRef<[u8]>> Buf for io::Cursor<T> { } fn bytes(&self) -> &[u8] { + let len = self.get_ref().as_ref().len(); let pos = self.position() as usize; + + if pos >= len { + return Default::default(); + } + &(self.get_ref().as_ref())[pos..] } fn advance(&mut self, cnt: usize) { - let pos = self.position() as usize; - let pos = cmp::min(self.get_ref().as_ref().len(), pos + cnt); + let pos = (self.position() as usize) + .checked_add(cnt).expect("overflow"); + + assert!(pos <= self.get_ref().as_ref().len()); + self.set_position(pos as u64); } } diff --git a/src/buf/buf_mut.rs b/src/buf/buf_mut.rs index 7ecc8b2d4e92141948c450b5dd23fb28aabb9217..def898d5c5eca20185078c27c04d54c854dcb629 100644 --- a/src/buf/buf_mut.rs +++ b/src/buf/buf_mut.rs @@ -45,6 +45,12 @@ pub trait BufMut { /// /// assert_eq!(5, buf.remaining_mut()); /// ``` + /// + /// # Implementer notes + /// + /// Implementations of `remaining_mut` should ensure that the return value + /// does not change unless a call is made to `advance_mut` or any other + /// function that is documented to change the `BufMut`'s current position. fn remaining_mut(&self) -> usize; /// Advance the internal cursor of the BufMut @@ -80,7 +86,15 @@ pub trait BufMut { /// /// # Panics /// - /// This function can panic if `cnt > self.remaining_mut()`. + /// This function **may** panic if `cnt > self.remaining_mut()`. + /// + /// # Implementer notes + /// + /// It is recommended for implementations of `advance_mut` to panic if `cnt + /// > self.remaining_mut()`. If the implementation does not panic, the call + /// must behave as if `cnt == self.remaining_mut()`. + /// + /// A call with `cnt == 0` should never panic and be a no-op. unsafe fn advance_mut(&mut self, cnt: usize); /// Returns true if there is space in `self` for more bytes. @@ -136,6 +150,12 @@ pub trait BufMut { /// assert_eq!(5, buf.len()); /// assert_eq!(buf, b"hello"); /// ``` + /// + /// # Implementer notes + /// + /// This function should never panic. Once the end of the buffer is reached, + /// i.e., `BufMut::remaining_mut` returns 0, calls to `bytes_mut` should + /// return an empty slice. unsafe fn bytes_mut(&mut self) -> &mut [u8]; /// Fills `dst` with potentially multiple mutable slices starting at `self`'s @@ -156,15 +176,27 @@ pub trait BufMut { /// This is a lower level function. Most operations are done with other /// functions. /// + /// # Implementer notes + /// + /// This function should never panic. Once the end of the buffer is reached, + /// i.e., `BufMut::remaining_mut` returns 0, calls to `bytes_vec_mut` must + /// return 0 without mutating `dst`. + /// + /// Implementations should also take care to properly handle being called + /// with `dst` being a zero length slice. + /// /// [`readv`]: http://man7.org/linux/man-pages/man2/readv.2.html unsafe fn bytes_vec_mut<'a>(&'a mut self, dst: &mut [&'a mut IoVec]) -> usize { if dst.is_empty() { return 0; } - dst[0] = self.bytes_mut().into(); - - 1 + if self.has_remaining_mut() { + dst[0] = self.bytes_mut().into(); + 1 + } else { + 0 + } } /// Transfer bytes into `self` from `src` and advance the cursor by the @@ -578,9 +610,8 @@ impl<T: AsMut<[u8]> + AsRef<[u8]>> BufMut for io::Cursor<T> { /// Advance the internal cursor of the BufMut unsafe fn advance_mut(&mut self, cnt: usize) { - let pos = self.position() as usize; - let pos = cmp::min(self.get_mut().as_mut().len(), pos + cnt); - self.set_position(pos as u64); + use Buf; + self.advance(cnt); } /// Returns a mutable slice starting at the current BufMut position and of @@ -588,7 +619,13 @@ impl<T: AsMut<[u8]> + AsRef<[u8]>> BufMut for io::Cursor<T> { /// /// The returned byte slice may represent uninitialized memory. unsafe fn bytes_mut(&mut self) -> &mut [u8] { + let len = self.get_ref().as_ref().len(); let pos = self.position() as usize; + + if pos >= len { + return Default::default(); + } + &mut (self.get_mut().as_mut())[pos..] } } @@ -599,12 +636,12 @@ impl BufMut for Vec<u8> { } unsafe fn advance_mut(&mut self, cnt: usize) { - let len = self.len() + cnt; + let cap = self.capacity(); + let len = self.len().checked_add(cnt) + .expect("overflow"); - if len > self.capacity() { + if len > cap { // Reserve additional - // TODO: Should this case panic? - let cap = self.capacity(); self.reserve(cap - len); } diff --git a/src/buf/chain.rs b/src/buf/chain.rs index 6b7cd4b98581a27f89e1970c2cd4811c81f4945e..c026981ea2e4e11b9ca3328a3ffd6c326b5987a9 100644 --- a/src/buf/chain.rs +++ b/src/buf/chain.rs @@ -177,17 +177,9 @@ impl<T, U> Buf for Chain<T, U> } fn bytes_vec<'a>(&'a self, dst: &mut [&'a IoVec]) -> usize { - if self.a.has_remaining() { - let mut n = self.a.bytes_vec(dst); - - if n < dst.len() { - n += self.b.bytes_vec(&mut dst[n..]); - } - - n - } else { - self.b.bytes_vec(dst) - } + let mut n = self.a.bytes_vec(dst); + n += self.b.bytes_vec(&mut dst[n..]); + n } } @@ -226,16 +218,8 @@ impl<T, U> BufMut for Chain<T, U> } unsafe fn bytes_vec_mut<'a>(&'a mut self, dst: &mut [&'a mut IoVec]) -> usize { - if self.a.has_remaining_mut() { - let mut n = self.a.bytes_vec_mut(dst); - - if n < dst.len() { - n += self.b.bytes_vec_mut(&mut dst[n..]); - } - - n - } else { - self.b.bytes_vec_mut(dst) - } + let mut n = self.a.bytes_vec_mut(dst); + n += self.b.bytes_vec_mut(&mut dst[n..]); + n } } diff --git a/src/buf/take.rs b/src/buf/take.rs index f49383833190a6f01506102020ae1d653f53519e..7e409597bf5deba24e3a3e7a6f592834dda55d93 100644 --- a/src/buf/take.rs +++ b/src/buf/take.rs @@ -146,8 +146,8 @@ impl<T: Buf> Buf for Take<T> { } fn advance(&mut self, cnt: usize) { - let cnt = cmp::min(cnt, self.limit); - self.limit -= cnt; + assert!(cnt <= self.limit); self.inner.advance(cnt); + self.limit -= cnt; } } diff --git a/src/bytes.rs b/src/bytes.rs index fb70101b56cd54a3b47c9bd3ebdda00beff47a6f..fc29c624fed386560ef5e0e0505c36bcee40fde5 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -1123,12 +1123,16 @@ impl BufMut for BytesMut { #[inline] unsafe fn advance_mut(&mut self, cnt: usize) { let new_len = self.len() + cnt; + + // This call will panic if `cnt` is too big self.inner.set_len(new_len); } #[inline] unsafe fn bytes_mut(&mut self) -> &mut [u8] { let len = self.len(); + + // This will never panic as `len` can never become invalid &mut self.inner.as_raw()[len..] } diff --git a/tests/test_buf.rs b/tests/test_buf.rs index 5c989c34af096735834cf77d8f2c24bd02b8c834..5a1baedf5c3b217c390b6412ac89e9309b75d511 100644 --- a/tests/test_buf.rs +++ b/tests/test_buf.rs @@ -22,11 +22,6 @@ fn test_fresh_cursor_vec() { assert_eq!(buf.remaining(), 0); assert_eq!(buf.bytes(), b""); - - buf.advance(1); - - assert_eq!(buf.remaining(), 0); - assert_eq!(buf.bytes(), b""); } #[test]