Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make io::BorrowedCursor::advance safe #120741

Merged
merged 1 commit into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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