Skip to content

Commit 5e5ce43

Browse files
committed
Rename and document the new BufReader internals
1 parent b9497be commit 5e5ce43

File tree

2 files changed

+33
-29
lines changed

2 files changed

+33
-29
lines changed

library/std/src/io/buffered/bufreader.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ impl<R: Seek> BufReader<R> {
238238
return Ok(());
239239
}
240240
} else if let Some(new_pos) = pos.checked_add(offset as u64) {
241-
if new_pos <= self.buf.cap() as u64 {
241+
if new_pos <= self.buf.filled() as u64 {
242242
self.buf.consume(offset as usize);
243243
return Ok(());
244244
}
@@ -254,7 +254,7 @@ impl<R: Read> Read for BufReader<R> {
254254
// If we don't have any buffered data and we're doing a massive read
255255
// (larger than our internal buffer), bypass our internal buffer
256256
// entirely.
257-
if self.buf.pos() == self.buf.cap() && buf.len() >= self.capacity() {
257+
if self.buf.pos() == self.buf.filled() && buf.len() >= self.capacity() {
258258
self.discard_buffer();
259259
return self.inner.read(buf);
260260
}
@@ -270,7 +270,7 @@ impl<R: Read> Read for BufReader<R> {
270270
// If we don't have any buffered data and we're doing a massive read
271271
// (larger than our internal buffer), bypass our internal buffer
272272
// entirely.
273-
if self.buf.pos() == self.buf.cap() && buf.remaining() >= self.capacity() {
273+
if self.buf.pos() == self.buf.filled() && buf.remaining() >= self.capacity() {
274274
self.discard_buffer();
275275
return self.inner.read_buf(buf);
276276
}
@@ -301,7 +301,7 @@ impl<R: Read> Read for BufReader<R> {
301301

302302
fn read_vectored(&mut self, bufs: &mut [IoSliceMut<'_>]) -> io::Result<usize> {
303303
let total_len = bufs.iter().map(|b| b.len()).sum::<usize>();
304-
if self.buf.pos() == self.buf.cap() && total_len >= self.capacity() {
304+
if self.buf.pos() == self.buf.filled() && total_len >= self.capacity() {
305305
self.discard_buffer();
306306
return self.inner.read_vectored(bufs);
307307
}
@@ -385,7 +385,7 @@ where
385385
.field("reader", &self.inner)
386386
.field(
387387
"buffer",
388-
&format_args!("{}/{}", self.buf.cap() - self.buf.pos(), self.capacity()),
388+
&format_args!("{}/{}", self.buf.filled() - self.buf.pos(), self.capacity()),
389389
)
390390
.finish()
391391
}
@@ -418,7 +418,7 @@ impl<R: Seek> Seek for BufReader<R> {
418418
fn seek(&mut self, pos: SeekFrom) -> io::Result<u64> {
419419
let result: u64;
420420
if let SeekFrom::Current(n) = pos {
421-
let remainder = (self.buf.cap() - self.buf.pos()) as i64;
421+
let remainder = (self.buf.filled() - self.buf.pos()) as i64;
422422
// it should be safe to assume that remainder fits within an i64 as the alternative
423423
// means we managed to allocate 8 exbibytes and that's absurd.
424424
// But it's not out of the realm of possibility for some weird underlying reader to
@@ -476,7 +476,7 @@ impl<R: Seek> Seek for BufReader<R> {
476476
/// }
477477
/// ```
478478
fn stream_position(&mut self) -> io::Result<u64> {
479-
let remainder = (self.buf.cap() - self.buf.pos()) as u64;
479+
let remainder = (self.buf.filled() - self.buf.pos()) as u64;
480480
self.inner.stream_position().map(|pos| {
481481
pos.checked_sub(remainder).expect(
482482
"overflow when subtracting remaining buffer size from inner stream position",

library/std/src/io/buffered/bufreader/buffer.rs

+26-22
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,39 @@
1+
///! An encapsulation of `BufReader`'s buffer management logic.
2+
///
3+
/// This module factors out the basic functionality of `BufReader` in order to protect two core
4+
/// invariants:
5+
/// * `filled` bytes of `buf` are always initialized
6+
/// * `pos` is always <= `filled`
7+
/// Since this module encapsulates the buffer management logic, we can ensure that the range
8+
/// `pos..filled` is always a valid index into the initialized region of the buffer. This means
9+
/// that user code which wants to do reads from a `BufReader` via `buffer` + `consume` can do so
10+
/// without encountering any runtime bounds checks.
111
use crate::cmp;
212
use crate::io::{self, Read, ReadBuf};
313
use crate::mem::MaybeUninit;
414

515
pub struct Buffer {
16+
// The buffer.
617
buf: Box<[MaybeUninit<u8>]>,
18+
// The current seek offset into `buf`, must always be <= `filled`.
719
pos: usize,
8-
cap: usize,
9-
init: usize,
20+
// Each call to `fill_buf` sets `filled` to indicate how many bytes at the start of `buf` are
21+
// initialized with bytes from a read.
22+
filled: usize,
1023
}
1124

1225
impl Buffer {
1326
#[inline]
1427
pub fn with_capacity(capacity: usize) -> Self {
1528
let buf = Box::new_uninit_slice(capacity);
16-
Self { buf, pos: 0, cap: 0, init: 0 }
29+
Self { buf, pos: 0, filled: 0 }
1730
}
1831

1932
#[inline]
2033
pub fn buffer(&self) -> &[u8] {
21-
// SAFETY: self.cap is always <= self.init, so self.buf[self.pos..self.cap] is always init
22-
// Additionally, both self.pos and self.cap are valid and and self.cap => self.pos, and
34+
// SAFETY: self.pos and self.cap are valid, and self.cap => self.pos, and
2335
// that region is initialized because those are all invariants of this type.
24-
unsafe { MaybeUninit::slice_assume_init_ref(&self.buf.get_unchecked(self.pos..self.cap)) }
36+
unsafe { MaybeUninit::slice_assume_init_ref(self.buf.get_unchecked(self.pos..self.filled)) }
2537
}
2638

2739
#[inline]
@@ -30,8 +42,8 @@ impl Buffer {
3042
}
3143

3244
#[inline]
33-
pub fn cap(&self) -> usize {
34-
self.cap
45+
pub fn filled(&self) -> usize {
46+
self.filled
3547
}
3648

3749
#[inline]
@@ -42,12 +54,12 @@ impl Buffer {
4254
#[inline]
4355
pub fn discard_buffer(&mut self) {
4456
self.pos = 0;
45-
self.cap = 0;
57+
self.filled = 0;
4658
}
4759

4860
#[inline]
4961
pub fn consume(&mut self, amt: usize) {
50-
self.pos = cmp::min(self.pos + amt, self.cap);
62+
self.pos = cmp::min(self.pos + amt, self.filled);
5163
}
5264

5365
#[inline]
@@ -58,25 +70,17 @@ impl Buffer {
5870
#[inline]
5971
pub fn fill_buf(&mut self, mut reader: impl Read) -> io::Result<&[u8]> {
6072
// If we've reached the end of our internal buffer then we need to fetch
61-
// some more data from the underlying reader.
73+
// some more data from the reader.
6274
// Branch using `>=` instead of the more correct `==`
6375
// to tell the compiler that the pos..cap slice is always valid.
64-
if self.pos >= self.cap {
65-
debug_assert!(self.pos == self.cap);
76+
if self.pos >= self.filled {
77+
debug_assert!(self.pos == self.filled);
6678

6779
let mut readbuf = ReadBuf::uninit(&mut self.buf);
6880

69-
// SAFETY: `self.init` is either 0 or set to `readbuf.initialized_len()`
70-
// from the last time this function was called
71-
unsafe {
72-
readbuf.assume_init(self.init);
73-
}
74-
7581
reader.read_buf(&mut readbuf)?;
7682

77-
self.cap = readbuf.filled_len();
78-
self.init = readbuf.initialized_len();
79-
83+
self.filled = readbuf.filled_len();
8084
self.pos = 0;
8185
}
8286
Ok(self.buffer())

0 commit comments

Comments
 (0)