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

Fix read_buf uses in std #125404

Merged
merged 5 commits into from
Sep 28, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -349,7 +349,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 @@ -133,11 +133,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
Loading