Bump version to v0.4.3

The shared debug_assert is to ensure that the internal Bytes
representation is such that offset views are supported. The only
representation that does not support offset views is vec.

Fixes #97

.travis.yml

@@ -19,7 +19,7 @@ matrix:
     #
     # This job will also build and deploy the docs to gh-pages.
     - env: TARGET=x86_64-unknown-linux-gnu
-      rust: 1.10.0
+      rust: 1.15.0
       after_success:
         - |
             pip install 'travis-cargo<0.2' --user &&
@@ -30,8 +30,8 @@ matrix:
     # Run tests on some extra platforms
     - env: TARGET=i686-unknown-linux-gnu
     - env: TARGET=armv7-unknown-linux-gnueabihf
-    - env: TARGET=powerpc-unknown-linux-gnu
-    - env: TARGET=powerpc64-unknown-linux-gnu
+    - env: RUST_TEST_THREADS=1 TARGET=powerpc-unknown-linux-gnu
+    - env: RUST_TEST_THREADS=1 TARGET=powerpc64-unknown-linux-gnu
 
 before_install: set -e
 

CHANGELOG.md

@@ -1,3 +1,14 @@
+# 0.4.3 (April, 30, 2017)
+
+* Fix Vec::advance_mut bug
+* Bump minimum Rust version to 1.15
+* Misc performance tweaks
+
+# 0.4.2 (April, 5, 2017)
+
+* Misc performance tweaks
+* Improved `Debug` implementation for `Bytes`
+* Avoid some incorrect assert panics
 
 # 0.4.1 (March 15, 2017)
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name          = "bytes"
-version       = "0.4.1"
+version       = "0.4.3"
 license       = "MIT"
 authors       = ["Carl Lerche <[email protected]>"]
 description   = "Types and traits for working with bytes"

src/buf/buf.rs

@@ -9,7 +9,8 @@ use std::{cmp, io, ptr};
 /// A buffer stores bytes in memory such that read operations are infallible.
 /// The underlying storage may or may not be in contiguous memory. A `Buf` value
 /// is a cursor into the buffer. Reading from `Buf` advances the cursor
-/// position.
+/// position. It can be thought of as an efficient `Iterator` for collections of
+/// bytes.
 ///
 /// The simplest `Buf` is a `Cursor` wrapping a `[u8]`.
 ///

src/buf/buf_mut.rs

