From 1a60d5fde9b3eb26580563460def44ffc7f21c97 Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Sat, 8 Aug 2026 20:42:13 +0600 Subject: [PATCH 1/6] cat: diagnose splice errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The splice() fast path swallowed every error as a silent read/write fallback, so a splice failure after some bytes were copied (e.g. an EIO injected by strace in tests/cat/splice.sh) produced no diagnostic. Once any data has been copied, treat a splice error as fatal and report it as "cat: : …" on the read side or "cat: write error: …" on the write side; failures before the first byte still fall back to read/write. Write errors elsewhere now also omit the input file name. Fixes tests/cat/splice.sh (backported in util/fetch-gnu.sh). Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 91 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 5 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 25340f8db14..30e3480a5e8 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -80,6 +80,10 @@ enum CatError { /// Wrapper around `io::Error` #[error("{}", strip_errno(.0))] Io(#[from] io::Error), + /// A write error to the output; unlike [`Self::Io`] it is reported + /// without the input filename. + #[error("write error: {}", strip_errno(.0))] + WriteIo(io::Error), /// Unknown file type; it's not a regular file, socket, etc. #[error("{}", translate!("cat-error-unknown-filetype", "ft_debug" => .ft_debug))] UnknownFiletype { @@ -99,6 +103,26 @@ enum CatError { type CatResult = Result; +impl CatError { + /// Compose the diagnostic for the input at `path`. + /// + /// Write errors are reported without the filename, + /// while input errors name the file. + fn display_message(&self, path: &OsString) -> String { + if matches!(self, Self::WriteIo(_)) { + format!("{self}") + } else { + format!("{}: {self}", path.maybe_quote()) + } + } +} + +#[cfg(any(unix, target_os = "wasi"))] +impl From for CatError { + fn from(value: rustix::io::Errno) -> Self { + Self::Io(value.into()) + } +} #[derive(PartialEq)] enum NumberingMode { None, @@ -403,7 +427,7 @@ where for path in files { if let Err(err) = cat_path(path, options, &mut state) { - error_messages.push(format!("{}: {err}", path.maybe_quote())); + error_messages.push(err.display_message(path)); } } if state.skipped_carriage_return { @@ -470,11 +494,9 @@ fn get_input_type(path: &OsString) -> CatResult { /// simple memory copy. fn print_fast(handle: &mut InputHandle) -> CatResult<()> { let stdout = io::stdout(); - #[cfg(any(target_os = "linux", target_os = "android"))] - let mut stdout = stdout; // Try to use the splice() system call for faster writing. If it works, we're done. #[cfg(any(target_os = "linux", target_os = "android"))] - if uucore::pipes::splice_unbounded_auto(&handle.reader, &mut stdout)?.is_ok() { + if splice_cat(handle, &stdout)? { return Ok(()); } @@ -483,6 +505,64 @@ fn print_fast(handle: &mut InputHandle) -> CatResult<()> { print_unbuffered(handle, stdout) } +/// Copy the input to `stdout` with the `splice(2)` syscall. +/// +/// Returns `Ok(true)` if the whole input was copied, `Ok(false)` if the +/// caller should fall back on ordinary read/write, and `Err(..)` for a +/// diagnosed error. Once any bytes have been spliced, a splice error is +/// fatal and is reported either as an input error (naming the file) or as +/// a "write error"; before that, splice errors just select read/write. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) -> CatResult { + // Create a broker pipe, since splice(2) needs a pipe on one side and we + // want to distinguish read errors from write errors. If the pipe cannot + // be created (e.g. file descriptor exhaustion), fall back on read/write. + let Ok((pipe_rd, pipe_wr)) = uucore::pipes::pipe::() else { + return Ok(false); + }; + let input = handle.reader.as_fd(); + let output = stdout.as_fd(); + + let mut some_copied = false; + loop { + match uucore::pipes::splice(&input, &pipe_wr, uucore::pipes::MAX_ROOTLESS_PIPE_SIZE) { + // End of input: splice handled the whole file. + Ok(0) => return Ok(true), + Ok(bytes_read) => { + let mut remaining = bytes_read; + while remaining > 0 { + match uucore::pipes::splice(&pipe_rd, &output, remaining) { + // No progress; stop splicing. + Ok(0) => return Ok(some_copied), + Ok(bytes_written) => { + some_copied = true; + remaining -= bytes_written; + } + Err(err) if some_copied => return Err(CatError::WriteIo(err.into())), + Err(_) => { + // stdout cannot take splice data: drain the + // intermediate pipe with read/write, then let + // the caller continue likewise. + let mut drain = Vec::with_capacity(remaining); + let _ = pipe_rd + .take(remaining as u64) + .read_to_end(&mut drain) + .map_err(CatError::Io)?; + uucore::io::RawWriter(&output) + .write_all(&drain) + .inspect_err(handle_broken_pipe) + .map_err(CatError::WriteIo)?; + return Ok(false); + } + } + } + } + Err(err) if some_copied => return Err(CatError::Io(err.into())), + Err(_) => return Ok(false), + } + } +} + #[cfg_attr(any(target_os = "linux", target_os = "android"), inline(never))] // splice fast-path does not require this allocation fn print_unbuffered( handle: &mut InputHandle, @@ -499,7 +579,8 @@ fn print_unbuffered( Ok(n) => { stdout .write_all(&buf[..n]) - .inspect_err(handle_broken_pipe)?; + .inspect_err(handle_broken_pipe) + .map_err(CatError::WriteIo)?; // cannot use rustix::io on Windows // really bad workaround for unbuffered write #[cfg(not(any(unix, target_os = "wasi")))] From 8e9faa5b07c25053991e075dea1455eb906ec72f Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Sat, 8 Aug 2026 21:34:36 +0600 Subject: [PATCH 2/6] cat: use translate!() macro for error and add a test for splice-test Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 2 +- tests/by-util/test_cat.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 30e3480a5e8..8b4ddd8b383 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -82,7 +82,7 @@ enum CatError { Io(#[from] io::Error), /// A write error to the output; unlike [`Self::Io`] it is reported /// without the input filename. - #[error("write error: {}", strip_errno(.0))] + #[error("{}: {}", translate!("common-write-error"), strip_errno(.0))] WriteIo(io::Error), /// Unknown file type; it's not a regular file, socket, etc. #[error("{}", translate!("cat-error-unknown-filetype", "ft_debug" => .ft_debug))] diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index e0baf5095b0..e6f744475ba 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -856,6 +856,24 @@ fn test_write_error_handling() { .stderr_contains("No space left on device"); } +/// Write errors must be diagnosed as "cat: write error: …" without naming +/// the input file. +#[test] +#[cfg(target_os = "linux")] +fn test_splice_write_error_message() { + use std::fs::File; + + let dev_full = + File::create("/dev/full").expect("Failed to open /dev/full - test must run on Linux"); + + new_ucmd!() + .pipe_in("test content that should cause write error to /dev/full") + .set_stdout(dev_full) + .fails() + .code_is(1) + .stderr_contains("cat: write error: No space left on device"); +} + #[test] #[cfg(target_os = "linux")] fn test_version_help_dev_full() { From 85a9345945a3f4e4b01ace5fa6e3740fa72ec2df Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Sat, 8 Aug 2026 21:37:56 +0600 Subject: [PATCH 3/6] cat: change WriteIo error to Write error Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 8b4ddd8b383..8ecd798a08f 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -83,7 +83,7 @@ enum CatError { /// A write error to the output; unlike [`Self::Io`] it is reported /// without the input filename. #[error("{}: {}", translate!("common-write-error"), strip_errno(.0))] - WriteIo(io::Error), + Write(io::Error), /// Unknown file type; it's not a regular file, socket, etc. #[error("{}", translate!("cat-error-unknown-filetype", "ft_debug" => .ft_debug))] UnknownFiletype { @@ -109,7 +109,7 @@ impl CatError { /// Write errors are reported without the filename, /// while input errors name the file. fn display_message(&self, path: &OsString) -> String { - if matches!(self, Self::WriteIo(_)) { + if matches!(self, Self::Write(_)) { format!("{self}") } else { format!("{}: {self}", path.maybe_quote()) @@ -538,7 +538,7 @@ fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) - some_copied = true; remaining -= bytes_written; } - Err(err) if some_copied => return Err(CatError::WriteIo(err.into())), + Err(err) if some_copied => return Err(CatError::Write(err.into())), Err(_) => { // stdout cannot take splice data: drain the // intermediate pipe with read/write, then let @@ -551,7 +551,7 @@ fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) - uucore::io::RawWriter(&output) .write_all(&drain) .inspect_err(handle_broken_pipe) - .map_err(CatError::WriteIo)?; + .map_err(CatError::Write)?; return Ok(false); } } @@ -580,7 +580,7 @@ fn print_unbuffered( stdout .write_all(&buf[..n]) .inspect_err(handle_broken_pipe) - .map_err(CatError::WriteIo)?; + .map_err(CatError::Write)?; // cannot use rustix::io on Windows // really bad workaround for unbuffered write #[cfg(not(any(unix, target_os = "wasi")))] From db83329b5e813c5b709573313ddbc828ea9290d4 Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Sat, 8 Aug 2026 22:01:53 +0600 Subject: [PATCH 4/6] cat: move to uucore, rename test, don't use as_fd, use map_err_context Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 89 +++++++-------------------- src/uucore/src/lib/features/pipes.rs | 91 ++++++++++++++++++++++++++++ tests/by-util/test_cat.rs | 2 +- 3 files changed, 115 insertions(+), 67 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 8ecd798a08f..e89d41d4c9a 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, strip_errno}; +use uucore::error::{FromIo, UIoError, UResult, strip_errno}; use uucore::translate; use uucore::{fast_inc::fast_inc_one, format_usage}; @@ -82,8 +82,8 @@ enum CatError { Io(#[from] io::Error), /// A write error to the output; unlike [`Self::Io`] it is reported /// without the input filename. - #[error("{}: {}", translate!("common-write-error"), strip_errno(.0))] - Write(io::Error), + #[error("{0}")] + Write(Box), /// Unknown file type; it's not a regular file, socket, etc. #[error("{}", translate!("cat-error-unknown-filetype", "ft_debug" => .ft_debug))] UnknownFiletype { @@ -123,6 +123,12 @@ impl From for CatError { Self::Io(value.into()) } } + +/// An error writing to the output, using the shared uucore "write error" +/// context (as opposed to [`CatError::Io`], which names the input file). +fn write_err(err: io::Error) -> CatError { + CatError::Write(err.map_err_context(|| translate!("common-write-error"))) +} #[derive(PartialEq)] enum NumberingMode { None, @@ -496,73 +502,24 @@ fn print_fast(handle: &mut InputHandle) -> CatResult<()> { let stdout = io::stdout(); // Try to use the splice() system call for faster writing. If it works, we're done. #[cfg(any(target_os = "linux", target_os = "android"))] - if splice_cat(handle, &stdout)? { - return Ok(()); + match uucore::pipes::splice_unbounded_diagnose(&handle.reader, &stdout) { + // splice() copied the whole input. + Ok(uucore::pipes::SpliceOutcome::Done) => return Ok(()), + // splice() is not usable here; fall back on slower writing. + Ok(uucore::pipes::SpliceOutcome::Fallback) => {} + // Once data was copied, a splice error is diagnosed: input errors + // name the file, output errors are generic write errors. + Err((uucore::pipes::SpliceErrorSide::Input, errno)) => { + return Err(CatError::Io(io::Error::from(errno))); + } + Err((uucore::pipes::SpliceErrorSide::Output, errno)) => { + return Err(write_err(io::Error::from(errno))); + } } - // If we're not on Linux or Android, or the splice() call failed, - // fall back on slower writing. print_unbuffered(handle, stdout) } -/// Copy the input to `stdout` with the `splice(2)` syscall. -/// -/// Returns `Ok(true)` if the whole input was copied, `Ok(false)` if the -/// caller should fall back on ordinary read/write, and `Err(..)` for a -/// diagnosed error. Once any bytes have been spliced, a splice error is -/// fatal and is reported either as an input error (naming the file) or as -/// a "write error"; before that, splice errors just select read/write. -#[cfg(any(target_os = "linux", target_os = "android"))] -fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) -> CatResult { - // Create a broker pipe, since splice(2) needs a pipe on one side and we - // want to distinguish read errors from write errors. If the pipe cannot - // be created (e.g. file descriptor exhaustion), fall back on read/write. - let Ok((pipe_rd, pipe_wr)) = uucore::pipes::pipe::() else { - return Ok(false); - }; - let input = handle.reader.as_fd(); - let output = stdout.as_fd(); - - let mut some_copied = false; - loop { - match uucore::pipes::splice(&input, &pipe_wr, uucore::pipes::MAX_ROOTLESS_PIPE_SIZE) { - // End of input: splice handled the whole file. - Ok(0) => return Ok(true), - Ok(bytes_read) => { - let mut remaining = bytes_read; - while remaining > 0 { - match uucore::pipes::splice(&pipe_rd, &output, remaining) { - // No progress; stop splicing. - Ok(0) => return Ok(some_copied), - Ok(bytes_written) => { - some_copied = true; - remaining -= bytes_written; - } - Err(err) if some_copied => return Err(CatError::Write(err.into())), - Err(_) => { - // stdout cannot take splice data: drain the - // intermediate pipe with read/write, then let - // the caller continue likewise. - let mut drain = Vec::with_capacity(remaining); - let _ = pipe_rd - .take(remaining as u64) - .read_to_end(&mut drain) - .map_err(CatError::Io)?; - uucore::io::RawWriter(&output) - .write_all(&drain) - .inspect_err(handle_broken_pipe) - .map_err(CatError::Write)?; - return Ok(false); - } - } - } - } - Err(err) if some_copied => return Err(CatError::Io(err.into())), - Err(_) => return Ok(false), - } - } -} - #[cfg_attr(any(target_os = "linux", target_os = "android"), inline(never))] // splice fast-path does not require this allocation fn print_unbuffered( handle: &mut InputHandle, @@ -580,7 +537,7 @@ fn print_unbuffered( stdout .write_all(&buf[..n]) .inspect_err(handle_broken_pipe) - .map_err(CatError::Write)?; + .map_err(write_err)?; // cannot use rustix::io on Windows // really bad workaround for unbuffered write #[cfg(not(any(unix, target_os = "wasi")))] diff --git a/src/uucore/src/lib/features/pipes.rs b/src/uucore/src/lib/features/pipes.rs index 02c13dc8bad..2adda736c29 100644 --- a/src/uucore/src/lib/features/pipes.rs +++ b/src/uucore/src/lib/features/pipes.rs @@ -112,6 +112,97 @@ pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> PipeRe } } +/// Whether an unbounded splice copy finished or must fall back. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SpliceOutcome { + /// Everything was spliced to the end of input. + Done, + /// splice() is not usable here; the caller should use read/write. + Fallback, +} + +/// The side of a failed splice. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SpliceErrorSide { + Input, + Output, +} + +/// Copy `source` to `sink` with splice(2) through a broker pipe, reporting +/// errors by direction. +/// +/// The first splice of the input may fail for innocent reasons (source or +/// sink unsupported, interrupted, ...), so an error before any byte was +/// moved just falls back to read/write. Once data has been moved, any +/// splice error is reported, tagged with the direction that failed. +#[inline] +pub fn splice_unbounded_diagnose( + source: &impl AsFd, + sink: &impl AsFd, +) -> Result { + // Create a broker pipe, since splice(2) needs a pipe on one side and we + // want to distinguish input errors from output errors. If the pipe + // cannot be created (e.g. file descriptor exhaustion), fall back. + let Ok((pipe_rd, pipe_wr)) = pipe::() else { + return Ok(SpliceOutcome::Fallback); + }; + + // fcntl for input would not improve throughput since + // - sender with splice probably increased size already + // - sender without splice is bottleneck + let _ = fcntl_setpipe_size(sink, MAX_ROOTLESS_PIPE_SIZE); + // pre-generate page caches for splice + let _ = rustix::fs::fadvise(source, 0, None, rustix::fs::Advice::Sequential); + + // First input splice: any failure just selects read/write. + let mut remaining = match splice(source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + Ok(0) => return Ok(SpliceOutcome::Done), + Ok(n) => n, + Err(_) => return Ok(SpliceOutcome::Fallback), + }; + + // First output splice: if the sink cannot accept splice data, drain the + // broker pipe with plain read/write and hand the rest over to the caller. + while remaining > 0 { + match splice(&pipe_rd, sink, remaining) { + Ok(0) => return Ok(SpliceOutcome::Fallback), + Ok(written) => remaining -= written, + Err(_) => { + let mut drain = Vec::with_capacity(remaining); + let _ = pipe_rd.take(remaining as u64).read_to_end(&mut drain); + match RawWriter(sink).write_all(&drain) { + Ok(()) => return Ok(SpliceOutcome::Fallback), + Err(e) => { + return Err(( + SpliceErrorSide::Output, + e.raw_os_error().map_or( + rustix::io::Errno::IO, + rustix::io::Errno::from_raw_os_error, + ), + )); + } + } + } + } + } + + // From here on splice works: any failure is a diagnosed error. + loop { + let mut remaining = match splice(source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + Ok(0) => return Ok(SpliceOutcome::Done), + Ok(n) => n, + Err(errno) => return Err((SpliceErrorSide::Input, errno)), + }; + while remaining > 0 { + match splice(&pipe_rd, sink, remaining) { + Ok(0) => return Ok(SpliceOutcome::Done), + Ok(written) => remaining -= written, + Err(errno) => return Err((SpliceErrorSide::Output, errno)), + } + } + } +} + /// splice `n` bytes with read/write fallback /// return actually sent bytes #[inline] diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index e6f744475ba..8559b3d426d 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -860,7 +860,7 @@ fn test_write_error_handling() { /// the input file. #[test] #[cfg(target_os = "linux")] -fn test_splice_write_error_message() { +fn test_write_error_message() { use std::fs::File; let dev_full = From 2d8d38f58b6df73fa5dc17dd06380c34c05b9186 Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Sat, 8 Aug 2026 22:52:43 +0600 Subject: [PATCH 5/6] cat: wait for pr Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 86 ++++++++++++++++++++++---- src/uucore/src/lib/features/pipes.rs | 91 ---------------------------- 2 files changed, 73 insertions(+), 104 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index e89d41d4c9a..cc66e670cae 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -502,24 +502,84 @@ fn print_fast(handle: &mut InputHandle) -> CatResult<()> { let stdout = io::stdout(); // Try to use the splice() system call for faster writing. If it works, we're done. #[cfg(any(target_os = "linux", target_os = "android"))] - match uucore::pipes::splice_unbounded_diagnose(&handle.reader, &stdout) { - // splice() copied the whole input. - Ok(uucore::pipes::SpliceOutcome::Done) => return Ok(()), - // splice() is not usable here; fall back on slower writing. - Ok(uucore::pipes::SpliceOutcome::Fallback) => {} - // Once data was copied, a splice error is diagnosed: input errors - // name the file, output errors are generic write errors. - Err((uucore::pipes::SpliceErrorSide::Input, errno)) => { - return Err(CatError::Io(io::Error::from(errno))); - } - Err((uucore::pipes::SpliceErrorSide::Output, errno)) => { - return Err(write_err(io::Error::from(errno))); - } + if splice_cat(handle, &stdout)? { + return Ok(()); } print_unbuffered(handle, stdout) } +/// Copy the input to `stdout` with the `splice(2)` syscall. +/// +/// Returns `Ok(true)` if the whole input was copied, `Ok(false)` if the +/// caller should fall back on ordinary read/write, and `Err(..)` for a +/// diagnosed error. +/// +/// The first splice of a file may fail for +/// innocent reasons (unsupported source or sink, interruption, descriptor +/// exhaustion), so a failure while copying the first chunk merely selects +/// the read/write fallback. Once splice is known to work, every error is +/// fatal, reported either as an input error (naming the file) or as a +/// write error. Move this into `uucore::pipes` once the shared splice +/// helpers support the same error distinction. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) -> CatResult { + use uucore::pipes::{MAX_ROOTLESS_PIPE_SIZE, pipe, splice}; + + // Create a broker pipe, since splice(2) needs a pipe on one side and we + // want to distinguish read errors from write errors. If the pipe cannot + // be created (e.g. file descriptor exhaustion), fall back on read/write. + let Ok((pipe_rd, pipe_wr)) = pipe::() else { + return Ok(false); + }; + + // First input splice: any failure just selects read/write. + let mut remaining = match splice(&handle.reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + // End of input: splice handled the whole file. + Ok(0) => return Ok(true), + Ok(bytes_read) => bytes_read, + Err(_) => return Ok(false), + }; + + // First output splice: if stdout cannot accept splice data, drain the + // broker pipe with plain read/write, then let the caller continue + // likewise. + while remaining > 0 { + match splice(&pipe_rd, stdout, remaining) { + // No progress; stop splicing. + Ok(0) => return Ok(false), + Ok(bytes_written) => remaining -= bytes_written, + Err(_) => { + let mut drain = Vec::with_capacity(remaining); + let _ = pipe_rd.take(remaining as u64).read_to_end(&mut drain); + uucore::io::RawWriter(stdout) + .write_all(&drain) + .inspect_err(handle_broken_pipe) + .map_err(write_err)?; + return Ok(false); + } + } + } + + // splice is usable: from here on every error is a diagnosed error. + loop { + let mut remaining = match splice(&handle.reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + // End of input: splice handled the whole file. + Ok(0) => return Ok(true), + Ok(bytes_read) => bytes_read, + Err(errno) => return Err(CatError::Io(errno.into())), + }; + while remaining > 0 { + match splice(&pipe_rd, stdout, remaining) { + // No progress; stop splicing. + Ok(0) => return Ok(true), + Ok(bytes_written) => remaining -= bytes_written, + Err(errno) => return Err(write_err(errno.into())), + } + } + } +} + #[cfg_attr(any(target_os = "linux", target_os = "android"), inline(never))] // splice fast-path does not require this allocation fn print_unbuffered( handle: &mut InputHandle, diff --git a/src/uucore/src/lib/features/pipes.rs b/src/uucore/src/lib/features/pipes.rs index 2adda736c29..02c13dc8bad 100644 --- a/src/uucore/src/lib/features/pipes.rs +++ b/src/uucore/src/lib/features/pipes.rs @@ -112,97 +112,6 @@ pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> PipeRe } } -/// Whether an unbounded splice copy finished or must fall back. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SpliceOutcome { - /// Everything was spliced to the end of input. - Done, - /// splice() is not usable here; the caller should use read/write. - Fallback, -} - -/// The side of a failed splice. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SpliceErrorSide { - Input, - Output, -} - -/// Copy `source` to `sink` with splice(2) through a broker pipe, reporting -/// errors by direction. -/// -/// The first splice of the input may fail for innocent reasons (source or -/// sink unsupported, interrupted, ...), so an error before any byte was -/// moved just falls back to read/write. Once data has been moved, any -/// splice error is reported, tagged with the direction that failed. -#[inline] -pub fn splice_unbounded_diagnose( - source: &impl AsFd, - sink: &impl AsFd, -) -> Result { - // Create a broker pipe, since splice(2) needs a pipe on one side and we - // want to distinguish input errors from output errors. If the pipe - // cannot be created (e.g. file descriptor exhaustion), fall back. - let Ok((pipe_rd, pipe_wr)) = pipe::() else { - return Ok(SpliceOutcome::Fallback); - }; - - // fcntl for input would not improve throughput since - // - sender with splice probably increased size already - // - sender without splice is bottleneck - let _ = fcntl_setpipe_size(sink, MAX_ROOTLESS_PIPE_SIZE); - // pre-generate page caches for splice - let _ = rustix::fs::fadvise(source, 0, None, rustix::fs::Advice::Sequential); - - // First input splice: any failure just selects read/write. - let mut remaining = match splice(source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { - Ok(0) => return Ok(SpliceOutcome::Done), - Ok(n) => n, - Err(_) => return Ok(SpliceOutcome::Fallback), - }; - - // First output splice: if the sink cannot accept splice data, drain the - // broker pipe with plain read/write and hand the rest over to the caller. - while remaining > 0 { - match splice(&pipe_rd, sink, remaining) { - Ok(0) => return Ok(SpliceOutcome::Fallback), - Ok(written) => remaining -= written, - Err(_) => { - let mut drain = Vec::with_capacity(remaining); - let _ = pipe_rd.take(remaining as u64).read_to_end(&mut drain); - match RawWriter(sink).write_all(&drain) { - Ok(()) => return Ok(SpliceOutcome::Fallback), - Err(e) => { - return Err(( - SpliceErrorSide::Output, - e.raw_os_error().map_or( - rustix::io::Errno::IO, - rustix::io::Errno::from_raw_os_error, - ), - )); - } - } - } - } - } - - // From here on splice works: any failure is a diagnosed error. - loop { - let mut remaining = match splice(source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { - Ok(0) => return Ok(SpliceOutcome::Done), - Ok(n) => n, - Err(errno) => return Err((SpliceErrorSide::Input, errno)), - }; - while remaining > 0 { - match splice(&pipe_rd, sink, remaining) { - Ok(0) => return Ok(SpliceOutcome::Done), - Ok(written) => remaining -= written, - Err(errno) => return Err((SpliceErrorSide::Output, errno)), - } - } - } -} - /// splice `n` bytes with read/write fallback /// return actually sent bytes #[inline] From c2a4f9e9ddbdf518e430b6a5fa9bb82024363c2e Mon Sep 17 00:00:00 2001 From: MuntasirSZN Date: Mon, 10 Aug 2026 17:01:34 +0600 Subject: [PATCH 6/6] utils: address comment Signed-off-by: MuntasirSZN --- src/uu/cat/src/cat.rs | 71 +++++++++++----------- src/uu/tail/src/tail.rs | 10 +++- src/uu/tee/src/tee.rs | 16 +++-- src/uucore/src/lib/features/buf_copy.rs | 16 +++-- src/uucore/src/lib/features/pipes.rs | 78 ++++++++++++++++--------- 5 files changed, 120 insertions(+), 71 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index cc66e670cae..e8191ab8ead 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -117,13 +117,6 @@ impl CatError { } } -#[cfg(any(unix, target_os = "wasi"))] -impl From for CatError { - fn from(value: rustix::io::Errno) -> Self { - Self::Io(value.into()) - } -} - /// An error writing to the output, using the shared uucore "write error" /// context (as opposed to [`CatError::Io`], which names the input file). fn write_err(err: io::Error) -> CatError { @@ -500,45 +493,47 @@ fn get_input_type(path: &OsString) -> CatResult { /// simple memory copy. fn print_fast(handle: &mut InputHandle) -> CatResult<()> { let stdout = io::stdout(); - // Try to use the splice() system call for faster writing. If it works, we're done. + // Try to use the splice() system call for faster writing. #[cfg(any(target_os = "linux", target_os = "android"))] - if splice_cat(handle, &stdout)? { - return Ok(()); + match splice_cat(&handle.reader, &stdout) { + // splice() copied the whole input. + Ok(()) => return Ok(()), + // splice() is unusable here; fall back on slower writing. + Err(CatError::Io(e)) if uucore::pipes::splice_unusable(&e) => {} + Err(e) => return Err(e), } + // If we're not on Linux or Android, or the splice() call failed, + // fall back on slower writing. print_unbuffered(handle, stdout) } -/// Copy the input to `stdout` with the `splice(2)` syscall. +/// Copy `source` to `stdout` with the `splice(2)` syscall, diagnosing +/// errors like GNU. /// -/// Returns `Ok(true)` if the whole input was copied, `Ok(false)` if the -/// caller should fall back on ordinary read/write, and `Err(..)` for a -/// diagnosed error. -/// -/// The first splice of a file may fail for -/// innocent reasons (unsupported source or sink, interruption, descriptor -/// exhaustion), so a failure while copying the first chunk merely selects -/// the read/write fallback. Once splice is known to work, every error is -/// fatal, reported either as an input error (naming the file) or as a -/// write error. Move this into `uucore::pipes` once the shared splice -/// helpers support the same error distinction. +/// Returns `Ok(())` if splice handled the whole input. `Err(EINVAL)` +/// (checked with [`uucore::pipes::splice_unusable`]) means splice is +/// unusable here and the caller should fall back on read/write; ENOSYS is +/// folded into EINVAL, and so are failures before any byte was moved. Once +/// splice is known to work, every error is fatal, reported either as an +/// input error (naming the file) or as a write error. #[cfg(any(target_os = "linux", target_os = "android"))] -fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) -> CatResult { +fn splice_cat(reader: &R, stdout: &io::Stdout) -> CatResult<()> { use uucore::pipes::{MAX_ROOTLESS_PIPE_SIZE, pipe, splice}; // Create a broker pipe, since splice(2) needs a pipe on one side and we // want to distinguish read errors from write errors. If the pipe cannot // be created (e.g. file descriptor exhaustion), fall back on read/write. let Ok((pipe_rd, pipe_wr)) = pipe::() else { - return Ok(false); + return Err(CatError::Io(unusable_errno())); }; // First input splice: any failure just selects read/write. - let mut remaining = match splice(&handle.reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + let mut remaining = match splice(reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { // End of input: splice handled the whole file. - Ok(0) => return Ok(true), + Ok(0) => return Ok(()), Ok(bytes_read) => bytes_read, - Err(_) => return Ok(false), + Err(_) => return Err(CatError::Io(unusable_errno())), }; // First output splice: if stdout cannot accept splice data, drain the @@ -547,7 +542,7 @@ fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) - while remaining > 0 { match splice(&pipe_rd, stdout, remaining) { // No progress; stop splicing. - Ok(0) => return Ok(false), + Ok(0) => return Err(CatError::Io(unusable_errno())), Ok(bytes_written) => remaining -= bytes_written, Err(_) => { let mut drain = Vec::with_capacity(remaining); @@ -556,30 +551,38 @@ fn splice_cat(handle: &mut InputHandle, stdout: &io::Stdout) - .write_all(&drain) .inspect_err(handle_broken_pipe) .map_err(write_err)?; - return Ok(false); + return Err(CatError::Io(unusable_errno())); } } } // splice is usable: from here on every error is a diagnosed error. loop { - let mut remaining = match splice(&handle.reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { + let mut remaining = match splice(reader, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { // End of input: splice handled the whole file. - Ok(0) => return Ok(true), + Ok(0) => return Ok(()), Ok(bytes_read) => bytes_read, - Err(errno) => return Err(CatError::Io(errno.into())), + Err(errno) => return Err(CatError::Io(io::Error::from(errno))), }; while remaining > 0 { match splice(&pipe_rd, stdout, remaining) { // No progress; stop splicing. - Ok(0) => return Ok(true), + Ok(0) => return Ok(()), Ok(bytes_written) => remaining -= bytes_written, - Err(errno) => return Err(write_err(errno.into())), + Err(errno) => return Err(write_err(io::Error::from(errno))), } } } } +/// Default splice failure: unusable, i.e. fold every pre-copy failure and +/// ENOSYS into EINVAL so callers test one errno (see +/// [`uucore::pipes::splice_unusable`]). +#[cfg(any(target_os = "linux", target_os = "android"))] +fn unusable_errno() -> io::Error { + io::Error::from_raw_os_error(rustix::io::Errno::INVAL.raw_os_error()) +} + #[cfg_attr(any(target_os = "linux", target_os = "android"), inline(never))] // splice fast-path does not require this allocation fn print_unbuffered( handle: &mut InputHandle, diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index a1efcf200cc..ee164bc570e 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -586,8 +586,14 @@ fn print_target_section< } } else { #[cfg(any(target_os = "linux", target_os = "android"))] - if uucore::pipes::splice_unbounded_auto(file, &mut stdout)?.is_err() { - io::copy(file, &mut stdout)?; + match uucore::pipes::splice_unbounded_auto(file, &mut stdout) { + // EINVAL means splice is unusable; copy with read/write. + Err(e) if uucore::pipes::splice_unusable(&e) => { + io::copy(file, &mut stdout)?; + } + // Real errors and success both need no further copying. + Ok(()) => {} + Err(e) => return Err(e.into()), } #[cfg(not(any(target_os = "linux", target_os = "android")))] io::copy(file, &mut stdout)?; diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index e43a0a02051..37195124ad5 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -155,10 +155,18 @@ impl MultiWriter { macro_rules! splice_or_detach { ($pipe:expr, $writer:expr, $len:expr) => { if let Err(e) = uucore::pipes::drain_pipe($pipe, $writer, $len) { - self.aborted |= - process_error(self.output_error_mode, e, $writer, &mut self.ignored_errors) - .is_err(); - $writer.name.clear(); //mark as exited + // EINVAL means splice fell back to read/write: keep + // this writer, the data was still written. + if !uucore::pipes::splice_unusable(&e) { + self.aborted |= process_error( + self.output_error_mode, + e, + $writer, + &mut self.ignored_errors, + ) + .is_err(); + $writer.name.clear(); //mark as exited + } } }; } diff --git a/src/uucore/src/lib/features/buf_copy.rs b/src/uucore/src/lib/features/buf_copy.rs index 9b3bc565e9d..72db9570a4d 100644 --- a/src/uucore/src/lib/features/buf_copy.rs +++ b/src/uucore/src/lib/features/buf_copy.rs @@ -15,11 +15,17 @@ pub fn copy_fast( src: &mut (impl std::io::Read + AsFd), dest: &mut impl AsFd, ) -> std::io::Result<()> { - if crate::pipes::splice_unbounded_auto(src, dest)?.is_err() { - // fall back on writing "without buffering", or order of output would be wrong - // unrelated for cp /dev/stdin since cp does not have multiple input? - // RawWriter also removes io::copy's specialization e.g. copy_file_range which might use reflink - std::io::copy(src, &mut crate::io::RawWriter(dest))?; + match crate::pipes::splice_unbounded_auto(src, dest) { + // EINVAL means splice is unusable; fall back on read/write. + Err(e) if crate::pipes::splice_unusable(&e) => { + // fall back on writing "without buffering", or order of output would be wrong + // unrelated for cp /dev/stdin since cp does not have multiple input? + // RawWriter also removes io::copy's specialization e.g. copy_file_range which might use reflink + std::io::copy(src, &mut crate::io::RawWriter(dest))?; + } + // Real errors and success both need no further copying. + Ok(()) => {} + Err(e) => return Err(e), } Ok(()) } diff --git a/src/uucore/src/lib/features/pipes.rs b/src/uucore/src/lib/features/pipes.rs index 02c13dc8bad..ae1e8c45d76 100644 --- a/src/uucore/src/lib/features/pipes.rs +++ b/src/uucore/src/lib/features/pipes.rs @@ -17,13 +17,24 @@ use std::{ pub const MAX_ROOTLESS_PIPE_SIZE: usize = 1024 * 1024; const KERNEL_DEFAULT_PIPE_SIZE: usize = 64 * 1024; -/// A type allows to -/// - check that zero-copy succeed by ?.is_ok() -/// - check that zero-copy failed, but read/write fallback succeed by ?.is_err() -/// - catch the read/write fallback's error by ? or let Err(e) +/// Whether an error from the splice helpers means that splice is unusable +/// here, so the caller should fall back on read/write. /// -/// use rustix::io::Result for functions without read/write fallback -type PipeRes = std::io::Result>; +/// The helpers in this module use `Err(EINVAL)` as that marker: +/// +/// - `drain_pipe` fell back to read/write (the data was still written) +/// - `splice_unbounded_auto` could not splice anything +/// +/// and they fold `ENOSYS` (kernel without the syscall) into it. +#[inline] +pub fn splice_unusable(err: &std::io::Error) -> bool { + err.raw_os_error() == Some(rustix::io::Errno::INVAL.raw_os_error()) +} + +#[inline] +fn splice_unusable_errno() -> std::io::Error { + std::io::Error::from_raw_os_error(rustix::io::Errno::INVAL.raw_os_error()) +} /// return pipe and try to extend its size /// SIZE_REQUIRED should be true if you want to fail when changing pipe size failed @@ -56,23 +67,34 @@ pub fn splice(source: &impl AsFd, target: &impl AsFd, len: usize) -> rustix::io: } /// splice `len` bytes from `pipe` into `dest`. +/// +/// Returns `Err(EINVAL)` if splice turned out to be unusable: the data was +/// delivered by the read/write fallback instead, see [`splice_unusable`]. #[inline] -pub fn drain_pipe(pipe: &PipeReader, dest: &impl AsFd, len: usize) -> PipeRes { +pub fn drain_pipe(pipe: &PipeReader, dest: &impl AsFd, len: usize) -> std::io::Result<()> { debug_assert!(len <= MAX_ROOTLESS_PIPE_SIZE, "unexpected RAM usage"); let mut remaining = len; while remaining > 0 { - if let Ok(s) = splice(pipe, dest, remaining) { - remaining -= s; - } else { - // read/write fallback - // use read_to_end to make pipe empty for the case write failed - let mut drain = Vec::with_capacity(remaining); - pipe.take(remaining as u64).read_to_end(&mut drain)?; - RawWriter(&dest).write_all(&drain)?; - return Ok(Err(())); + match splice(pipe, dest, remaining) { + Ok(0) => { + // no progress; drain by hand + let mut drain = Vec::with_capacity(remaining); + pipe.take(remaining as u64).read_to_end(&mut drain)?; + RawWriter(&dest).write_all(&drain)?; + return Err(splice_unusable_errno()); + } + Ok(s) => remaining -= s, + Err(_) => { + // read/write fallback + // use read_to_end to make pipe empty for the case write failed + let mut drain = Vec::with_capacity(remaining); + pipe.take(remaining as u64).read_to_end(&mut drain)?; + RawWriter(&dest).write_all(&drain)?; + return Err(splice_unusable_errno()); + } } } - Ok(Ok(())) + Ok(()) } /// check that source is FUSE @@ -86,11 +108,14 @@ pub fn might_fuse(source: &impl AsFd) -> bool { /// /// throughput is better than direct splice for the case one of in/output is pipe by unknown reason /// This includes read ahead and optimization for stdout's pipe size +/// +/// Returns `Err(EINVAL)` when nothing could be spliced, see [`splice_unusable`]. +/// Errors while draining the intermediate pipe are real output errors. #[inline] -pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> PipeRes { +pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> std::io::Result<()> { static PIPE_CACHE: OnceLock> = OnceLock::new(); let Some((pipe_rd, pipe_wr)) = PIPE_CACHE.get_or_init(|| pipe::().ok()) else { - return Ok(Err(())); + return Err(splice_unusable_errno()); }; // fcntl for input would not improve throughput since @@ -101,13 +126,11 @@ pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> PipeRe let _ = rustix::fs::fadvise(source, 0, None, rustix::fs::Advice::Sequential); loop { match splice(&source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) { - Ok(0) => return Ok(Ok(())), + Ok(0) => return Ok(()), Ok(n) => { - if drain_pipe(pipe_rd, dest, n)?.is_err() { - return Ok(Err(())); - } + drain_pipe(pipe_rd, dest, n)?; } - Err(_) => return Ok(Err(())), + Err(_) => return Err(splice_unusable_errno()), } } } @@ -161,8 +184,11 @@ pub fn send_n_bytes(input: impl AsFd, target: impl AsFd, n: u64) -> std::io::Res Ok(s) => { n -= s as u64; bytes_written += s as u64; - if drain_pipe(broker_r, &target, s)?.is_err() { - break false; + if let Err(e) = drain_pipe(broker_r, &target, s) { + if splice_unusable(&e) { + break false; + } + return Err(e); } } _ => break false,