From ea9dd6050463185b8d4a9022218763902abd7c1c Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:08:43 -0500 Subject: [PATCH 1/7] set len a little more concisely --- src/bytes_mut.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 88d596cf6..155d686fb 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -888,13 +888,7 @@ impl BytesMut { // new start and updating the `len` field to reflect the new length // of the view. self.ptr = vptr(self.ptr.as_ptr().add(start)); - - if self.len >= start { - self.len -= start; - } else { - self.len = 0; - } - + self.len = self.len.checked_sub(start).unwrap_or(0); self.cap -= start; } From 77a6f1c94ddae613bedad04a13299849098357ee Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:11:58 -0500 Subject: [PATCH 2/7] inline set_end --- src/bytes_mut.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 155d686fb..e9fd9d8aa 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -318,7 +318,11 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); other.set_start(at); - self.set_end(at); + debug_assert_eq!(self.kind(), KIND_ARC); + assert!(at <= self.cap, "set_end out of bounds"); + + self.cap = at; + self.len = cmp::min(self.len, at); other } } @@ -391,7 +395,11 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); - other.set_end(at); + debug_assert_eq!(other.kind(), KIND_ARC); + assert!(at <= other.cap, "set_end out of bounds"); + + other.cap = at; + other.len = cmp::min(other.len, at); self.set_start(at); other } @@ -892,14 +900,6 @@ impl BytesMut { self.cap -= start; } - unsafe fn set_end(&mut self, end: usize) { - debug_assert_eq!(self.kind(), KIND_ARC); - assert!(end <= self.cap, "set_end out of bounds"); - - self.cap = end; - self.len = cmp::min(self.len, end); - } - fn try_unsplit(&mut self, other: BytesMut) -> Result<(), BytesMut> { if other.capacity() == 0 { return Ok(()); From 9bdf5ebad25357afd803c94085f4b0d43acc42d5 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:12:27 -0500 Subject: [PATCH 3/7] remove kind assertions The shallow clone call just above always results in the kind being shared so we don't need to assert it here. --- src/bytes_mut.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index e9fd9d8aa..cbdde8d31 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -318,7 +318,6 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); other.set_start(at); - debug_assert_eq!(self.kind(), KIND_ARC); assert!(at <= self.cap, "set_end out of bounds"); self.cap = at; @@ -395,7 +394,6 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); - debug_assert_eq!(other.kind(), KIND_ARC); assert!(at <= other.cap, "set_end out of bounds"); other.cap = at; From 00a60ebef45d495cc959d940d51927e11dfcd115 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:13:35 -0500 Subject: [PATCH 4/7] remove duplicate assertion we already assert this at the top of the method --- src/bytes_mut.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index cbdde8d31..f3ad42e80 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -318,8 +318,6 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); other.set_start(at); - assert!(at <= self.cap, "set_end out of bounds"); - self.cap = at; self.len = cmp::min(self.len, at); other From 00f2b20c581d1f592cb95010e56f379953b8e125 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:13:59 -0500 Subject: [PATCH 5/7] remove redundant assertion and min We know several things here: 1. self.len <= self.cap, always 2. at <= self.len, asserted at the top of this method 3. after calling shallow_clone, other.cap == self.cap Therefore, at <= self.len <= other.cap. --- src/bytes_mut.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index f3ad42e80..5aaf2958d 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -392,10 +392,8 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); - assert!(at <= other.cap, "set_end out of bounds"); - other.cap = at; - other.len = cmp::min(other.len, at); + other.len = at; self.set_start(at); other } From 442f85d9596f4667545b3e3954830a0e1970b0b4 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Sun, 21 Jan 2024 12:33:15 -0500 Subject: [PATCH 6/7] rename set_start to advance_unchecked This method never moves the cursor backward, only advances it forwards. I think reflecting that in the name makes things a bit more clear. I also added explicit safety comments to make it clear why each usage is sound. --- src/bytes_mut.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 5aaf2958d..e3a9ddd2c 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -317,7 +317,8 @@ impl BytesMut { ); unsafe { let mut other = self.shallow_clone(); - other.set_start(at); + // SAFETY: We've checked that `at` <= `self.capacity()` above. + other.advance_unchecked(at); self.cap = at; self.len = cmp::min(self.len, at); other @@ -394,7 +395,9 @@ impl BytesMut { let mut other = self.shallow_clone(); other.cap = at; other.len = at; - self.set_start(at); + // SAFETY: We've checked that `at` <= `self.len()` and we know that `self.len()` <= + // `self.capacity()`. + self.advance_unchecked(at); other } } @@ -856,14 +859,19 @@ impl BytesMut { unsafe { slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len) } } - unsafe fn set_start(&mut self, start: usize) { + /// Advance the buffer without bounds checking. + /// + /// # SAFETY + /// + /// The caller must ensure that `count` <= `self.cap`. + unsafe fn advance_unchecked(&mut self, count: usize) { // Setting the start to 0 is a no-op, so return early if this is the // case. - if start == 0 { + if count == 0 { return; } - debug_assert!(start <= self.cap, "internal: set_start out of bounds"); + debug_assert!(count <= self.cap, "internal: set_start out of bounds"); let kind = self.kind(); @@ -873,7 +881,7 @@ impl BytesMut { // "start" of the byte buffer from the beginning of the vec. We // also have to ensure that we don't exceed the maximum shift. let (mut pos, prev) = self.get_vec_pos(); - pos += start; + pos += count; if pos <= MAX_VEC_POS { self.set_vec_pos(pos, prev); @@ -889,9 +897,9 @@ impl BytesMut { // Updating the start of the view is setting `ptr` to point to the // new start and updating the `len` field to reflect the new length // of the view. - self.ptr = vptr(self.ptr.as_ptr().add(start)); - self.len = self.len.checked_sub(start).unwrap_or(0); - self.cap -= start; + self.ptr = vptr(self.ptr.as_ptr().add(count)); + self.len = self.len.checked_sub(count).unwrap_or(0); + self.cap -= count; } fn try_unsplit(&mut self, other: BytesMut) -> Result<(), BytesMut> { @@ -1062,7 +1070,9 @@ impl Buf for BytesMut { self.remaining(), ); unsafe { - self.set_start(cnt); + // SAFETY: We've checked that `cnt` <= `self.remaining()` and we know that + // `self.remaining()` <= `self.cap`. + self.advance_unchecked(cnt); } } From b97174340bad78f2df33d8fb8bef9c841c6e511b Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Tue, 6 Feb 2024 16:20:31 -0500 Subject: [PATCH 7/7] consistency in split_off and split_to --- src/bytes_mut.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index e485363d4..5784ff871 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -393,11 +393,11 @@ impl BytesMut { unsafe { let mut other = self.shallow_clone(); - other.cap = at; - other.len = at; // SAFETY: We've checked that `at` <= `self.len()` and we know that `self.len()` <= // `self.capacity()`. self.advance_unchecked(at); + other.cap = at; + other.len = at; other } }