@@ -59,7 +59,7 @@ pub trait BufMut {
     /// further into the underlying buffer.
     ///
     /// This function is unsafe because there is no guarantee that the bytes
-    /// being advanced to have been initialized.
+    /// being advanced past have been initialized.
     ///
     /// # Examples
     ///
@@ -90,9 +90,9 @@ pub trait BufMut {
     ///
     /// # 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()`.
+    /// 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);
@@ -153,8 +153,10 @@ pub trait BufMut {
     ///
     /// # 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
+    /// This function should never panic. `bytes_mut` should return an empty
+    /// slice **if and only if** `remaining_mut` returns 0. In other words,
+    /// `bytes_mut` returning an empty slice implies that `remaining_mut` will
+    /// return 0 and `remaining_mut` returning 0 implies that `bytes_mut` will
     /// return an empty slice.
     unsafe fn bytes_mut(&mut self) -> &mut [u8];
 
@@ -699,23 +701,25 @@ impl<T: AsMut<[u8]> + AsRef<[u8]>> BufMut for io::Cursor<T> {
 }
 
 impl BufMut for Vec<u8> {
+    #[inline]
     fn remaining_mut(&self) -> usize {
         usize::MAX - self.len()
     }
 
+    #[inline]
     unsafe fn advance_mut(&mut self, cnt: usize) {
-        let cap = self.capacity();
-        let len = self.len().checked_add(cnt)
-            .expect("overflow");
-
-        if len > cap {
-            // Reserve additional
-            self.reserve(cap - len);
+        let len = self.len();
+        let remaining = self.capacity() - len;
+        if cnt > remaining {
+            // Reserve additional capacity, and ensure that the total length
+            // will not overflow usize.
+            self.reserve(cnt);
         }
 
-        self.set_len(len);
+        self.set_len(len + cnt);
     }
 
+    #[inline]
     unsafe fn bytes_mut(&mut self) -> &mut [u8] {
         use std::slice;
 

src/bytes.rs

@@ -1,5 +1,6 @@
 use {IntoBuf, Buf, BufMut};
 use buf::Iter;
+use debug;
 
 use std::{cmp, fmt, mem, hash, ops, slice, ptr, usize};
 use std::borrow::Borrow;
@@ -86,13 +87,13 @@ use std::sync::atomic::Ordering::{Relaxed, Acquire, Release, AcqRel};
 /// underlying memory slice and may not be mutated, `BytesMut` handles are
 /// guaranteed to be the only handle able to view that slice of memory. As such,
 /// `BytesMut` handles are able to mutate the underlying memory. Note that
-/// holding a unique view to a region of memory does not mean that there are not
+/// holding a unique view to a region of memory does not mean that there are no
 /// other `Bytes` and `BytesMut` handles with disjoint views of the underlying
 /// memory.
 ///
 /// # Inline bytes.
 ///
-/// As an opitmization, when the slice referenced by a `Bytes` or `BytesMut`
+/// As an optimization, when the slice referenced by a `Bytes` or `BytesMut`
 /// handle is small enough [1], `Bytes` will avoid the allocation by inlining
 /// the slice directly in the handle. In this case, a clone is no longer
 /// "shallow" and the data will be copied.
@@ -107,10 +108,24 @@ pub struct Bytes {
 ///
 /// `BytesMut` represents a unique view into a potentially shared memory region.
 /// Given the uniqueness guarantee, owners of `BytesMut` handles are able to
-/// mutate the memory.
+/// mutate the memory. It is similar to a `Vec<u8>` but with less copies and
+/// allocations.
 ///
 /// For more detail, see [Bytes](struct.Bytes.html).
 ///
+/// # Growth
+///
+/// One key difference from `Vec<u8>` is that most operations **do not
+/// implicitly grow the buffer**. This means that calling `my_bytes.put("hello
+/// world");` could panic if `my_bytes` does not have enough capacity. Before
+/// writing to the buffer, ensure that there is enough remaining capacity by
+/// calling `my_bytes.remaining_mut()`. In general, avoiding calls to `reserve`
+/// is preferable.
+///
+/// The only exception is `extend` which implicitly reserves required capacity.
+///
+/// # Examples
+///
 /// ```
 /// use bytes::{BytesMut, BufMut};
 ///
@@ -534,6 +549,16 @@ impl Bytes {
     ///
     /// Panics if `at > len`
     pub fn split_off(&mut self, at: usize) -> Bytes {
+        assert!(at <= self.len());
+
+        if at == self.len() {
+            return Bytes::new();
+        }
+
+        if at == 0 {
+            return mem::replace(self, Bytes::new());
+        }
+
         Bytes {
             inner: Inner2 {
                 inner: self.inner.split_off(at),
@@ -565,6 +590,16 @@ impl Bytes {
     ///
     /// Panics if `at > len`
     pub fn split_to(&mut self, at: usize) -> Bytes {
+        assert!(at <= self.len());
+
+        if at == self.len() {
+            return mem::replace(self, Bytes::new());
+        }
+
+        if at == 0 {
+            return Bytes::new();
+        }
+
         Bytes {
             inner: Inner2 {
                 inner: self.inner.split_to(at),
@@ -650,6 +685,7 @@ impl AsRef<[u8]> for Bytes {
 impl ops::Deref for Bytes {
     type Target = [u8];
 
+    #[inline]
     fn deref(&self) -> &[u8] {
         self.inner.as_ref()
     }
@@ -708,7 +744,7 @@ impl Eq for Bytes {
 
 impl fmt::Debug for Bytes {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Debug::fmt(&self.inner.as_ref(), fmt)
+        fmt::Debug::fmt(&debug::BsDebug(&self.inner.as_ref()), fmt)
     }
 }
 
@@ -1075,7 +1111,7 @@ impl BytesMut {
     /// buf.put(&[0; 64][..]);
     ///
     /// let ptr = buf.as_ptr();
-    /// let other = buf.drain();
+    /// let other = buf.take();
     ///
     /// assert!(buf.is_empty());
     /// assert_eq!(buf.capacity(), 64);
@@ -1128,6 +1164,16 @@ impl BufMut for BytesMut {
             self.advance_mut(len);
         }
     }
+
+    #[inline]
+    fn put_u8(&mut self, n: u8) {
+        self.inner.put_u8(n);
+    }
+
+    #[inline]
+    fn put_i8(&mut self, n: i8) {
+        self.put_u8(n as u8);
+    }
 }
 
 impl IntoBuf for BytesMut {
@@ -1155,12 +1201,14 @@ impl AsRef<[u8]> for BytesMut {
 impl ops::Deref for BytesMut {
     type Target = [u8];
 
+    #[inline]
     fn deref(&self) -> &[u8] {
         self.as_ref()
     }
 }
 
 impl ops::DerefMut for BytesMut {
+    #[inline]
     fn deref_mut(&mut self) -> &mut [u8] {
         self.inner.as_mut()
     }
@@ -1246,7 +1294,7 @@ impl Eq for BytesMut {
 
 impl fmt::Debug for BytesMut {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Debug::fmt(self.inner.as_ref(), fmt)
+        fmt::Debug::fmt(&debug::BsDebug(&self.inner.as_ref()), fmt)
     }
 }
 
@@ -1425,6 +1473,25 @@ impl Inner {
         }
     }
 
+    /// Insert a byte into the next slot and advance the len by 1.
+    #[inline]
+    fn put_u8(&mut self, n: u8) {
+        if self.is_inline() {
+            let len = self.inline_len();
+            assert!(len < INLINE_CAP);
+            unsafe {
+                *self.inline_ptr().offset(len as isize) = n;
+            }
+            self.set_inline_len(len + 1);
+        } else {
+            assert!(self.len < self.cap);
+            unsafe {
+                *self.ptr.offset(self.len as isize) = n;
+            }
+            self.len += 1;
+        }
+    }
+
     #[inline]
     fn len(&self) -> usize {
         if self.is_inline() {
@@ -1852,8 +1919,8 @@ impl Inner {
     #[inline]
     fn is_shared(&mut self) -> bool {
         match self.kind() {
-            KIND_INLINE | KIND_ARC => true,
-            _ => false,
+            KIND_VEC => false,
+            _ => true,
         }
     }
 
@@ -1984,12 +2051,14 @@ unsafe impl Sync for Inner {}
 impl ops::Deref for Inner2 {
     type Target = Inner;
 
+    #[inline]
     fn deref(&self) -> &Inner {
         &self.inner
     }
 }
 
 impl ops::DerefMut for Inner2 {
+    #[inline]
     fn deref_mut(&mut self) -> &mut Inner {
         &mut self.inner
     }

src/debug.rs

@@ -0,0 +1,40 @@
+use std::fmt;
+
+/// Alternative implementation of `fmt::Debug` for byte slice.
+///
+/// Standard `Debug` implementation for `[u8]` is comma separated
+/// list of numbers. Since large amount of byte strings are in fact
+/// ASCII strings or contain a lot of ASCII strings (e. g. HTTP),
+/// it is convenient to print strings as ASCII when possible.
+///
+/// This struct wraps `&[u8]` just to override `fmt::Debug`.
+///
+/// `BsDebug` is not a part of public API of bytes crate.
+pub struct BsDebug<'a>(pub &'a [u8]);
+
+impl<'a> fmt::Debug for BsDebug<'a> {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        try!(write!(fmt, "b\""));
+        for &c in self.0 {
+            // https://doc.rust-lang.org/reference.html#byte-escapes
+            if c == b'\n' {
+                try!(write!(fmt, "\\n"));
+            } else if c == b'\r' {
+                try!(write!(fmt, "\\r"));
+            } else if c == b'\t' {
+                try!(write!(fmt, "\\t"));
+            } else if c == b'\\' || c == b'"' {
+                try!(write!(fmt, "\\{}", c as char));
+            } else if c == b'\0' {
+                try!(write!(fmt, "\\0"));
+            // ASCII printable except space
+            } else if c > 0x20 && c < 0x7f {
+                try!(write!(fmt, "{}", c as char));
+            } else {
+                try!(write!(fmt, "\\x{:02x}", c));
+            }
+        }
+        try!(write!(fmt, "\""));
+        Ok(())
+    }
+}

src/lib.rs

@@ -29,12 +29,12 @@
 //! buf.put(&b"hello world"[..]);
 //! buf.put_u16::<BigEndian>(1234);
 //!
-//! let a = buf.drain();
+//! let a = buf.take();
 //! assert_eq!(a, b"hello world\x04\xD2"[..]);
 //!
 //! buf.put(&b"goodbye world"[..]);
 //!
-//! let b = buf.drain();
+//! let b = buf.take();
 //! assert_eq!(b, b"goodbye world"[..]);
 //!
 //! assert_eq!(buf.capacity(), 998);
@@ -89,6 +89,7 @@ pub use buf::{
 };
 
 mod bytes;
+mod debug;
 pub use bytes::{Bytes, BytesMut};
 
 pub use byteorder::{ByteOrder, BigEndian, LittleEndian};

tests/test_buf_mut.rs

@@ -49,6 +49,17 @@ fn test_put_u16() {
     assert_eq!(b"\x54\x21", &buf[..]);
 }
 
+#[test]
+fn test_vec_advance_mut() {
+    // Regression test for carllerche/bytes#108.
+    let mut buf = Vec::with_capacity(8);
+    unsafe {
+        buf.advance_mut(12);
+        assert_eq!(buf.len(), 12);
+        assert!(buf.capacity() >= 12, "capacity: {}", buf.capacity());
+    }
+}
+
 #[test]
 fn test_clone() {
     let mut buf = BytesMut::with_capacity(100);

tests/test_bytes.rs

@@ -43,7 +43,7 @@ fn from_slice() {
 #[test]
 fn fmt() {
     let a = format!("{:?}", Bytes::from(&b"abcdefg"[..]));
-    let b = format!("{:?}", b"abcdefg");
+    let b = "b\"abcdefg\"";
 
     assert_eq!(a, b);
 
@@ -134,6 +134,50 @@ fn split_off_uninitialized() {
     assert_eq!(other.capacity(), 896);
 }
 
+#[test]
+fn split_off_to_loop() {
+    let s = b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+
+    for i in 0..(s.len() + 1) {
+        {
+            let mut bytes = Bytes::from(&s[..]);
+            let off = bytes.split_off(i);
+            assert_eq!(i, bytes.len());
+            let mut sum = Vec::new();
+            sum.extend(&bytes);
+            sum.extend(&off);
+            assert_eq!(&s[..], &sum[..]);
+        }
+        {
+            let mut bytes = BytesMut::from(&s[..]);
+            let off = bytes.split_off(i);
+            assert_eq!(i, bytes.len());
+            let mut sum = Vec::new();
+            sum.extend(&bytes);
+            sum.extend(&off);
+            assert_eq!(&s[..], &sum[..]);
+        }
+        {
+            let mut bytes = Bytes::from(&s[..]);
+            let off = bytes.split_to(i);
+            assert_eq!(i, off.len());
+            let mut sum = Vec::new();
+            sum.extend(&off);
+            sum.extend(&bytes);
+            assert_eq!(&s[..], &sum[..]);
+        }
+        {
+            let mut bytes = BytesMut::from(&s[..]);
+            let off = bytes.split_to(i);
+            assert_eq!(i, off.len());
+            let mut sum = Vec::new();
+            sum.extend(&off);
+            sum.extend(&bytes);
+            assert_eq!(&s[..], &sum[..]);
+        }
+    }
+}
+
 #[test]
 fn split_to_1() {
     // Inline
@@ -194,6 +238,28 @@ fn split_to_uninitialized() {
     assert_eq!(other.capacity(), 128);
 }
 
+#[test]
+fn split_off_to_at_gt_len() {
+    fn make_bytes() -> Bytes {
+        let mut bytes = BytesMut::with_capacity(100);
+        bytes.put_slice(&[10, 20, 30, 40]);
+        bytes.freeze()
+    }
+
+    use std::panic;
+
+    make_bytes().split_to(4);
+    make_bytes().split_off(4);
+
+    assert!(panic::catch_unwind(move || {
+        make_bytes().split_to(5);
+    }).is_err());
+
+    assert!(panic::catch_unwind(move || {
+        make_bytes().split_off(5);
+    }).is_err());
+}
+
 #[test]
 fn fns_defined_for_bytes_mut() {
     let mut bytes = BytesMut::from(&b"hello world"[..]);
@@ -298,6 +364,18 @@ fn extend() {
 }
 
 #[test]
+fn from_static() {
+    let mut a = Bytes::from_static(b"ab");
+    let b = a.split_off(1);
+
+    assert_eq!(a, b"a"[..]);
+    assert_eq!(b, b"b"[..]);
+}
+
+#[test]
+// Only run these tests on little endian systems. CI uses qemu for testing
+// little endian... and qemu doesn't really support threading all that well.
+#[cfg(target_endian = "little")]
 fn stress() {
     // Tests promoting a buffer from a vec -> shared in a concurrent situation
     use std::sync::{Arc, Barrier};
@@ -308,7 +386,7 @@ fn stress() {
 
     for i in 0..ITERS {
         let data = [i as u8; 256];
-        let buf = Arc::new(BytesMut::from(&data[..]));
+        let buf = Arc::new(Bytes::from(&data[..]));
 
         let barrier = Arc::new(Barrier::new(THREADS));
         let mut joins = Vec::with_capacity(THREADS);
@@ -319,7 +397,8 @@ fn stress() {
 
             joins.push(thread::spawn(move || {
                 c.wait();
-                let _buf = buf.clone();
+                let buf: Bytes = (*buf).clone();
+                drop(buf);
             }));
         }
 

tests/test_chain.rs

@@ -53,41 +53,50 @@ fn vectored_read() {
     let b = Cursor::new(Bytes::from(&b"world"[..]));
 
     let mut buf = a.chain(b);
-    let mut iovecs: [&IoVec; 4] = Default::default();
 
-    assert_eq!(2, buf.bytes_vec(&mut iovecs));
-    assert_eq!(iovecs[0][..], b"hello"[..]);
-    assert_eq!(iovecs[1][..], b"world"[..]);
-    assert!(iovecs[2].is_empty());
-    assert!(iovecs[3].is_empty());
+    {
+        let mut iovecs: [&IoVec; 4] = Default::default();
+
+        assert_eq!(2, buf.bytes_vec(&mut iovecs));
+        assert_eq!(iovecs[0][..], b"hello"[..]);
+        assert_eq!(iovecs[1][..], b"world"[..]);
+        assert!(iovecs[2].is_empty());
+        assert!(iovecs[3].is_empty());
+    }
 
     buf.advance(2);
 
-    iovecs = Default::default();
+    {
+        let mut iovecs: [&IoVec; 4] = Default::default();
 
-    assert_eq!(2, buf.bytes_vec(&mut iovecs));
-    assert_eq!(iovecs[0][..], b"llo"[..]);
-    assert_eq!(iovecs[1][..], b"world"[..]);
-    assert!(iovecs[2].is_empty());
-    assert!(iovecs[3].is_empty());
+        assert_eq!(2, buf.bytes_vec(&mut iovecs));
+        assert_eq!(iovecs[0][..], b"llo"[..]);
+        assert_eq!(iovecs[1][..], b"world"[..]);
+        assert!(iovecs[2].is_empty());
+        assert!(iovecs[3].is_empty());
+    }
 
     buf.advance(3);
 
-    iovecs = Default::default();
+    {
+        let mut iovecs: [&IoVec; 4] = Default::default();
 
-    assert_eq!(1, buf.bytes_vec(&mut iovecs));
-    assert_eq!(iovecs[0][..], b"world"[..]);
-    assert!(iovecs[1].is_empty());
-    assert!(iovecs[2].is_empty());
-    assert!(iovecs[3].is_empty());
+        assert_eq!(1, buf.bytes_vec(&mut iovecs));
+        assert_eq!(iovecs[0][..], b"world"[..]);
+        assert!(iovecs[1].is_empty());
+        assert!(iovecs[2].is_empty());
+        assert!(iovecs[3].is_empty());
+    }
 
     buf.advance(3);
 
-    iovecs = Default::default();
+    {
+        let mut iovecs: [&IoVec; 4] = Default::default();
 
-    assert_eq!(1, buf.bytes_vec(&mut iovecs));
-    assert_eq!(iovecs[0][..], b"ld"[..]);
-    assert!(iovecs[1].is_empty());
-    assert!(iovecs[2].is_empty());
-    assert!(iovecs[3].is_empty());
+        assert_eq!(1, buf.bytes_vec(&mut iovecs));
+        assert_eq!(iovecs[0][..], b"ld"[..]);
+        assert!(iovecs[1].is_empty());
+        assert!(iovecs[2].is_empty());
+        assert!(iovecs[3].is_empty());
+    }
 }

tests/test_debug.rs

@@ -0,0 +1,35 @@
+extern crate bytes;
+
+use bytes::Bytes;
+
+#[test]
+fn fmt() {
+    let vec: Vec<_> = (0..0x100).map(|b| b as u8).collect();
+
+    let expected = "b\"\
+        \\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07\
+        \\x08\\t\\n\\x0b\\x0c\\r\\x0e\\x0f\
+        \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17\
+        \\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f\
+        \\x20!\\\"#$%&'()*+,-./0123456789:;<=>?\
+        @ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_\
+        `abcdefghijklmnopqrstuvwxyz{|}~\\x7f\
+        \\x80\\x81\\x82\\x83\\x84\\x85\\x86\\x87\
+        \\x88\\x89\\x8a\\x8b\\x8c\\x8d\\x8e\\x8f\
+        \\x90\\x91\\x92\\x93\\x94\\x95\\x96\\x97\
+        \\x98\\x99\\x9a\\x9b\\x9c\\x9d\\x9e\\x9f\
+        \\xa0\\xa1\\xa2\\xa3\\xa4\\xa5\\xa6\\xa7\
+        \\xa8\\xa9\\xaa\\xab\\xac\\xad\\xae\\xaf\
+        \\xb0\\xb1\\xb2\\xb3\\xb4\\xb5\\xb6\\xb7\
+        \\xb8\\xb9\\xba\\xbb\\xbc\\xbd\\xbe\\xbf\
+        \\xc0\\xc1\\xc2\\xc3\\xc4\\xc5\\xc6\\xc7\
+        \\xc8\\xc9\\xca\\xcb\\xcc\\xcd\\xce\\xcf\
+        \\xd0\\xd1\\xd2\\xd3\\xd4\\xd5\\xd6\\xd7\
+        \\xd8\\xd9\\xda\\xdb\\xdc\\xdd\\xde\\xdf\
+        \\xe0\\xe1\\xe2\\xe3\\xe4\\xe5\\xe6\\xe7\
+        \\xe8\\xe9\\xea\\xeb\\xec\\xed\\xee\\xef\
+        \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\
+        \\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff\"";
+
+    assert_eq!(expected, format!("{:?}", Bytes::from(vec)));
+}