Skip to content

Commit

Permalink
Auto merge of #120741 - a1phyr:safe_buffer_advance, r=m-ou-se
Browse files Browse the repository at this point in the history
Make `io::BorrowedCursor::advance` safe

This also keeps the old `advance` method under `advance_unchecked` name.

This makes pattern like `std::io::default_read_buf` safe to write.
  • Loading branch information
bors committed Feb 17, 2024
2 parents bccb9bb + 0a42a54 commit 405b22f
Show file tree
Hide file tree
Showing 14 changed files with 38 additions and 34 deletions.
22 changes: 21 additions & 1 deletion library/core/src/io/borrowed_buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,26 @@ impl<'a> BorrowedCursor<'a> {
&mut self.buf.buf[self.buf.filled..]
}

/// Advance the cursor by asserting that `n` bytes have been filled.
///
/// After advancing, the `n` bytes are no longer accessible via the cursor and can only be
/// accessed via the underlying buffer. I.e., the buffer's filled portion grows by `n` elements
/// and its unfilled portion (and the capacity of this cursor) shrinks by `n` elements.
///
/// If less than `n` bytes initialized (by the cursor's point of view), `set_init` should be
/// called first.
///
/// # Panics
///
/// Panics if there are less than `n` bytes initialized.
#[inline]
pub fn advance(&mut self, n: usize) -> &mut Self {
assert!(self.buf.init >= self.buf.filled + n);

self.buf.filled += n;
self
}

/// Advance the cursor by asserting that `n` bytes have been filled.
///
/// After advancing, the `n` bytes are no longer accessible via the cursor and can only be
Expand All @@ -244,7 +264,7 @@ impl<'a> BorrowedCursor<'a> {
/// The caller must ensure that the first `n` bytes of the cursor have been properly
/// initialised.
#[inline]
pub unsafe fn advance(&mut self, n: usize) -> &mut Self {
pub unsafe fn advance_unchecked(&mut self, n: usize) -> &mut Self {
self.buf.filled += n;
self.buf.init = cmp::max(self.buf.init, self.buf.filled);
self
Expand Down
16 changes: 4 additions & 12 deletions library/core/tests/io/borrowed_buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ fn advance_filled() {
let buf: &mut [_] = &mut [0; 16];
let mut rbuf: BorrowedBuf<'_> = buf.into();

unsafe {
rbuf.unfilled().advance(1);
}
rbuf.unfilled().advance(1);

assert_eq!(rbuf.filled().len(), 1);
assert_eq!(rbuf.unfilled().capacity(), 15);
Expand All @@ -53,9 +51,7 @@ fn clear() {
let buf: &mut [_] = &mut [255; 16];
let mut rbuf: BorrowedBuf<'_> = buf.into();

unsafe {
rbuf.unfilled().advance(16);
}
rbuf.unfilled().advance(16);

assert_eq!(rbuf.filled().len(), 16);
assert_eq!(rbuf.unfilled().capacity(), 0);
Expand All @@ -79,9 +75,7 @@ fn set_init() {

assert_eq!(rbuf.init_len(), 8);

unsafe {
rbuf.unfilled().advance(4);
}
rbuf.unfilled().advance(4);

unsafe {
rbuf.set_init(2);
Expand Down Expand Up @@ -153,9 +147,7 @@ fn cursor_set_init() {
assert_eq!(rbuf.unfilled().uninit_mut().len(), 8);
assert_eq!(unsafe { rbuf.unfilled().as_mut() }.len(), 16);

unsafe {
rbuf.unfilled().advance(4);
}
rbuf.unfilled().advance(4);

unsafe {
rbuf.unfilled().set_init(2);
Expand Down
12 changes: 2 additions & 10 deletions library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,15 +578,7 @@ where
F: FnOnce(&mut [u8]) -> Result<usize>,
{
let n = read(cursor.ensure_init().init_mut())?;
assert!(
n <= cursor.capacity(),
"read should not return more bytes than there is capacity for in the read buffer"
);
unsafe {
// SAFETY: we initialised using `ensure_init` so there is no uninit data to advance to
// and we have checked that the read amount is not over capacity (see #120603)
cursor.advance(n);
}
cursor.advance(n);
Ok(())
}

Expand Down Expand Up @@ -2915,7 +2907,7 @@ impl<T: Read> Read for Take<T> {

unsafe {
// SAFETY: filled bytes have been filled and therefore initialized
buf.advance(filled);
buf.advance_unchecked(filled);
// SAFETY: new_init bytes of buf's unfilled buffer have been initialized
buf.set_init(new_init);
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/io/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ fn bench_take_read_buf(b: &mut test::Bencher) {

// Issue #120603
#[test]
#[should_panic = "read should not return more bytes than there is capacity for in the read buffer"]
#[should_panic]
fn read_buf_broken_read() {
struct MalformedRead;

Expand Down
2 changes: 1 addition & 1 deletion library/std/src/io/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ impl Read for Repeat {

// SAFETY: the entire unfilled portion of buf has been initialized
unsafe {
buf.advance(remaining);
buf.advance_unchecked(remaining);
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/hermit/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl Socket {
)
})?;
unsafe {
buf.advance(ret as usize);
buf.advance_unchecked(ret as usize);
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/solid/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ impl File {

// Safety: `num_bytes_read` bytes were written to the unfilled
// portion of the buffer
cursor.advance(num_bytes_read);
cursor.advance_unchecked(num_bytes_read);

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/solid/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl Socket {
netc::recv(self.as_raw_fd(), buf.as_mut().as_mut_ptr().cast(), buf.capacity(), flags)
})?;
unsafe {
buf.advance(ret as usize);
buf.advance_unchecked(ret as usize);
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl FileDesc {

// Safety: `ret` bytes were written to the initialized portion of the buffer
unsafe {
cursor.advance(ret as usize);
cursor.advance_unchecked(ret as usize);
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/unix/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl Socket {
)
})?;
unsafe {
buf.advance(ret as usize);
buf.advance_unchecked(ret as usize);
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/wasi/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl WasiFd {
}];
match wasi::fd_read(self.as_raw_fd() as wasi::Fd, &bufs) {
Ok(n) => {
buf.advance(n);
buf.advance_unchecked(n);
Ok(())
}
Err(e) => Err(err2io(e)),
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl Handle {
Ok(read) => {
// Safety: `read` bytes were written to the initialized portion of the buffer
unsafe {
cursor.advance(read);
cursor.advance_unchecked(read);
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/windows/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl Socket {
}
}
_ => {
unsafe { buf.advance(result as usize) };
unsafe { buf.advance_unchecked(result as usize) };
Ok(())
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/windows/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl AnonPipe {
Err(e) => Err(e),
Ok(n) => {
unsafe {
buf.advance(n);
buf.advance_unchecked(n);
}
Ok(())
}
Expand Down

0 comments on commit 405b22f

Please sign in to comment.