Merge pull request #7622 from blyxxyz/flush-bufwriters

Flush `BufWriter`s, clean up error reporting

src/uu/cut/src/cut.rs

@@ -62,14 +62,6 @@ impl<'a> From<&'a OsString> for Delimiter<'a> {
     }
 }
 
-fn stdout_writer() -> Box<dyn Write> {
-    if std::io::stdout().is_terminal() {
-        Box::new(stdout())
-    } else {
-        Box::new(BufWriter::new(stdout())) as Box<dyn Write>
-    }
-}
-
 fn list_to_ranges(list: &str, complement: bool) -> Result<Vec<Range>, String> {
     if complement {
         Range::from_list(list).map(|r| uucore::ranges::complement(&r))
@@ -78,10 +70,14 @@ fn list_to_ranges(list: &str, complement: bool) -> Result<Vec<Range>, String> {
     }
 }
 
-fn cut_bytes<R: Read>(reader: R, ranges: &[Range], opts: &Options) -> UResult<()> {
+fn cut_bytes<R: Read, W: Write>(
+    reader: R,
+    out: &mut W,
+    ranges: &[Range],
+    opts: &Options,
+) -> UResult<()> {
     let newline_char = opts.line_ending.into();
     let mut buf_in = BufReader::new(reader);
-    let mut out = stdout_writer();
     let out_delim = opts.out_delimiter.unwrap_or(b"\t");
 
     let result = buf_in.for_byte_record(newline_char, |line| {
@@ -112,8 +108,9 @@ fn cut_bytes<R: Read>(reader: R, ranges: &[Range], opts: &Options) -> UResult<()
 }
 
 // Output delimiter is explicitly specified
-fn cut_fields_explicit_out_delim<R: Read, M: Matcher>(
+fn cut_fields_explicit_out_delim<R: Read, W: Write, M: Matcher>(
     reader: R,
+    out: &mut W,
     matcher: &M,
     ranges: &[Range],
     only_delimited: bool,
@@ -121,7 +118,6 @@ fn cut_fields_explicit_out_delim<R: Read, M: Matcher>(
     out_delim: &[u8],
 ) -> UResult<()> {
     let mut buf_in = BufReader::new(reader);
-    let mut out = stdout_writer();
 
     let result = buf_in.for_byte_record_with_terminator(newline_char, |line| {
         let mut fields_pos = 1;
@@ -197,15 +193,15 @@ fn cut_fields_explicit_out_delim<R: Read, M: Matcher>(
 }
 
 // Output delimiter is the same as input delimiter
-fn cut_fields_implicit_out_delim<R: Read, M: Matcher>(
+fn cut_fields_implicit_out_delim<R: Read, W: Write, M: Matcher>(
     reader: R,
+    out: &mut W,
     matcher: &M,
     ranges: &[Range],
     only_delimited: bool,
     newline_char: u8,
 ) -> UResult<()> {
     let mut buf_in = BufReader::new(reader);
-    let mut out = stdout_writer();
 
     let result = buf_in.for_byte_record_with_terminator(newline_char, |line| {
         let mut fields_pos = 1;
@@ -268,14 +264,14 @@ fn cut_fields_implicit_out_delim<R: Read, M: Matcher>(
 }
 
 // The input delimiter is identical to `newline_char`
-fn cut_fields_newline_char_delim<R: Read>(
+fn cut_fields_newline_char_delim<R: Read, W: Write>(
     reader: R,
+    out: &mut W,
     ranges: &[Range],
     newline_char: u8,
     out_delim: &[u8],
 ) -> UResult<()> {
     let buf_in = BufReader::new(reader);
-    let mut out = stdout_writer();
 
     let segments: Vec<_> = buf_in.split(newline_char).filter_map(|x| x.ok()).collect();
     let mut print_delim = false;
@@ -299,19 +295,25 @@ fn cut_fields_newline_char_delim<R: Read>(
     Ok(())
 }
 
-fn cut_fields<R: Read>(reader: R, ranges: &[Range], opts: &Options) -> UResult<()> {
+fn cut_fields<R: Read, W: Write>(
+    reader: R,
+    out: &mut W,
+    ranges: &[Range],
+    opts: &Options,
+) -> UResult<()> {
     let newline_char = opts.line_ending.into();
     let field_opts = opts.field_opts.as_ref().unwrap(); // it is safe to unwrap() here - field_opts will always be Some() for cut_fields() call
     match field_opts.delimiter {
         Delimiter::Slice(delim) if delim == [newline_char] => {
             let out_delim = opts.out_delimiter.unwrap_or(delim);
-            cut_fields_newline_char_delim(reader, ranges, newline_char, out_delim)
+            cut_fields_newline_char_delim(reader, out, ranges, newline_char, out_delim)
         }
         Delimiter::Slice(delim) => {
             let matcher = ExactMatcher::new(delim);
             match opts.out_delimiter {
                 Some(out_delim) => cut_fields_explicit_out_delim(
                     reader,
+                    out,
                     &matcher,
                     ranges,
                     field_opts.only_delimited,
@@ -320,6 +322,7 @@ fn cut_fields<R: Read>(reader: R, ranges: &[Range], opts: &Options) -> UResult<(
                 ),
                 None => cut_fields_implicit_out_delim(
                     reader,
+                    out,
                     &matcher,
                     ranges,
                     field_opts.only_delimited,
@@ -331,6 +334,7 @@ fn cut_fields<R: Read>(reader: R, ranges: &[Range], opts: &Options) -> UResult<(
             let matcher = WhitespaceMatcher {};
             cut_fields_explicit_out_delim(
                 reader,
+                out,
                 &matcher,
                 ranges,
                 field_opts.only_delimited,
@@ -348,6 +352,12 @@ fn cut_files(mut filenames: Vec<String>, mode: &Mode) {
         filenames.push("-".to_owned());
     }
 
+    let mut out: Box<dyn Write> = if std::io::stdout().is_terminal() {
+        Box::new(stdout())
+    } else {
+        Box::new(BufWriter::new(stdout())) as Box<dyn Write>
+    };
+
     for filename in &filenames {
         if filename == "-" {
             if stdin_read {
@@ -355,9 +365,9 @@ fn cut_files(mut filenames: Vec<String>, mode: &Mode) {
             }
 
             show_if_err!(match mode {
-                Mode::Bytes(ranges, opts) => cut_bytes(stdin(), ranges, opts),
+                Mode::Bytes(ranges, opts) => cut_bytes(stdin(), &mut out, ranges, opts),
-                Mode::Characters(ranges, opts) => cut_bytes(stdin(), ranges, opts),
+                Mode::Characters(ranges, opts) => cut_bytes(stdin(), &mut out, ranges, opts),
-                Mode::Fields(ranges, opts) => cut_fields(stdin(), ranges, opts),
+                Mode::Fields(ranges, opts) => cut_fields(stdin(), &mut out, ranges, opts),
             });
 
             stdin_read = true;
@@ -376,14 +386,16 @@ fn cut_files(mut filenames: Vec<String>, mode: &Mode) {
                     .and_then(|file| {
                         match &mode {
                             Mode::Bytes(ranges, opts) | Mode::Characters(ranges, opts) => {
-                                cut_bytes(file, ranges, opts)
+                                cut_bytes(file, &mut out, ranges, opts)
                             }
-                            Mode::Fields(ranges, opts) => cut_fields(file, ranges, opts),
+                            Mode::Fields(ranges, opts) => cut_fields(file, &mut out, ranges, opts),
                         }
                     })
             );
         }
     }
+
+    show_if_err!(out.flush().map_err_context(|| "write error".into()));
 }
 
 // Get delimiter and output delimiter from `-d`/`--delimiter` and `--output-delimiter` options respectively

src/uu/ptx/src/ptx.rs

@@ -633,7 +633,8 @@ fn write_traditional_output(
     let mut writer: BufWriter<Box<dyn Write>> = BufWriter::new(if output_filename == "-" {
         Box::new(stdout())
     } else {
-        let file = File::create(output_filename).map_err_context(String::new)?;
+        let file = File::create(output_filename)
+            .map_err_context(|| output_filename.maybe_quote().to_string())?;
         Box::new(file)
     });
 
@@ -673,8 +674,11 @@ fn write_traditional_output(
                 return Err(PtxError::DumbFormat.into());
             }
         };
-        writeln!(writer, "{output_line}").map_err_context(String::new)?;
+        writeln!(writer, "{output_line}").map_err_context(|| "write failed".into())?;
     }
+
+    writer.flush().map_err_context(|| "write failed".into())?;
+
     Ok(())
 }
 

src/uu/sort/src/ext_sort.rs

@@ -115,9 +115,9 @@ fn reader_writer<
                     }),
                     settings,
                     output,
-                );
+                )?;
             } else {
-                print_sorted(chunk.lines().iter(), settings, output);
+                print_sorted(chunk.lines().iter(), settings, output)?;
             }
         }
         ReadResult::SortedTwoChunks([a, b]) => {
@@ -138,9 +138,9 @@ fn reader_writer<
                         .map(|(line, _)| line),
                     settings,
                     output,
-                );
+                )?;
             } else {
-                print_sorted(merged_iter.map(|(line, _)| line), settings, output);
+                print_sorted(merged_iter.map(|(line, _)| line), settings, output)?;
             }
         }
         ReadResult::EmptyInput => {

src/uu/sort/src/merge.rs

@@ -25,7 +25,7 @@ use std::{
 };
 
 use compare::Compare;
-use uucore::error::UResult;
+use uucore::error::{FromIo, UResult};
 
 use crate::{
     GlobalSettings, Output, SortError,
@@ -278,12 +278,19 @@ impl FileMerger<'_> {
     }
 
     fn write_all_to(mut self, settings: &GlobalSettings, out: &mut impl Write) -> UResult<()> {
-        while self.write_next(settings, out) {}
+        while self
+            .write_next(settings, out)
+            .map_err_context(|| "write failed".into())?
+        {}
         drop(self.request_sender);
         self.reader_join_handle.join().unwrap()
     }
 
-    fn write_next(&mut self, settings: &GlobalSettings, out: &mut impl Write) -> bool {
+    fn write_next(
+        &mut self,
+        settings: &GlobalSettings,
+        out: &mut impl Write,
+    ) -> std::io::Result<bool> {
         if let Some(file) = self.heap.peek() {
             let prev = self.prev.replace(PreviousLine {
                 chunk: file.current_chunk.clone(),
@@ -303,12 +310,12 @@ impl FileMerger<'_> {
                             file.current_chunk.line_data(),
                         );
                         if cmp == Ordering::Equal {
-                            return;
+                            return Ok(());
                         }
                     }
                 }
-                current_line.print(out, settings);
+                current_line.print(out, settings)
-            });
+            })?;
 
             let was_last_line_for_file = file.current_chunk.lines().len() == file.line_idx + 1;
 
@@ -335,7 +342,7 @@ impl FileMerger<'_> {
                 }
             }
         }
-        !self.heap.is_empty()
+        Ok(!self.heap.is_empty())
     }
 }
 

src/uu/sort/src/sort.rs

@@ -42,7 +42,7 @@ use std::str::Utf8Error;
 use thiserror::Error;
 use unicode_width::UnicodeWidthStr;
 use uucore::display::Quotable;
-use uucore::error::strip_errno;
+use uucore::error::{FromIo, strip_errno};
 use uucore::error::{UError, UResult, USimpleError, UUsageError, set_exit_code};
 use uucore::line_ending::LineEnding;
 use uucore::parser::parse_size::{ParseSizeError, Parser};
@@ -488,13 +488,14 @@ impl<'a> Line<'a> {
         Self { line, index }
     }
 
-    fn print(&self, writer: &mut impl Write, settings: &GlobalSettings) {
+    fn print(&self, writer: &mut impl Write, settings: &GlobalSettings) -> std::io::Result<()> {
         if settings.debug {
-            self.print_debug(settings, writer).unwrap();
+            self.print_debug(settings, writer)?;
         } else {
-            writer.write_all(self.line.as_bytes()).unwrap();
+            writer.write_all(self.line.as_bytes())?;
-            writer.write_all(&[settings.line_ending.into()]).unwrap();
+            writer.write_all(&[settings.line_ending.into()])?;
         }
+        Ok(())
     }
 
     /// Writes indicators for the selections this line matched. The original line content is NOT expected
@@ -1852,11 +1853,19 @@ fn print_sorted<'a, T: Iterator<Item = &'a Line<'a>>>(
     iter: T,
     settings: &GlobalSettings,
     output: Output,
-) {
+) -> UResult<()> {
+    let output_name = output
+        .as_output_name()
+        .unwrap_or("standard output")
+        .to_owned();
+    let ctx = || format!("write failed: {}", output_name.maybe_quote());
+
     let mut writer = output.into_write();
     for line in iter {
-        line.print(&mut writer, settings);
+        line.print(&mut writer, settings).map_err_context(ctx)?;
     }
+    writer.flush().map_err_context(ctx)?;
+    Ok(())
 }
 
 fn open(path: impl AsRef<OsStr>) -> UResult<Box<dyn Read + Send>> {

src/uu/tac/src/tac.rs

@@ -164,6 +164,7 @@ fn buffer_tac_regex(
     // After the loop terminates, write whatever bytes are remaining at
     // the beginning of the buffer.
     out.write_all(&data[0..following_line_start])?;
+    out.flush()?;
     Ok(())
 }
 
@@ -215,6 +216,7 @@ fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()>
     // After the loop terminates, write whatever bytes are remaining at
     // the beginning of the buffer.
     out.write_all(&data[0..following_line_start])?;
+    out.flush()?;
     Ok(())
 }
 

src/uu/tail/src/chunks.rs

@@ -319,7 +319,7 @@ impl BytesChunkBuffer {
         Ok(())
     }
 
-    pub fn print(&self, mut writer: impl Write) -> UResult<()> {
+    pub fn print(&self, writer: &mut impl Write) -> UResult<()> {
         for chunk in &self.chunks {
             writer.write_all(chunk.get_buffer())?;
         }

src/uu/tail/src/follow/files.rs

@@ -12,7 +12,7 @@ use crate::text;
 use std::collections::HashMap;
 use std::collections::hash_map::Keys;
 use std::fs::{File, Metadata};
-use std::io::{BufRead, BufReader, BufWriter, stdout};
+use std::io::{BufRead, BufReader, BufWriter, Write, stdout};
 use std::path::{Path, PathBuf};
 use uucore::error::UResult;
 
@@ -146,9 +146,9 @@ impl FileHandling {
                 self.header_printer.print(display_name.as_str());
             }
 
-            let stdout = stdout();
+            let mut writer = BufWriter::new(stdout().lock());
-            let writer = BufWriter::new(stdout.lock());
+            chunks.print(&mut writer)?;
-            chunks.print(writer)?;
+            writer.flush()?;
 
             self.last.replace(path.to_owned());
             self.update_metadata(path, None);

src/uu/tail/src/tail.rs

@@ -410,8 +410,7 @@ fn bounded_tail(file: &mut File, settings: &Settings) {
 }
 
 fn unbounded_tail<T: Read>(reader: &mut BufReader<T>, settings: &Settings) -> UResult<()> {
-    let stdout = stdout();
+    let mut writer = BufWriter::new(stdout().lock());
-    let mut writer = BufWriter::new(stdout.lock());
     match &settings.mode {
         FilterMode::Lines(Signum::Negative(count), sep) => {
             let mut chunks = chunks::LinesChunkBuffer::new(*sep, *count);
@@ -470,6 +469,7 @@ fn unbounded_tail<T: Read>(reader: &mut BufReader<T>, settings: &Settings) -> UR
         }
         _ => {}
     }
+    writer.flush()?;
     Ok(())
 }
 

src/uu/tr/src/operation.rs

@@ -23,7 +23,7 @@ use std::{
     io::{BufRead, Write},
     ops::Not,
 };
-use uucore::error::{UError, UResult, USimpleError};
+use uucore::error::{FromIo, UError, UResult};
 use uucore::show_warning;
 
 #[derive(Debug, Clone)]
@@ -669,12 +669,9 @@ where
         let filtered = buf.iter().filter_map(|&c| translator.translate(c));
         output_buf.extend(filtered);
 
-        if let Err(e) = output.write_all(&output_buf) {
+        output
-            return Err(USimpleError::new(
+            .write_all(&output_buf)
-                1,
+            .map_err_context(|| "write error".into())?;
-                format!("{}: write error: {}", uucore::util_name(), e),
-            ));
-        }
 
         buf.clear();
         output_buf.clear();

src/uu/tr/src/tr.rs

@@ -14,9 +14,9 @@ use operation::{
     Sequence, SqueezeOperation, SymbolTranslator, TranslateOperation, translate_input,
 };
 use std::ffi::OsString;
-use std::io::{BufWriter, stdin, stdout};
+use std::io::{BufWriter, Write, stdin, stdout};
 use uucore::display::Quotable;
-use uucore::error::{UResult, USimpleError, UUsageError};
+use uucore::error::{FromIo, UResult, USimpleError, UUsageError};
 use uucore::fs::is_stdin_directory;
 use uucore::{format_usage, help_about, help_section, help_usage, os_str_as_bytes, show};
 
@@ -110,9 +110,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     let stdin = stdin();
     let mut locked_stdin = stdin.lock();
-    let stdout = stdout();
+    let mut buffered_stdout = BufWriter::new(stdout().lock());
-    let locked_stdout = stdout.lock();
-    let mut buffered_stdout = BufWriter::new(locked_stdout);
 
     // According to the man page: translating only happens if deleting or if a second set is given
     let translating = !delete_flag && sets.len() > 1;
@@ -155,6 +153,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         let op = TranslateOperation::new(set1, set2)?;
         translate_input(&mut locked_stdin, &mut buffered_stdout, op)?;
     }
+
+    buffered_stdout
+        .flush()
+        .map_err_context(|| "write error".into())?;
+
     Ok(())
 }
 

src/uu/uniq/src/uniq.rs

@@ -110,6 +110,7 @@ impl Uniq {
         {
             write_line_terminator!(writer, line_terminator)?;
         }
+        writer.flush().map_err_context(|| "write error".into())?;
         Ok(())
     }
 
@@ -226,7 +227,7 @@ impl Uniq {
         } else {
             writer.write_all(line)
         }
-        .map_err_context(|| "Failed to write line".to_string())?;
+        .map_err_context(|| "write error".to_string())?;
 
         write_line_terminator!(writer, line_terminator)
     }

tests/by-util/test_cut.rs

@@ -375,3 +375,15 @@ fn test_output_delimiter_with_adjacent_ranges() {
         .succeeds()
         .stdout_only("ab:cd\n");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .arg("-d=")
+        .arg("-f1")
+        .pipe_in("key=value")
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("cut: write error: No space left on device\n");
+}

tests/by-util/test_ptx.rs

@@ -163,3 +163,14 @@ fn test_format() {
         .succeeds()
         .stdout_only("\\xx {}{}{a}{}{}\n");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .arg("-G")
+        .pipe_in("hello")
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("ptx: write failed: No space left on device\n");
+}

tests/by-util/test_sort.rs

@@ -1335,3 +1335,13 @@ fn test_human_blocks_r_and_q() {
 fn test_args_check_conflict() {
     new_ucmd!().arg("-c").arg("-C").fails();
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .pipe_in("hello")
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("sort: write failed: 'standard output': No space left on device\n");
+}

tests/by-util/test_tac.rs

@@ -309,3 +309,13 @@ fn test_regex_before() {
         //          |--------||----||---|
         .stdout_is("+---+c+d-e+--++b+-+a+");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .pipe_in("hello")
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("tac: failed to write to stdout: No space left on device (os error 28)\n");
+}

tests/by-util/test_tail.rs

@@ -4843,3 +4843,13 @@ fn test_child_when_run_with_stderr_to_stdout() {
         .fails()
         .stdout_only(expected_stdout);
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .pipe_in("hello")
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("tail: No space left on device\n");
+}

tests/by-util/test_tr.rs

@@ -1545,3 +1545,14 @@ fn test_non_digit_repeat() {
         .fails()
         .stderr_only("tr: invalid repeat count 'c' in [c*n] construct\n");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .pipe_in("hello")
+        .args(&["e", "a"])
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("tr: write error: No space left on device\n");
+}

tests/by-util/test_uniq.rs

@@ -1181,3 +1181,14 @@ fn test_stdin_w1_multibyte() {
         .succeeds()
         .stdout_is("à\ná\n");
 }
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_failed_write_is_reported() {
+    new_ucmd!()
+        .pipe_in("hello")
+        .args(&["-z"])
+        .set_stdout(std::fs::File::create("/dev/full").unwrap())
+        .fails()
+        .stderr_is("uniq: write error: No space left on device\n");
+}