From d456e905967f8f837d00271df453147495f056b4 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Mon, 31 Mar 2025 13:13:23 +0200 Subject: [PATCH] sort: Flush `BufWriter`, don't panic on write errors --- src/uu/sort/src/ext_sort.rs | 8 ++++---- src/uu/sort/src/merge.rs | 21 ++++++++++++++------- src/uu/sort/src/sort.rs | 23 ++++++++++++++++------- tests/by-util/test_sort.rs | 10 ++++++++++ 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/uu/sort/src/ext_sort.rs b/src/uu/sort/src/ext_sort.rs index cc042a2de..7a65fea5b 100644 --- a/src/uu/sort/src/ext_sort.rs +++ b/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 => { diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index 822eeb1d6..a59474021 100644 --- a/src/uu/sort/src/merge.rs +++ b/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 { 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()) } } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 31dc81751..51b5c2960 100644 --- a/src/uu/sort/src/sort.rs +++ b/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::parse_size::{ParseSizeError, Parser}; @@ -481,13 +481,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(&[settings.line_ending.into()]).unwrap(); + writer.write_all(self.line.as_bytes())?; + writer.write_all(&[settings.line_ending.into()])?; } + Ok(()) } /// Writes indicators for the selections this line matched. The original line content is NOT expected @@ -1827,11 +1828,19 @@ fn print_sorted<'a, T: Iterator>>( 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) -> UResult> { diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 1f3b2a8b1..57cca0aa4 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1336,3 +1336,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"); +}