Skip to content

Commit

Permalink
Unrolled build for rust-lang#125404
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#125404 - a1phyr:fix-read_buf-uses, r=workingjubilee

Fix `read_buf` uses in `std`

Following lib-team decision here: rust-lang#78485 (comment)

Guard against the pathological behavior of both returning an error and performing a read.
  • Loading branch information
rust-timer authored Sep 28, 2024
2 parents 150247c + d77664b commit cb494e2
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 16 deletions.
2 changes: 1 addition & 1 deletion library/std/src/io/buffered/bufreader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ impl<R: ?Sized + Read> Read for BufReader<R> {
let prev = cursor.written();

let mut rem = self.fill_buf()?;
rem.read_buf(cursor.reborrow())?;
rem.read_buf(cursor.reborrow())?; // actually never fails

self.consume(cursor.written() - prev); //slice impl of read_buf known to never unfill buf

Expand Down
4 changes: 3 additions & 1 deletion library/std/src/io/buffered/bufreader/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,13 @@ impl Buffer {
buf.set_init(self.initialized);
}

reader.read_buf(buf.unfilled())?;
let result = reader.read_buf(buf.unfilled());

self.pos = 0;
self.filled = buf.len();
self.initialized = buf.init_len();

result?;
}
Ok(self.buffer())
}
Expand Down
35 changes: 21 additions & 14 deletions library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,18 +474,28 @@ pub(crate) fn default_read_to_end<R: Read + ?Sized>(
}

let mut cursor = read_buf.unfilled();
loop {
let result = loop {
match r.read_buf(cursor.reborrow()) {
Ok(()) => break,
Err(e) if e.is_interrupted() => continue,
Err(e) => return Err(e),
// Do not stop now in case of error: we might have received both data
// and an error
res => break res,
}
}
};

let unfilled_but_initialized = cursor.init_ref().len();
let bytes_read = cursor.written();
let was_fully_initialized = read_buf.init_len() == buf_len;

// SAFETY: BorrowedBuf's invariants mean this much memory is initialized.
unsafe {
let new_len = bytes_read + buf.len();
buf.set_len(new_len);
}

// Now that all data is pushed to the vector, we can fail without data loss
result?;

if bytes_read == 0 {
return Ok(buf.len() - start_len);
}
Expand All @@ -499,12 +509,6 @@ pub(crate) fn default_read_to_end<R: Read + ?Sized>(
// store how much was initialized but not filled
initialized = unfilled_but_initialized;

// SAFETY: BorrowedBuf's invariants mean this much memory is initialized.
unsafe {
let new_len = bytes_read + buf.len();
buf.set_len(new_len);
}

// Use heuristics to determine the max read size if no initial size hint was provided
if size_hint.is_none() {
// The reader is returning short reads but it doesn't call ensure_init().
Expand Down Expand Up @@ -974,6 +978,8 @@ pub trait Read {
/// with uninitialized buffers. The new data will be appended to any existing contents of `buf`.
///
/// The default implementation delegates to `read`.
///
/// This method makes it possible to return both data and an error but it is advised against.
#[unstable(feature = "read_buf", issue = "78485")]
fn read_buf(&mut self, buf: BorrowedCursor<'_>) -> Result<()> {
default_read_buf(|b| self.read(b), buf)
Expand Down Expand Up @@ -2941,7 +2947,7 @@ impl<T: Read> Read for Take<T> {
}

let mut cursor = sliced_buf.unfilled();
self.inner.read_buf(cursor.reborrow())?;
let result = self.inner.read_buf(cursor.reborrow());

let new_init = cursor.init_ref().len();
let filled = sliced_buf.len();
Expand All @@ -2956,13 +2962,14 @@ impl<T: Read> Read for Take<T> {
}

self.limit -= filled as u64;

result
} else {
let written = buf.written();
self.inner.read_buf(buf.reborrow())?;
let result = self.inner.read_buf(buf.reborrow());
self.limit -= (buf.written() - written) as u64;
result
}

Ok(())
}
}

Expand Down
63 changes: 63 additions & 0 deletions library/std/src/io/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,69 @@ fn read_buf_full_read() {
assert_eq!(BufReader::new(FullRead).fill_buf().unwrap().len(), DEFAULT_BUF_SIZE);
}

struct DataAndErrorReader(&'static [u8]);

impl Read for DataAndErrorReader {
fn read(&mut self, _buf: &mut [u8]) -> io::Result<usize> {
panic!("We want tests to use `read_buf`")
}

fn read_buf(&mut self, buf: io::BorrowedCursor<'_>) -> io::Result<()> {
self.0.read_buf(buf).unwrap();
Err(io::Error::other("error"))
}
}

#[test]
fn read_buf_data_and_error_take() {
let mut buf = [0; 64];
let mut buf = io::BorrowedBuf::from(buf.as_mut_slice());

let mut r = DataAndErrorReader(&[4, 5, 6]).take(1);
assert!(r.read_buf(buf.unfilled()).is_err());
assert_eq!(buf.filled(), &[4]);

assert!(r.read_buf(buf.unfilled()).is_ok());
assert_eq!(buf.filled(), &[4]);
assert_eq!(r.get_ref().0, &[5, 6]);
}

#[test]
fn read_buf_data_and_error_buf() {
let mut r = BufReader::new(DataAndErrorReader(&[4, 5, 6]));

assert!(r.fill_buf().is_err());
assert_eq!(r.fill_buf().unwrap(), &[4, 5, 6]);
}

#[test]
fn read_buf_data_and_error_read_to_end() {
let mut r = DataAndErrorReader(&[4, 5, 6]);

let mut v = Vec::with_capacity(200);
assert!(r.read_to_end(&mut v).is_err());

assert_eq!(v, &[4, 5, 6]);
}

#[test]
fn read_to_end_error() {
struct ErrorReader;

impl Read for ErrorReader {
fn read(&mut self, _buf: &mut [u8]) -> io::Result<usize> {
Err(io::Error::other("error"))
}
}

let mut r = [4, 5, 6].chain(ErrorReader);

let mut v = Vec::with_capacity(200);
assert!(r.read_to_end(&mut v).is_err());

assert_eq!(v, &[4, 5, 6]);
}

#[test]
// Miri does not support signalling OOM
#[cfg_attr(miri, ignore)]
Expand Down

0 comments on commit cb494e2

Please sign in to comment.