From 73b1639f0b61cab2134092d634aaf04599be08a1 Mon Sep 17 00:00:00 2001 From: Jake Wallin Date: Thu, 15 Oct 2026 15:14:29 +0000 Subject: [PATCH] rmdir: handle verbose output failures without panicking --- src/uu/rmdir/src/rmdir.rs | 31 +++++++++++++++-------- tests/by-util/test_rmdir.rs | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/uu/rmdir/src/rmdir.rs b/src/uu/rmdir/src/rmdir.rs index 41e4d0c5cff..2d7f4d64e55 100644 --- a/src/uu/rmdir/src/rmdir.rs +++ b/src/uu/rmdir/src/rmdir.rs @@ -9,10 +9,10 @@ use clap::builder::ValueParser; use clap::{Arg, ArgAction, Command}; use std::ffi::OsString; use std::fs::{read_dir, remove_dir}; -use std::io; +use std::io::{self, Write}; use std::path::Path; use uucore::display::Quotable; -use uucore::error::{UResult, set_exit_code, strip_errno}; +use uucore::error::{FromIo, UResult, set_exit_code, strip_errno}; use uucore::translate; use uucore::{format_usage, show_error}; @@ -32,13 +32,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { parents: matches.get_flag(OPT_PARENTS), verbose: matches.get_flag(OPT_VERBOSE), }; + let mut verbose_result = Ok(()); for path in matches .get_many::(ARG_DIRS) .unwrap_or_default() .map(Path::new) { - if let Err(error) = remove(path, opts) { + if let Err(error) = remove(path, opts, &mut verbose_result) { let Error { error, path } = error; if opts.ignore && dir_not_empty(&error, path) { @@ -88,7 +89,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - Ok(()) + verbose_result.map_err_context(|| translate!("common-write-error")) } struct Error<'a> { @@ -96,23 +97,33 @@ struct Error<'a> { path: &'a Path, } -fn remove(mut path: &Path, opts: Opts) -> Result<(), Error<'_>> { - remove_single(path, opts)?; +fn remove<'a>( + mut path: &'a Path, + opts: Opts, + verbose_result: &mut io::Result<()>, +) -> Result<(), Error<'a>> { + remove_single(path, opts, verbose_result)?; if opts.parents { while let Some(new) = path.parent() { path = new; if path.as_os_str().is_empty() { break; } - remove_single(path, opts)?; + remove_single(path, opts, verbose_result)?; } } Ok(()) } -fn remove_single(path: &Path, opts: Opts) -> Result<(), Error<'_>> { - if opts.verbose { - println!( +fn remove_single<'a>( + path: &'a Path, + opts: Opts, + verbose_result: &mut io::Result<()>, +) -> Result<(), Error<'a>> { + // Defer reporting a write error until all directory removals have been attempted. + if opts.verbose && verbose_result.is_ok() { + *verbose_result = writeln!( + io::stdout(), "{}", translate!("rmdir-verbose-removing-directory", "util_name" => "rmdir", "path" => path.quote()) ); diff --git a/tests/by-util/test_rmdir.rs b/tests/by-util/test_rmdir.rs index 16f1cb25443..81db6e59b15 100644 --- a/tests/by-util/test_rmdir.rs +++ b/tests/by-util/test_rmdir.rs @@ -61,6 +61,55 @@ fn test_rmdir_empty_directory_with_parents() { assert!(!at.dir_exists(DIR)); } +#[test] +#[cfg(target_os = "linux")] +fn test_rmdir_verbose_write_error() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir_all(NESTED_DIR); + at.mkdir("other"); + at.mkdir_all("nonempty/child"); + let dev_full = std::fs::OpenOptions::new() + .write(true) + .open("/dev/full") + .unwrap(); + + ucmd.args(&[ + "-pv", + "--ignore-fail-on-non-empty", + "nonempty", + NESTED_DIR, + "other", + ]) + .set_stdout(dev_full) + .fails_with_code(1) + .stderr_is("rmdir: write error: No space left on device\n"); + + assert!(!at.dir_exists(DIR)); + assert!(!at.dir_exists("other")); + assert!(at.dir_exists("nonempty/child")); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_rmdir_verbose_write_error_with_removal_error() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir(DIR); + let dev_full = std::fs::OpenOptions::new() + .write(true) + .open("/dev/full") + .unwrap(); + + ucmd.args(&["-v", "missing", DIR]) + .set_stdout(dev_full) + .fails_with_code(1) + .stderr_is( + "rmdir: failed to remove 'missing': No such file or directory\n\ + rmdir: write error: No space left on device\n", + ); + + assert!(!at.dir_exists(DIR)); +} + #[test] fn test_rmdir_nonempty_directory_no_parents() { let (at, mut ucmd) = at_and_ucmd!();