refactor: reuse copy_{from,to}_slice for {extend_from,drain_to}_slice + loosen slice size checks

Author: cdellacqua

src/lib.rs

@@ -1696,6 +1696,47 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_copy_from_slice_partial() {
+        macro_rules! test_concrete {
+            ($rb_init: expr) => {
+                let mut rb = $rb_init(&[3, 2, 1]);
+                assert_eq!(rb.capacity(), 7);
+                // we have some space left
+                assert!(rb.len() < rb.capacity());
+
+                // copy preserves length
+                rb.copy_from_slice(0, &[1, 2]);
+                assert_eq!(rb.to_vec(), alloc::vec![1, 2, 1]);
+
+                let _ = rb.enqueue(4);
+                let _ = rb.enqueue(5);
+                let _ = rb.enqueue(6);
+                assert_eq!(rb.to_vec(), alloc::vec![1, 2, 1, 4, 5, 6]);
+
+                // still preserving length
+                rb.copy_from_slice(1, &[5, 4, 3, 2, 1]);
+                assert_eq!(rb.to_vec(), alloc::vec![1, 5, 4, 3, 2, 1]);
+            };
+        }
+
+        test_concrete!(|values: &[i32]| {
+            let mut rb = ConstGenericRingBuffer::<_, 7>::new();
+            rb.extend(values.iter().copied());
+            rb
+        });
+        test_concrete!(|values: &[i32]| {
+            let mut rb = GrowableAllocRingBuffer::<_>::with_capacity(7);
+            rb.extend(values.iter().copied());
+            rb
+        });
+        test_concrete!(|values: &[i32]| {
+            let mut rb = AllocRingBuffer::<_>::new(7);
+            rb.extend(values.iter().copied());
+            rb
+        });
+    }
+
     #[test]
     fn test_copy_from_slice_empty() {
         macro_rules! test_concrete {
@@ -1907,6 +1948,60 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_copy_to_slice_partial() {
+        macro_rules! test_concrete {
+            ($rb_init: expr) => {
+                let mut rb = $rb_init(&[1, 2, 3]);
+                assert_eq!(rb.capacity(), 7);
+                // we have some space left
+                assert!(rb.len() < rb.capacity());
+
+                // copy based on length
+                let mut slice = [0; 2];
+                rb.copy_to_slice(0, &mut slice);
+                assert_eq!(slice.as_slice(), &[1, 2]);
+
+                let _ = rb.enqueue(4);
+                let _ = rb.enqueue(5);
+                let _ = rb.enqueue(6);
+                // still based on length
+                let mut slice = [0; 5];
+                rb.copy_to_slice(0, &mut slice);
+                assert_eq!(slice.as_slice(), &[1, 2, 3, 4, 5]);
+
+                // making sure the read/write ptrs have traversed the ring
+                for i in 0..6 {
+                    let _ = rb.enqueue(i + 1);
+                    let _ = rb.dequeue();
+                }
+
+                // sanity check
+                assert_eq!(rb.to_vec(), alloc::vec![1, 2, 3, 4, 5, 6]);
+                // copy again
+                let mut slice = [0; 5];
+                rb.copy_to_slice(1, &mut slice);
+                assert_eq!(slice.as_slice(), &[2, 3, 4, 5, 6]);
+            };
+        }
+
+        test_concrete!(|values: &[i32]| {
+            let mut rb = ConstGenericRingBuffer::<_, 7>::new();
+            rb.extend(values.iter().copied());
+            rb
+        });
+        test_concrete!(|values: &[i32]| {
+            let mut rb = GrowableAllocRingBuffer::<_>::with_capacity(7);
+            rb.extend(values.iter().copied());
+            rb
+        });
+        test_concrete!(|values: &[i32]| {
+            let mut rb = AllocRingBuffer::<_>::new(7);
+            rb.extend(values.iter().copied());
+            rb
+        });
+    }
+
     #[test]
     fn test_copy_to_slice_empty() {
         macro_rules! test_concrete {
@@ -2124,6 +2219,20 @@ mod tests {
         test_concrete!(|| AllocRingBuffer::<i32>::new(3));
     }
 
+    #[test]
+    fn test_extend_from_slice_partial() {
+        macro_rules! test_concrete {
+            ($rb_init: expr) => {
+                let mut rb = $rb_init();
+                rb.extend_from_slice(&[9; 2]);
+                assert_eq!(&rb.to_vec(), &[9, 9]);
+            };
+        }
+
+        test_concrete!(ConstGenericRingBuffer::<i32, 3>::new);
+        test_concrete!(|| AllocRingBuffer::<i32>::new(3));
+    }
+
     #[test]
     fn test_extend_from_slice_empty() {
         macro_rules! test_concrete {
@@ -2218,6 +2327,32 @@ mod tests {
         test_concrete!(|| AllocRingBuffer::<i32>::new(3));
     }
 
+    #[test]
+    fn test_drain_to_slice_partial() {
+        macro_rules! test_concrete {
+            ($rb_init: expr, $growable: expr) => {
+                let mut rb = $rb_init();
+                rb.extend_from_slice(&[9; 3]);
+                let mut slice = [0; 2];
+                rb.drain_to_slice(&mut slice);
+                assert_eq!(&slice, &[9; 2]);
+                rb.extend_from_slice(&[1, 2, 3]);
+                rb.drain_to_slice(&mut slice);
+                if $growable {
+                    assert_eq!(&slice, &[9, 1]);
+                    assert_eq!(&rb.to_vec(), &[2, 3]);
+                } else {
+                    assert_eq!(&slice, &[1, 2]);
+                    assert_eq!(&rb.to_vec(), &[3]);
+                }
+            };
+        }
+
+        test_concrete!(ConstGenericRingBuffer::<i32, 3>::new, false);
+        test_concrete!(|| GrowableAllocRingBuffer::<i32>::with_capacity(3), true);
+        test_concrete!(|| AllocRingBuffer::<i32>::new(3), false);
+    }
+
     #[test]
     fn test_drain_to_slice_empty() {
         macro_rules! test_concrete {
@@ -2269,13 +2404,16 @@ mod tests {
     }
 
     #[test]
-    #[should_panic = "destination slice length (10) greater than buffer length (0)"]
     fn test_drain_to_slice_longer_than_len() {
         macro_rules! test_concrete {
             ($rb_init: expr) => {
                 let mut rb = $rb_init();
                 let mut slice = [0; 10];
                 rb.drain_to_slice(&mut slice);
+                assert_eq!(slice, [0; 10]);
+                rb.extend_from_slice(&[1, 2]);
+                rb.drain_to_slice(&mut slice);
+                assert_eq!(slice, [1, 2, 0, 0, 0, 0, 0, 0, 0, 0]);
             };
         }
 

src/ringbuffer_trait.rs

@@ -257,7 +257,7 @@ pub unsafe trait RingBuffer<T>:
     /// Efficiently copy items from the ringbuffer to a target slice.
     ///
     /// # Panics
-    /// Panics if the buffer length minus the offset is NOT equal to `target.len()`.
+    /// Panics if the buffer length minus the offset is smaller than `dst.len()`.
     ///
     /// # Safety
     /// ONLY SAFE WHEN self is a *const to to an implementor of `RingBuffer`
@@ -268,7 +268,7 @@ pub unsafe trait RingBuffer<T>:
     /// Efficiently copy items from the ringbuffer to a target slice.
     ///
     /// # Panics
-    /// Panics if the buffer length minus the offset is NOT equal to `target.len()`.
+    /// Panics if the buffer length minus the offset is smaller than `dst.len()`.
     fn copy_to_slice(&self, offset: usize, dst: &mut [T])
     where
         T: Copy,
@@ -277,8 +277,9 @@ pub unsafe trait RingBuffer<T>:
     }
 
     /// Efficiently copy items from a slice to the ringbuffer.
+    ///
     /// # Panics
-    /// Panics if the buffer length minus the offset is NOT equal to `source.len()`.
+    /// Panics if the buffer length minus the offset is smaller than `src.len()`.
     ///
     /// # Safety
     /// ONLY SAFE WHEN self is a *mut to to an implementor of `RingBuffer`
@@ -289,7 +290,7 @@ pub unsafe trait RingBuffer<T>:
     /// Efficiently copy items from a slice to the ringbuffer.
     ///
     /// # Panics
-    /// Panics if the buffer length minus the offset is NOT equal to `source.len()`.
+    /// Panics if the buffer length minus the offset is smaller than `src.len()`.
     fn copy_from_slice(&mut self, offset: usize, src: &[T])
     where
         T: Copy,
@@ -688,7 +689,7 @@ macro_rules! impl_ringbuffer_ext {
                 (offset == 0 && len == 0) || offset < len,
                 "offset ({offset}) is out of bounds for the current buffer length ({len})"
             );
-            assert!(len - offset == dst_len, "destination slice length ({dst_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})");
+            assert!(len - offset >= dst_len, "destination slice length ({dst_len}) greater than buffer length ({len}) when considering the specified offset ({offset})");
 
             if dst_len == 0 {
                 return;
@@ -731,7 +732,7 @@ macro_rules! impl_ringbuffer_ext {
                 (offset == 0 && len == 0) || offset < len,
                 "offset ({offset}) is out of bounds for the current buffer length ({len})"
             );
-            assert!(len - offset == src_len, "source slice length ({src_len}) doesn't match buffer length ({len}) when considering the specified offset ({offset})");
+            assert!(len - offset >= src_len, "source slice length ({src_len}) greater than buffer length ({len}) when considering the specified offset ({offset})");
 
             if src_len == 0 {
                 return;
@@ -771,97 +772,25 @@ macro_rules! impl_ringbuffer_ext {
         where
             T: Copy,
         {
+            let len = Self::ptr_len(rb);
             let capacity = Self::ptr_capacity(rb);
+            (*rb).$writeptr += src.len();
+            (*rb).$readptr = (*rb).$writeptr - (len + src.len()).min(capacity);
+            let final_len = Self::ptr_len(rb);
 
-            if src.len() == 0 {
-                return;
-            }
-
-            let truncated_src = if src.len() > capacity {
-                &src[src.len() - capacity..]
-            } else {
-                src
-            };
-
-            let truncated_src_len = truncated_src.len();
-
-            let base: *mut T = $get_base_mut_ptr(rb);
-            let size = Self::ptr_buffer_size(rb);
-
-            let from_idx = $mask(size, (*rb).$writeptr);
-            let to_idx = $mask(size, (*rb).$writeptr + truncated_src_len);
-
-            if from_idx < to_idx {
-                unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts_mut(base.add(from_idx), truncated_src_len)
-                }
-                .copy_from_slice(truncated_src);
-            } else {
-                unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts_mut(base.add(from_idx), size - from_idx)
-                }
-                .copy_from_slice(&truncated_src[..size - from_idx]);
-                unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts_mut(base, to_idx)
-                }
-                .copy_from_slice(&truncated_src[size - from_idx..]);
-            }
-
-
-            let initially_available = capacity - Self::ptr_len(rb);
-            if truncated_src_len > initially_available {
-                (*rb).$readptr += truncated_src_len - initially_available;
-            }
-            (*rb).$writeptr += truncated_src_len;
+            let src_offset = src.len().saturating_sub(capacity);
+            Self::ptr_copy_from_slice(rb, final_len - (src.len() - src_offset), &src[src_offset..]);
         }
 
         unsafe fn ptr_drain_to_slice(rb: *mut Self, dst: &mut [T])
         where
             T: Copy,
         {
-            let dst_len = dst.len();
             let len = Self::ptr_len(rb);
-            assert!(
-                dst_len <= len,
-                "destination slice length ({dst_len}) greater than buffer length ({len})"
-            );
-
-            if dst_len == 0 {
-                return;
-            }
-
-            let base: *mut T = $get_base_mut_ptr(rb);
-            let size = Self::ptr_buffer_size(rb);
-
-            let from_idx = $mask(size, (*rb).$readptr);
-            let to_idx = $mask(size, (*rb).$readptr + dst_len);
-
-            if from_idx < to_idx {
-                dst.copy_from_slice(unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts(base.add(from_idx), dst_len)
-                });
-            } else {
-                dst[..size - from_idx].copy_from_slice(unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts(base.add(from_idx), size - from_idx)
-                });
-                dst[size - from_idx..].copy_from_slice(unsafe {
-                    // SAFETY: index has been modulo-ed to be within range
-                    // to be within bounds
-                    core::slice::from_raw_parts(base, to_idx)
-                });
-            }
+            let truncated_dst_len = dst.len().min(len);
+            Self::ptr_copy_to_slice(rb, 0, &mut dst[..truncated_dst_len]);
 
-            (*rb).$readptr += dst_len;
+            (*rb).$readptr += truncated_dst_len;
         }
     };
 }