From 1578492e7feb73f3549a27a644226cac15605c26 Mon Sep 17 00:00:00 2001
From: Joeri Samson <joer@samson-vanassche.be>
Date: Mon, 26 Sep 2022 23:50:30 +0200
Subject: [PATCH] Push unsafety to the Rust std library

By making use of the built in methods copy_from_slice (stable since
1.9.0) and copy_within (stable since 1.37.0) we avoid having any
  unsafety ourselves.
This comes at the cost of potentially panicking if our assumptions don't
hold.

In addition I've tried to document the logic behind replace_slice a bit
more and simplified the case where the source and destination slice are
of equal length.
---
 src/lib.rs | 80 ++++++++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 44 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 1b0e13d..503e41e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -47,7 +47,7 @@
 //!   assert_eq!(b.data(), &b"cd"[..]);
 //! }
 //!
-use std::{cmp, ptr};
+use std::cmp;
 use std::io::{self,Write,Read};
 use std::iter::repeat;
 
@@ -213,36 +213,26 @@ impl Buffer {
   /// if the position was more than 0, it is now 0
   pub fn shift(&mut self) {
     if self.position > 0 {
-      unsafe {
-        let length = self.end - self.position;
-        ptr::copy( (&self.memory[self.position..self.end]).as_ptr(), (&mut self.memory[..length]).as_mut_ptr(), length);
-        self.position = 0;
-        self.end      = length;
-      }
+      let length = self.end - self.position;
+      self.memory.copy_within(self.position..self.end, 0);
+      self.position = 0;
+      self.end      = length;
     }
   }
 
-  //FIXME: this should probably be rewritten, and tested extensively
   #[doc(hidden)]
   pub fn delete_slice(&mut self, start: usize, length: usize) -> Option<usize> {
     if start + length >= self.available_data() {
       return None
     }
 
-    unsafe {
-      let begin    = self.position + start;
-      let next_end = self.end - length;
-      ptr::copy(
-        (&self.memory[begin+length..self.end]).as_ptr(),
-        (&mut self.memory[begin..next_end]).as_mut_ptr(),
-        self.end - (begin+length)
-      );
-      self.end = next_end;
-    }
+    let begin    = self.position + start;
+    let next_end = self.end - length;
+    self.memory.copy_within(begin+length..self.end, begin);
+    self.end = next_end;
     Some(self.available_data())
   }
 
-  //FIXME: this should probably be rewritten, and tested extensively
   #[doc(hidden)]
   pub fn replace_slice(&mut self, data: &[u8], start: usize, length: usize) -> Option<usize> {
     let data_len = data.len();
@@ -251,27 +241,33 @@ impl Buffer {
       return None
     }
 
-    unsafe {
-      let begin     = self.position + start;
-      let slice_end = begin + data_len;
-      // we reduced the data size
-      if data_len < length {
-        ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
+    let begin     = self.position + start;
+    let slice_end = begin + data_len;
 
-        ptr::copy((&self.memory[start+length..self.end]).as_ptr(), (&mut self.memory[slice_end..]).as_mut_ptr(), self.end - (start + length));
-        self.end = self.end - (length - data_len);
+    if data_len < length {
+      // we reduced the data size
 
+      // the order here doesn't matter that much, we need to copy the replacement in
+      self.memory[begin..slice_end].copy_from_slice(data);
+      // and move the data from after the original slice to right behind the new slice
+      self.memory.copy_within(begin+length..=self.end, begin + data_len);
+      self.end = self.end - (length - data_len);
+    } else if data_len == length {
+      // the size of the slice and the buffer remains unchanged, only the slice
+      // needs to be written
+      self.memory[begin..slice_end].copy_from_slice(data);
+    } else {
       // we put more data in the buffer
-      } else {
-        ptr::copy((&self.memory[start+length..self.end]).as_ptr(), (&mut self.memory[start+data_len..]).as_mut_ptr(), self.end - (start + length));
-        ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
-        self.end = self.end + data_len - length;
-      }
+
+      // first copy all the data behind the old slice to be behind the new slice
+      self.memory.copy_within(begin+length..self.end, begin + data_len);
+      // then copy the new slice in the vector at the desired location
+      self.memory[begin..slice_end].copy_from_slice(data);
+      self.end = self.end + data_len - length;
     }
     Some(self.available_data())
   }
 
-  //FIXME: this should probably be rewritten, and tested extensively
   #[doc(hidden)]
   pub fn insert_slice(&mut self, data: &[u8], start: usize) -> Option<usize> {
     let data_len = data.len();
@@ -280,13 +276,11 @@ impl Buffer {
       return None
     }
 
-    unsafe {
-      let begin     = self.position + start;
-      let slice_end = begin + data_len;
-      ptr::copy((&self.memory[start..self.end]).as_ptr(), (&mut self.memory[start+data_len..]).as_mut_ptr(), self.end - start);
-      ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
-      self.end = self.end + data_len;
-    }
+    let begin     = self.position + start;
+    let slice_end = begin + data_len;
+    self.memory.copy_within(start..self.end, start+data_len);
+    self.memory[begin..slice_end].copy_from_slice(data);
+    self.end = self.end + data_len;
     Some(self.available_data())
   }
 }
@@ -307,10 +301,8 @@ impl Write for Buffer {
 impl Read for Buffer {
   fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
     let len = cmp::min(self.available_data(), buf.len());
-    unsafe {
-      ptr::copy((&self.memory[self.position..self.position+len]).as_ptr(), buf.as_mut_ptr(), len);
-      self.position += len;
-    }
+    buf[0..len].copy_from_slice(&self.memory[self.position..self.position+len]);
+    self.position += len;
     Ok(len)
   }
 }