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

Avoid short writes in LineWriter #134620

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
9 changes: 8 additions & 1 deletion library/std/src/io/buffered/linewritershim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,14 @@ impl<'a, W: ?Sized + Write> Write for LineWriterShim<'a, W> {
// the buffer?
// - If not, scan for the last newline that *does* fit in the buffer
let tail = if flushed >= newline_idx {
&buf[flushed..]
let tail = &buf[flushed..];
// Avoid unnecessary short writes by not splitting the remaining
// bytes if they're larger than the buffer.
// They can be written in full by the next call to write.
if tail.len() >= self.buffer.capacity() {
return Ok(flushed);
}
tail
} else if newline_idx - flushed <= self.buffer.capacity() {
&buf[flushed..newline_idx]
} else {
Expand Down
18 changes: 13 additions & 5 deletions library/std/src/io/buffered/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,22 +847,30 @@ fn long_line_flushed() {
}

/// Test that, given a very long partial line *after* successfully
/// flushing a complete line, the very long partial line is buffered
/// unconditionally, and no additional writes take place. This assures
/// flushing a complete line, no additional writes take place. This assures
/// the property that `write` should make at-most-one attempt to write
/// new data.
#[test]
fn line_long_tail_not_flushed() {
let writer = ProgrammableSink::default();
let mut writer = LineWriter::with_capacity(5, writer);

// Assert that Line 1\n is flushed, and 01234 is buffered
assert_eq!(writer.write(b"Line 1\n0123456789").unwrap(), 12);
// Assert that Line 1\n is flushed and the long tail isn't.
let bytes = b"Line 1\n0123456789";
writer.write(bytes).unwrap();
assert_eq!(&writer.get_ref().buffer, b"Line 1\n");
}

// Test that appending to a full buffer emits a single write, flushing the buffer.
#[test]
fn line_full_buffer_flushed() {
let writer = ProgrammableSink::default();
let mut writer = LineWriter::with_capacity(5, writer);
assert_eq!(writer.write(b"01234").unwrap(), 5);

// Because the buffer is full, this subsequent write will flush it
assert_eq!(writer.write(b"5").unwrap(), 1);
assert_eq!(&writer.get_ref().buffer, b"Line 1\n01234");
assert_eq!(&writer.get_ref().buffer, b"01234");
}

/// Test that, if an attempt to pre-flush buffered data returns Ok(0),
Expand Down
Loading