Clarify API edge cases

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);
     }
 }

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);
         }
 

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
     }
 }

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;
     }
 }

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..]
     }
 

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]