From 18d2ee7c4dbef1961ce921cbf460285ac85bc66a Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 31 Dec 2025 13:02:17 +0900 Subject: [PATCH 1/4] feat: add write error handling for cat output - Introduce WriteError variant in CatError enum to specifically handle stdout write failures - Add map_write_err helper to wrap io::Result into CatResult with WriteError - Update cat_files to catch WriteError early and report using localized message - Wrap write_all and flush calls in write_fast and write_lines with map_write_err - Add localized error messages for "write error" in en-US and fr-FR locales This improves error reporting for I/O issues like broken pipes during output. --- src/uu/cat/locales/en-US.ftl | 1 + src/uu/cat/locales/fr-FR.ftl | 1 + src/uu/cat/src/cat.rs | 93 ++++++++++++++++++++++-------------- src/uu/cat/src/splice.rs | 9 +++- tests/by-util/test_cat.rs | 2 +- 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/uu/cat/locales/en-US.ftl b/src/uu/cat/locales/en-US.ftl index bf81d6d7ffb..16cda4f2e5a 100644 --- a/src/uu/cat/locales/en-US.ftl +++ b/src/uu/cat/locales/en-US.ftl @@ -20,3 +20,4 @@ cat-error-is-directory = Is a directory cat-error-input-file-is-output-file = input file is output file cat-error-too-many-symbolic-links = Too many levels of symbolic links cat-error-no-such-device-or-address = No such device or address +cat-error-write-error = write error diff --git a/src/uu/cat/locales/fr-FR.ftl b/src/uu/cat/locales/fr-FR.ftl index 2316544ce03..fd348e0d89a 100644 --- a/src/uu/cat/locales/fr-FR.ftl +++ b/src/uu/cat/locales/fr-FR.ftl @@ -20,3 +20,4 @@ cat-error-is-directory = Est un répertoire cat-error-input-file-is-output-file = le fichier d'entrée est le fichier de sortie cat-error-too-many-symbolic-links = Trop de niveaux de liens symboliques cat-error-no-such-device-or-address = Aucun appareil ou adresse de ce type +cat-error-write-error = erreur d'écriture diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 3497429d2de..e4872e2c401 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -19,7 +19,7 @@ use std::os::fd::AsFd; use std::os::unix::fs::FileTypeExt; use thiserror::Error; use uucore::display::Quotable; -use uucore::error::UResult; +use uucore::error::{UIoError, UResult}; #[cfg(not(target_os = "windows"))] use uucore::libc; use uucore::translate; @@ -86,6 +86,9 @@ enum CatError { /// Wrapper around `io::Error` #[error("{0}")] Io(#[from] io::Error), + /// Wrapper around `io::Error` for stdout writes + #[error("{0}")] + WriteError(io::Error), /// Wrapper around `nix::Error` #[cfg(any(target_os = "linux", target_os = "android"))] #[error("{0}")] @@ -109,6 +112,11 @@ enum CatError { type CatResult = Result; +#[inline] +fn map_write_err(res: io::Result) -> CatResult { + res.map_err(CatError::WriteError) +} + #[derive(PartialEq)] enum NumberingMode { None, @@ -416,15 +424,30 @@ fn cat_files(files: &[OsString], options: &OutputOptions) -> UResult<()> { one_blank_kept: false, }; let mut error_messages: Vec = Vec::new(); + let mut write_error: Option = None; for path in files { - if let Err(err) = cat_path(path, options, &mut state) { - error_messages.push(format!("{}: {err}", path.maybe_quote())); + match cat_path(path, options, &mut state) { + Ok(()) => {} + Err(CatError::WriteError(err)) => { + write_error = Some(err); + break; + } + Err(err) => { + error_messages.push(format!("{}: {err}", path.maybe_quote())); + } } } if state.skipped_carriage_return { print!("\r"); } + if let Some(err) = write_error { + let message = UIoError::from(err); + error_messages.push(format!( + "{}: {message}", + translate!("cat-error-write-error") + )); + } if error_messages.is_empty() { Ok(()) } else { @@ -505,9 +528,11 @@ fn write_fast(handle: &mut InputHandle) -> CatResult<()> { if n == 0 { break; } - stdout_lock - .write_all(&buf[..n]) - .inspect_err(handle_broken_pipe)?; + map_write_err( + stdout_lock + .write_all(&buf[..n]) + .inspect_err(handle_broken_pipe), + )?; } Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e.into()), @@ -519,7 +544,7 @@ fn write_fast(handle: &mut InputHandle) -> CatResult<()> { // that will succeed, data pushed through splice will be output before // the data buffered in stdout.lock. Therefore additional explicit flush // is required here. - stdout_lock.flush().inspect_err(handle_broken_pipe)?; + map_write_err(stdout_lock.flush().inspect_err(handle_broken_pipe))?; Ok(()) } @@ -554,13 +579,13 @@ fn write_lines( continue; } if state.skipped_carriage_return { - writer.write_all(b"\r")?; + map_write_err(writer.write_all(b"\r"))?; state.skipped_carriage_return = false; state.at_line_start = false; } state.one_blank_kept = false; if state.at_line_start && options.number != NumberingMode::None { - state.line_number.write(&mut writer)?; + map_write_err(state.line_number.write(&mut writer))?; state.line_number.increment(); } @@ -593,7 +618,7 @@ fn write_lines( // and not be buffered internally to the `cat` process. // Hence it's necessary to flush our buffer before every time we could potentially block // on a `std::io::Read::read` call. - writer.flush().inspect_err(handle_broken_pipe)?; + map_write_err(writer.flush().inspect_err(handle_broken_pipe))?; } Ok(()) @@ -608,9 +633,9 @@ fn write_new_line( ) -> CatResult<()> { if state.skipped_carriage_return { if options.show_ends { - writer.write_all(b"^M")?; + map_write_err(writer.write_all(b"^M"))?; } else { - writer.write_all(b"\r")?; + map_write_err(writer.write_all(b"\r"))?; } state.skipped_carriage_return = false; @@ -620,7 +645,7 @@ fn write_new_line( if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept { state.one_blank_kept = true; if state.at_line_start && options.number == NumberingMode::All { - state.line_number.write(writer)?; + map_write_err(state.line_number.write(writer))?; state.line_number.increment(); } write_end_of_line(writer, options.end_of_line().as_bytes(), is_interactive)?; @@ -628,11 +653,7 @@ fn write_new_line( Ok(()) } -fn write_end( - writer: &mut W, - in_buf: &[u8], - options: &OutputOptions, -) -> io::Result { +fn write_end(writer: &mut W, in_buf: &[u8], options: &OutputOptions) -> CatResult { if options.show_nonprint { write_nonprint_to_end(in_buf, writer, options.tab().as_bytes()) } else if options.show_tabs { @@ -648,21 +669,21 @@ fn write_end( // however, write_nonprint_to_end doesn't need to stop at \r because it will always write \r as ^M. // Return the number of written symbols -fn write_to_end(in_buf: &[u8], writer: &mut W) -> io::Result { +fn write_to_end(in_buf: &[u8], writer: &mut W) -> CatResult { // using memchr2 significantly improves performances match memchr2(b'\n', b'\r', in_buf) { Some(p) => { - writer.write_all(&in_buf[..p])?; + map_write_err(writer.write_all(&in_buf[..p]))?; Ok(p) } None => { - writer.write_all(in_buf)?; + map_write_err(writer.write_all(in_buf))?; Ok(in_buf.len()) } } } -fn write_tab_to_end(mut in_buf: &[u8], writer: &mut W) -> io::Result { +fn write_tab_to_end(mut in_buf: &[u8], writer: &mut W) -> CatResult { let mut count = 0; loop { match in_buf @@ -670,9 +691,9 @@ fn write_tab_to_end(mut in_buf: &[u8], writer: &mut W) -> io::Result { - writer.write_all(&in_buf[..p])?; + map_write_err(writer.write_all(&in_buf[..p]))?; if in_buf[p] == b'\t' { - writer.write_all(b"^I")?; + map_write_err(writer.write_all(b"^I"))?; in_buf = &in_buf[p + 1..]; count += p + 1; } else { @@ -681,14 +702,14 @@ fn write_tab_to_end(mut in_buf: &[u8], writer: &mut W) -> io::Result { - writer.write_all(in_buf)?; + map_write_err(writer.write_all(in_buf))?; return Ok(in_buf.len() + count); } } } } -fn write_nonprint_to_end(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> io::Result { +fn write_nonprint_to_end(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> CatResult { let mut count = 0; for byte in in_buf.iter().copied() { @@ -696,14 +717,14 @@ fn write_nonprint_to_end(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> break; } match byte { - 9 => writer.write_all(tab), - 0..=8 | 10..=31 => writer.write_all(&[b'^', byte + 64]), - 32..=126 => writer.write_all(&[byte]), - 127 => writer.write_all(b"^?"), - 128..=159 => writer.write_all(&[b'M', b'-', b'^', byte - 64]), - 160..=254 => writer.write_all(&[b'M', b'-', byte - 128]), - _ => writer.write_all(b"M-^?"), - }?; + 9 => map_write_err(writer.write_all(tab))?, + 0..=8 | 10..=31 => map_write_err(writer.write_all(&[b'^', byte + 64]))?, + 32..=126 => map_write_err(writer.write_all(&[byte]))?, + 127 => map_write_err(writer.write_all(b"^?"))?, + 128..=159 => map_write_err(writer.write_all(&[b'M', b'-', b'^', byte - 64]))?, + 160..=254 => map_write_err(writer.write_all(&[b'M', b'-', byte - 128]))?, + _ => map_write_err(writer.write_all(b"M-^?"))?, + }; count += 1; } Ok(count) @@ -714,9 +735,9 @@ fn write_end_of_line( end_of_line: &[u8], is_interactive: bool, ) -> CatResult<()> { - writer.write_all(end_of_line)?; + map_write_err(writer.write_all(end_of_line))?; if is_interactive { - writer.flush().inspect_err(handle_broken_pipe)?; + map_write_err(writer.flush().inspect_err(handle_broken_pipe))?; } Ok(()) } diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs index ca5265d2bf8..cca0f09221e 100644 --- a/src/uu/cat/src/splice.rs +++ b/src/uu/cat/src/splice.rs @@ -2,9 +2,10 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use super::{CatResult, FdReadable, InputHandle}; +use super::{CatError, CatResult, FdReadable, InputHandle}; use nix::unistd; +use std::io; use std::os::{fd::AsFd, unix::io::AsRawFd}; use uucore::pipes::{pipe, splice, splice_exact}; @@ -38,7 +39,11 @@ pub(super) fn write_fast_using_splice( // we can recover by copying the data that we have from the // intermediate pipe to stdout using normal read/write. Then // we tell the caller to fall back. - copy_exact(&pipe_rd, write_fd, n)?; + if let Err(err) = copy_exact(&pipe_rd, write_fd, n) { + return Err(CatError::WriteError(io::Error::from_raw_os_error( + err as i32, + ))); + } return Ok(true); } } diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index 33796f3ae41..dd047d16fc0 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -223,7 +223,7 @@ fn test_piped_to_dev_full() { .pipe_in_fixture("alpha.txt") .ignore_stdin_write_error() .fails() - .stderr_contains("No space left on device"); + .stderr_contains("write error: No space left on device"); } } } From 08946a2d4b52c78b5c8bc1dfedf055768dd1b7bc Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 31 Dec 2025 13:09:36 +0900 Subject: [PATCH 2/4] fix(uu/cat): remove unnecessary semicolon after match statement in write_nonprint_to_end Removed a trailing semicolon following the match arms in the `write_nonprint_to_end` function, which was likely causing a compiler warning or style issue in Rust. This ensures cleaner code without altering functionality. --- src/uu/cat/src/cat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index e4872e2c401..9ddd7ed0567 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -724,7 +724,7 @@ fn write_nonprint_to_end(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> 128..=159 => map_write_err(writer.write_all(&[b'M', b'-', b'^', byte - 64]))?, 160..=254 => map_write_err(writer.write_all(&[b'M', b'-', byte - 128]))?, _ => map_write_err(writer.write_all(b"M-^?"))?, - }; + } count += 1; } Ok(count) From ec2deaa35517b7e97b60e447a59ba0c379dffb70 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 31 Dec 2025 13:18:25 +0900 Subject: [PATCH 3/4] test: add test for piping to /dev/full from /dev/stdin - Adds a new test case to verify that `cat /dev/stdin` properly handles writing to a full device (/dev/full) and outputs the correct error message "No space left on device" without the raw errno code. This improves test coverage for error handling on Unix-like systems. --- tests/by-util/test_cat.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index dd047d16fc0..8fc22ee5045 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -228,6 +228,21 @@ fn test_piped_to_dev_full() { } } +#[test] +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +fn test_piped_to_dev_full_from_dev_stdin() { + let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap(); + + new_ucmd!() + .arg("/dev/stdin") + .set_stdout(dev_full) + .pipe_in("1") + .ignore_stdin_write_error() + .fails() + .stderr_contains("write error: No space left on device") + .stderr_does_not_contain("ENOSPC"); +} + #[test] fn test_directory() { let s = TestScenario::new(util_name!()); From f46569ed5839e030b297f19b358732af3b02de5b Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 31 Dec 2025 13:22:06 +0900 Subject: [PATCH 4/4] chore(test): add ENOSPC to spell-checker ignore list in cat tests - Prevents spell-checker from flagging ENOSPC as a misspelling, as it's a valid errno constant (e.g., "No space left on device") used in the test code. --- tests/by-util/test_cat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index 8fc22ee5045..b4d0736408c 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore NOFILE nonewline cmdline +// spell-checker:ignore NOFILE nonewline cmdline　ENOSPC #[cfg(any(target_os = "linux", target_os = "android"))] use rlimit::Resource;