Skip to content

Commit

Permalink
Avoid short writes in LineWriter
Browse files Browse the repository at this point in the history
Also update the tests to avoid testing implementation details.
  • Loading branch information
ChrisDenton committed Dec 20, 2024
1 parent bd64bcb commit 317d00a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
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

0 comments on commit 317d00a

Please sign in to comment.