Skip to content

Commit fdb43ef

Browse files
committed
Avoid short writes in LineWriter
Also update the tests to avoid testing implementation details.
1 parent 54dcff1 commit fdb43ef

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

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

+8-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,14 @@ impl<'a, W: ?Sized + Write> Write for LineWriterShim<'a, W> {
119119
// the buffer?
120120
// - If not, scan for the last newline that *does* fit in the buffer
121121
let tail = if flushed >= newline_idx {
122-
&buf[flushed..]
122+
let tail = &buf[flushed..];
123+
// Avoid unnecessary short writes by not splitting the remaining
124+
// bytes if they're larger than the buffer.
125+
// They can be written in full by the next call to write.
126+
if tail.len() >= self.buffer.capacity() {
127+
return Ok(flushed);
128+
}
129+
tail
123130
} else if newline_idx - flushed <= self.buffer.capacity() {
124131
&buf[flushed..newline_idx]
125132
} else {

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

+13-5
Original file line numberDiff line numberDiff line change
@@ -847,22 +847,30 @@ fn long_line_flushed() {
847847
}
848848

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

859-
// Assert that Line 1\n is flushed, and 01234 is buffered
860-
assert_eq!(writer.write(b"Line 1\n0123456789").unwrap(), 12);
858+
// Assert that Line 1\n is flushed and the long tail isn't.
859+
let bytes = b"Line 1\n0123456789";
860+
writer.write(bytes).unwrap();
861861
assert_eq!(&writer.get_ref().buffer, b"Line 1\n");
862+
}
863+
864+
// Test that appending to a full buffer emits a single write, flushing the buffer.
865+
#[test]
866+
fn line_full_buffer_flushed() {
867+
let writer = ProgrammableSink::default();
868+
let mut writer = LineWriter::with_capacity(5, writer);
869+
assert_eq!(writer.write(b"01234").unwrap(), 5);
862870

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

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

0 commit comments

Comments
 (0)