From e93b8670dbd1545350464883d5405f2042126c59 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 00:35:54 -0800 Subject: [PATCH 1/7] implicitly grow BytesMut; add BufMutExt::chain_mut This brings `BytesMut` in line with `Vec` behavior. In order to fix a test, `BufMutExt::chain_mut` is provided. Withou this, it is not possible to chain two `&mut [u8]`. Closes #170 --- src/buf/ext/mod.rs | 26 ++++++++++++++++++++++++++ src/bytes_mut.rs | 6 +++++- tests/test_buf_mut.rs | 2 +- tests/test_bytes.rs | 4 ++-- tests/test_chain.rs | 13 +++++-------- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/buf/ext/mod.rs b/src/buf/ext/mod.rs index 10b9a34c0..452535a70 100644 --- a/src/buf/ext/mod.rs +++ b/src/buf/ext/mod.rs @@ -124,6 +124,32 @@ pub trait BufMutExt: BufMut { fn writer(self) -> Writer where Self: Sized { writer::new(self) } + + /// Creates an adaptor which will chain this buffer with another. + /// + /// The returned `BufMut` instance will first write to all bytes from + /// `self`. Afterwards, it will write to `next`. + /// + /// # Examples + /// + /// ``` + /// use bytes::{BufMut, buf::BufMutExt}; + /// + /// let mut a = [0u8; 5]; + /// let mut b = [0u8; 6]; + /// + /// let mut chain = (&mut a[..]).chain_mut(&mut b[..]); + /// + /// chain.put_slice(b"hello world"); + /// + /// assert_eq!(&a[..], b"hello"); + /// assert_eq!(&b[..], b" world"); + /// ``` + fn chain_mut(self, next: U) -> Chain + where Self: Sized + { + Chain::new(self, next) + } } impl BufMutExt for B {} diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 6cf791a01..d030dcb9d 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -924,7 +924,7 @@ impl Buf for BytesMut { impl BufMut for BytesMut { #[inline] fn remaining_mut(&self) -> usize { - self.capacity() - self.len() + usize::MAX - self.len() } #[inline] @@ -936,6 +936,10 @@ impl BufMut for BytesMut { #[inline] fn bytes_mut(&mut self) -> &mut [mem::MaybeUninit] { + if self.capacity() == self.len() { + self.reserve(64); + } + unsafe { slice::from_raw_parts_mut(self.ptr.as_ptr().offset(self.len as isize) as *mut mem::MaybeUninit, self.cap) } diff --git a/tests/test_buf_mut.rs b/tests/test_buf_mut.rs index 0d372d1ec..dd414e5a2 100644 --- a/tests/test_buf_mut.rs +++ b/tests/test_buf_mut.rs @@ -74,7 +74,7 @@ fn test_bufs_vec_mut() { // with no capacity let mut buf = BytesMut::new(); assert_eq!(buf.capacity(), 0); - assert_eq!(0, buf.bytes_vectored_mut(&mut dst[..])); + assert_eq!(1, buf.bytes_vectored_mut(&mut dst[..])); // with capacity let mut buf = BytesMut::with_capacity(64); diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 9fa8019ee..918210695 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -93,8 +93,8 @@ fn fmt_write() { let mut c = BytesMut::with_capacity(64); - write!(c, "{}", s).unwrap_err(); - assert!(c.is_empty()); + write!(c, "{}", s).unwrap(); + assert_eq!(c, s[..].as_bytes()); } #[test] diff --git a/tests/test_chain.rs b/tests/test_chain.rs index 877618843..332571d8b 100644 --- a/tests/test_chain.rs +++ b/tests/test_chain.rs @@ -1,7 +1,7 @@ #![deny(warnings, rust_2018_idioms)] -use bytes::{Buf, BufMut, Bytes, BytesMut}; -use bytes::buf::BufExt; +use bytes::{Buf, BufMut, Bytes}; +use bytes::buf::{BufExt, BufMutExt}; use std::io::IoSlice; #[test] @@ -15,20 +15,17 @@ fn collect_two_bufs() { #[test] fn writing_chained() { - let mut a = BytesMut::with_capacity(64); - let mut b = BytesMut::with_capacity(64); + let mut a = [0u8; 64]; + let mut b = [0u8; 64]; { - let mut buf = (&mut a).chain(&mut b); + let mut buf = (&mut a[..]).chain_mut(&mut b[..]); for i in 0u8..128 { buf.put_u8(i); } } - assert_eq!(64, a.len()); - assert_eq!(64, b.len()); - for i in 0..64 { let expect = i as u8; assert_eq!(expect, a[i]); From 685e1be408739ab2604a196243811eefc6d45cb1 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 10:50:25 -0800 Subject: [PATCH 2/7] fix bug in BytesMut::bytes_mut --- src/buf/buf_mut.rs | 2 +- src/bytes_mut.rs | 4 ++-- tests/test_bytes.rs | 22 ++++++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/buf/buf_mut.rs b/src/buf/buf_mut.rs index 8ec568aae..d29591010 100644 --- a/src/buf/buf_mut.rs +++ b/src/buf/buf_mut.rs @@ -270,7 +270,7 @@ pub trait BufMut { fn put_slice(&mut self, src: &[u8]) { let mut off = 0; - assert!(self.remaining_mut() >= src.len(), "buffer overflow"); + assert!(self.remaining_mut() >= src.len(), "buffer overflow; remaining = {}; src = {}", self.remaining_mut(), src.len()); while off < src.len() { let cnt; diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index d030dcb9d..58ac7550b 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -930,7 +930,7 @@ impl BufMut for BytesMut { #[inline] unsafe fn advance_mut(&mut self, cnt: usize) { let new_len = self.len() + cnt; - assert!(new_len <= self.cap); + assert!(new_len <= self.cap, "new_len = {}; capacity = {}", new_len, self.cap); self.len = new_len; } @@ -941,7 +941,7 @@ impl BufMut for BytesMut { } unsafe { - slice::from_raw_parts_mut(self.ptr.as_ptr().offset(self.len as isize) as *mut mem::MaybeUninit, self.cap) + slice::from_raw_parts_mut(self.ptr.as_ptr().offset(self.len as isize) as *mut mem::MaybeUninit, self.cap - self.len) } } } diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 918210695..d5bcf2e9b 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -820,3 +820,25 @@ fn empty_slice_ref_catches_not_an_empty_subset() { bytes.slice_ref(slice); } + +#[test] +fn bytes_buf_mut_advance() { + let mut bytes = BytesMut::with_capacity(1024); + + unsafe { + let ptr = bytes.bytes_mut().as_ptr(); + assert_eq!(1024, bytes.bytes_mut().len()); + + bytes.advance_mut(10); + + let next = bytes.bytes_mut().as_ptr(); + assert_eq!(1024 - 10, bytes.bytes_mut().len()); + assert_eq!(ptr.offset(10), next); + + // advance to the end + bytes.advance_mut(1024 - 10); + + // The buffer size is doubled + assert_eq!(1024, bytes.bytes_mut().len()); + } +} From 597ab3d2f79b5fce130b7277afa6ac9572dcdfcd Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 11:07:56 -0800 Subject: [PATCH 3/7] use more recent rust versions on CI --- azure-pipelines.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4c221821f..81bd54079 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -20,7 +20,7 @@ jobs: - template: ci/azure-test-stable.yml parameters: name: minrust - rust_version: 1.36.0 + rust_version: 1.39.0 cmd: check # Stable @@ -37,7 +37,7 @@ jobs: parameters: name: nightly # Pin nightly to avoid being impacted by breakage - rust_version: nightly-2019-07-17 + rust_version: nightly-2019-11-20 benches: true # Run tests on some extra platforms From 083f1d08ccb1938046b880b2312da36eead254f9 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 11:41:27 -0800 Subject: [PATCH 4/7] Update src/buf/ext/mod.rs Co-Authored-By: Eliza Weisman --- src/buf/ext/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/buf/ext/mod.rs b/src/buf/ext/mod.rs index 452535a70..2fefde90b 100644 --- a/src/buf/ext/mod.rs +++ b/src/buf/ext/mod.rs @@ -125,7 +125,7 @@ pub trait BufMutExt: BufMut { writer::new(self) } - /// Creates an adaptor which will chain this buffer with another. + /// Creates an adapter which will chain this buffer with another. /// /// The returned `BufMut` instance will first write to all bytes from /// `self`. Afterwards, it will write to `next`. From d186e5336b1e55859e75a0ef73c87f4f2966cdef Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 11:45:15 -0800 Subject: [PATCH 5/7] revert change to azure-pipelines.yml --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 81bd54079..416e1fb7e 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -37,7 +37,7 @@ jobs: parameters: name: nightly # Pin nightly to avoid being impacted by breakage - rust_version: nightly-2019-11-20 + rust_version: nightly-2019-09-25 benches: true # Run tests on some extra platforms From 50fff99b223cdd680f7fa9ebef853d925420e6a9 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 11:48:18 -0800 Subject: [PATCH 6/7] break up line --- src/bytes_mut.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 58ac7550b..1fd6a9148 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -941,7 +941,10 @@ impl BufMut for BytesMut { } unsafe { - slice::from_raw_parts_mut(self.ptr.as_ptr().offset(self.len as isize) as *mut mem::MaybeUninit, self.cap - self.len) + let ptr = self.ptr.as_ptr().offset(self.len as isize); + let len = self.cap - self.len + + slice::from_raw_parts_mut(ptr as *mut mem::MaybeUninit, len) } } } From 2a63674f68021b788b86ae95568a1685b74f173b Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 20 Nov 2019 11:56:14 -0800 Subject: [PATCH 7/7] handle overflow in BytesMut::reserve --- src/bytes_mut.rs | 24 +++++++++++------------- tests/test_bytes.rs | 11 +++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 1fd6a9148..ea71a9aaf 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -1,4 +1,5 @@ -use core::{cmp, fmt, hash, isize, mem, slice, usize}; +use core::{cmp, fmt, hash, isize, slice, usize}; +use core::mem::{self, ManuallyDrop}; use core::ops::{Deref, DerefMut}; use core::ptr::{self, NonNull}; use core::iter::{FromIterator, Iterator}; @@ -559,17 +560,15 @@ impl BytesMut { self.cap += off; } else { // No space - allocate more - let mut v = rebuild_vec(self.ptr.as_ptr(), self.len, self.cap, off); + let mut v = ManuallyDrop::new(rebuild_vec(self.ptr.as_ptr(), self.len, self.cap, off)); v.reserve(additional); // Update the info self.ptr = vptr(v.as_mut_ptr().offset(off as isize)); self.len = v.len() - off; self.cap = v.capacity() - off; - - // Drop the vec reference - mem::forget(v); } + return; } } @@ -582,7 +581,8 @@ impl BytesMut { // allocating a new vector with the requested capacity. // // Compute the new capacity - let mut new_cap = len + additional; + let mut new_cap = len.checked_add(additional).expect("overflow"); + let original_capacity; let original_capacity_repr; @@ -618,8 +618,10 @@ impl BytesMut { // There are some situations, using `reserve_exact` that the // buffer capacity could be below `original_capacity`, so do a // check. + let double = v.capacity().checked_shl(1).unwrap_or(new_cap); + new_cap = cmp::max( - cmp::max(v.capacity() << 1, new_cap), + cmp::max(double, new_cap), original_capacity); } else { new_cap = cmp::max(new_cap, original_capacity); @@ -627,7 +629,7 @@ impl BytesMut { } // Create a new vector to store the data - let mut v = Vec::with_capacity(new_cap); + let mut v = ManuallyDrop::new(Vec::with_capacity(new_cap)); // Copy the bytes v.extend_from_slice(self.as_ref()); @@ -642,10 +644,6 @@ impl BytesMut { self.ptr = vptr(v.as_mut_ptr()); self.len = v.len(); self.cap = v.capacity(); - - // Forget the vector handle - mem::forget(v); - } /// Appends given bytes to this object. /// @@ -942,7 +940,7 @@ impl BufMut for BytesMut { unsafe { let ptr = self.ptr.as_ptr().offset(self.len as isize); - let len = self.cap - self.len + let len = self.cap - self.len; slice::from_raw_parts_mut(ptr as *mut mem::MaybeUninit, len) } diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index d5bcf2e9b..6e3c4b6da 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -2,6 +2,8 @@ use bytes::{Bytes, BytesMut, Buf, BufMut}; +use std::usize; + const LONG: &'static [u8] = b"mary had a little lamb, little lamb, little lamb"; const SHORT: &'static [u8] = b"hello world"; @@ -842,3 +844,12 @@ fn bytes_buf_mut_advance() { assert_eq!(1024, bytes.bytes_mut().len()); } } + +#[test] +#[should_panic] +fn bytes_reserve_overflow() { + let mut bytes = BytesMut::with_capacity(1024); + bytes.put_slice(b"hello world"); + + bytes.reserve(usize::MAX); +}