Skip to content

Commit 1578492

Browse files
author
Joeri Samson
committed
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.
1 parent 8d9566d commit 1578492

File tree

1 file changed

+36
-44
lines changed

1 file changed

+36
-44
lines changed

src/lib.rs

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
//! assert_eq!(b.data(), &b"cd"[..]);
4848
//! }
4949
//!
50-
use std::{cmp, ptr};
50+
use std::cmp;
5151
use std::io::{self,Write,Read};
5252
use std::iter::repeat;
5353

@@ -213,36 +213,26 @@ impl Buffer {
213213
/// if the position was more than 0, it is now 0
214214
pub fn shift(&mut self) {
215215
if self.position > 0 {
216-
unsafe {
217-
let length = self.end - self.position;
218-
ptr::copy( (&self.memory[self.position..self.end]).as_ptr(), (&mut self.memory[..length]).as_mut_ptr(), length);
219-
self.position = 0;
220-
self.end = length;
221-
}
216+
let length = self.end - self.position;
217+
self.memory.copy_within(self.position..self.end, 0);
218+
self.position = 0;
219+
self.end = length;
222220
}
223221
}
224222

225-
//FIXME: this should probably be rewritten, and tested extensively
226223
#[doc(hidden)]
227224
pub fn delete_slice(&mut self, start: usize, length: usize) -> Option<usize> {
228225
if start + length >= self.available_data() {
229226
return None
230227
}
231228

232-
unsafe {
233-
let begin = self.position + start;
234-
let next_end = self.end - length;
235-
ptr::copy(
236-
(&self.memory[begin+length..self.end]).as_ptr(),
237-
(&mut self.memory[begin..next_end]).as_mut_ptr(),
238-
self.end - (begin+length)
239-
);
240-
self.end = next_end;
241-
}
229+
let begin = self.position + start;
230+
let next_end = self.end - length;
231+
self.memory.copy_within(begin+length..self.end, begin);
232+
self.end = next_end;
242233
Some(self.available_data())
243234
}
244235

245-
//FIXME: this should probably be rewritten, and tested extensively
246236
#[doc(hidden)]
247237
pub fn replace_slice(&mut self, data: &[u8], start: usize, length: usize) -> Option<usize> {
248238
let data_len = data.len();
@@ -251,27 +241,33 @@ impl Buffer {
251241
return None
252242
}
253243

254-
unsafe {
255-
let begin = self.position + start;
256-
let slice_end = begin + data_len;
257-
// we reduced the data size
258-
if data_len < length {
259-
ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
244+
let begin = self.position + start;
245+
let slice_end = begin + data_len;
260246

261-
ptr::copy((&self.memory[start+length..self.end]).as_ptr(), (&mut self.memory[slice_end..]).as_mut_ptr(), self.end - (start + length));
262-
self.end = self.end - (length - data_len);
247+
if data_len < length {
248+
// we reduced the data size
263249

250+
// the order here doesn't matter that much, we need to copy the replacement in
251+
self.memory[begin..slice_end].copy_from_slice(data);
252+
// and move the data from after the original slice to right behind the new slice
253+
self.memory.copy_within(begin+length..=self.end, begin + data_len);
254+
self.end = self.end - (length - data_len);
255+
} else if data_len == length {
256+
// the size of the slice and the buffer remains unchanged, only the slice
257+
// needs to be written
258+
self.memory[begin..slice_end].copy_from_slice(data);
259+
} else {
264260
// we put more data in the buffer
265-
} else {
266-
ptr::copy((&self.memory[start+length..self.end]).as_ptr(), (&mut self.memory[start+data_len..]).as_mut_ptr(), self.end - (start + length));
267-
ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
268-
self.end = self.end + data_len - length;
269-
}
261+
262+
// first copy all the data behind the old slice to be behind the new slice
263+
self.memory.copy_within(begin+length..self.end, begin + data_len);
264+
// then copy the new slice in the vector at the desired location
265+
self.memory[begin..slice_end].copy_from_slice(data);
266+
self.end = self.end + data_len - length;
270267
}
271268
Some(self.available_data())
272269
}
273270

274-
//FIXME: this should probably be rewritten, and tested extensively
275271
#[doc(hidden)]
276272
pub fn insert_slice(&mut self, data: &[u8], start: usize) -> Option<usize> {
277273
let data_len = data.len();
@@ -280,13 +276,11 @@ impl Buffer {
280276
return None
281277
}
282278

283-
unsafe {
284-
let begin = self.position + start;
285-
let slice_end = begin + data_len;
286-
ptr::copy((&self.memory[start..self.end]).as_ptr(), (&mut self.memory[start+data_len..]).as_mut_ptr(), self.end - start);
287-
ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
288-
self.end = self.end + data_len;
289-
}
279+
let begin = self.position + start;
280+
let slice_end = begin + data_len;
281+
self.memory.copy_within(start..self.end, start+data_len);
282+
self.memory[begin..slice_end].copy_from_slice(data);
283+
self.end = self.end + data_len;
290284
Some(self.available_data())
291285
}
292286
}
@@ -307,10 +301,8 @@ impl Write for Buffer {
307301
impl Read for Buffer {
308302
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
309303
let len = cmp::min(self.available_data(), buf.len());
310-
unsafe {
311-
ptr::copy((&self.memory[self.position..self.position+len]).as_ptr(), buf.as_mut_ptr(), len);
312-
self.position += len;
313-
}
304+
buf[0..len].copy_from_slice(&self.memory[self.position..self.position+len]);
305+
self.position += len;
314306
Ok(len)
315307
}
316308
}

0 commit comments

Comments
 (0)