fix: properly handle write errors for --version and --help output (#10223)

- Modified ClapErrorWrapper to detect and report when printing --version/--help fails
- Added test_version_help_dev_full to verify error handling
- Fixes issue where cat --version > /dev/full silently succeeded instead of failing

Addresses feedback from commit 9cc2e096d8

Co-authored-by: Cả thế giới là Rust <naoNao89@users.noreply.github.com>
This commit is contained in:
Cả thế giới là Rust
2026-01-18 06:02:57 +07:00
committed by GitHub
parent fc17efe7be
commit 799de29ec1
3 changed files with 48 additions and 11 deletions
+29 -8
View File
@@ -55,8 +55,10 @@
// spell-checker:ignore uioerror rustdoc
use std::{
cell::Cell,
error::Error,
fmt::{Display, Formatter},
io::Write,
sync::atomic::{AtomicI32, Ordering},
};
@@ -700,6 +702,7 @@ impl From<i32> for Box<dyn UError> {
pub struct ClapErrorWrapper {
code: i32,
error: clap::Error,
print_failed: Cell<bool>,
}
/// Extension trait for `clap::Error` to adjust the exit code.
@@ -710,13 +713,21 @@ pub trait UClapError<T> {
impl From<clap::Error> for Box<dyn UError> {
fn from(e: clap::Error) -> Self {
Box::new(ClapErrorWrapper { code: 1, error: e })
Box::new(ClapErrorWrapper {
code: 1,
error: e,
print_failed: Cell::new(false),
})
}
}
impl UClapError<ClapErrorWrapper> for clap::Error {
fn with_exit_code(self, code: i32) -> ClapErrorWrapper {
ClapErrorWrapper { code, error: self }
ClapErrorWrapper {
code,
error: self,
print_failed: Cell::new(false),
}
}
}
@@ -731,12 +742,11 @@ impl UClapError<Result<clap::ArgMatches, ClapErrorWrapper>>
impl UError for ClapErrorWrapper {
fn code(&self) -> i32 {
// If the error is a DisplayHelp or DisplayVersion variant,
// we don't want to apply the custom error code, but leave
// it 0.
// check if printing failed. If it did, return 1, otherwise 0.
if let clap::error::ErrorKind::DisplayHelp | clap::error::ErrorKind::DisplayVersion =
self.error.kind()
{
0
i32::from(self.print_failed.get())
} else {
self.code
}
@@ -748,9 +758,20 @@ impl Error for ClapErrorWrapper {}
// This is abuse of the Display trait
impl Display for ClapErrorWrapper {
fn fmt(&self, _f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
// Intentionally ignore the result - error.print() writes directly to stderr
// and we always return Ok(()) to satisfy Display's contract
let _ = self.error.print();
// Check if printing succeeds. For DisplayHelp and DisplayVersion,
// error.print() writes to stdout, so we need to detect write failures
// (e.g., when stdout is /dev/full).
if let Err(print_fail) = self.error.print() {
// Mark that printing failed so code() can return the appropriate exit code
self.print_failed.set(true);
// Try to display this error to stderr, but ignore if that fails too
// since we're already in an error state.
let _ = writeln!(std::io::stderr(), "{}: {print_fail}", crate::util_name());
// Mirror GNU behavior: when failing to print help or version, exit with error code.
// This avoids silent failures when stdout is full or closed.
set_exit_code(1);
}
// Always return Ok(()) to satisfy Display's contract and prevent panic
Ok(())
}
}
+16
View File
@@ -864,6 +864,22 @@ fn test_write_error_handling() {
.stderr_contains("No space left on device");
}
#[test]
#[cfg(target_os = "linux")]
fn test_version_help_dev_full() {
use std::fs::OpenOptions;
for option in ["--version", "--help"] {
let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap();
new_ucmd!()
.arg(option)
.set_stdout(dev_full)
.fails()
.stderr_contains("No space left on device");
}
}
#[test]
fn test_cat_eintr_handling() {
// Test that cat properly handles EINTR (ErrorKind::Interrupted) during I/O operations
+3 -3
View File
@@ -11,9 +11,9 @@
//! # CI Integration
//! EINTR handling tests are NOW visible in CI logs through integration tests:
//! - `test_cat_eintr_handling` in `tests/by-util/test_cat.rs`
//! - `test_comm_eintr_handling` in `tests/by-util/test_comm.rs`
//! - `test_comm_eintr_handling` in `tests/by-util/test_comm.rs`
//! - `test_od_eintr_handling` in `tests/by-util/test_od.rs`
//!
//!
//! These integration tests use the mock utilities from this module to verify
//! that each utility properly handles signal interruptions during I/O operations.
//! Test results appear in CI logs under the "Test" steps when running `cargo nextest run`.
@@ -171,7 +171,7 @@ mod tests {
assert_eq!(n, 5);
assert_eq!(&buf, b"hello");
// Read rest of data without interruption
// Read rest of data without interruption
let n = reader.read(&mut buf).unwrap();
assert_eq!(n, 5);
assert_eq!(&buf, b" worl"); // Second chunk of "hello world"