rm: report a verbose write failure once without aborting the removal

src/uu/rm/src/platform/unix.rs

@@ -22,7 +22,7 @@ use uucore::translate;
 
 use super::super::{
     InteractiveMode, Options, is_dir_empty, is_readable_metadata, prompt_descend, remove_file,
-    show_permission_denied_error, show_removal_error, show_verbose_write_error,
+    report_verbose_write_error, show_permission_denied_error, show_removal_error,
     verbose_removed_directory, verbose_removed_file,
 };
 
@@ -128,9 +128,8 @@ pub fn safe_remove_file(
             if let Some(pb) = progress_bar {
                 pb.inc(1);
             }
-            Some(show_verbose_write_error(verbose_removed_file(
-                path, options,
-            )))
+            report_verbose_write_error(verbose_removed_file(path, options));
+            Some(false)
         }
         Err(e) => {
             if e.kind() == std::io::ErrorKind::PermissionDenied {
@@ -160,9 +159,8 @@ pub fn safe_remove_empty_dir(
             if let Some(pb) = progress_bar {
                 pb.inc(1);
             }
-            Some(show_verbose_write_error(verbose_removed_directory(
-                path, options,
-            )))
+            report_verbose_write_error(verbose_removed_directory(path, options));
+            Some(false)
         }
         Err(e) => {
             let e =
@@ -208,7 +206,8 @@ fn handle_permission_denied(
         return true;
     }
     // Successfully removed empty directory
-    show_verbose_write_error(verbose_removed_directory(entry_path, options))
+    report_verbose_write_error(verbose_removed_directory(entry_path, options));
+    false
 }
 
 /// Helper to handle unlink operation with error reporting
@@ -225,12 +224,12 @@ fn handle_unlink(
         show_error!("{e}");
         true
     } else {
-        let result = if is_dir {
+        report_verbose_write_error(if is_dir {
             verbose_removed_directory(entry_path, options)
         } else {
             verbose_removed_file(entry_path, options)
-        };
-        show_verbose_write_error(result)
+        });
+        false
     }
 }
 
@@ -258,7 +257,10 @@ pub fn remove_dir_with_special_cases(path: &Path, options: &Options, error_occur
             // of the recursion.
             error_occurred
         }
-        Ok(_) => show_verbose_write_error(verbose_removed_directory(path, options)),
+        Ok(_) => {
+            report_verbose_write_error(verbose_removed_directory(path, options));
+            false
+        }
     }
 }
 
@@ -321,7 +323,8 @@ pub fn safe_remove_dir_recursive(
             if e.kind() == std::io::ErrorKind::PermissionDenied {
                 // Try to remove the directory directly if it's empty
                 if fs::remove_dir(path).is_ok() {
-                    return show_verbose_write_error(verbose_removed_directory(path, options));
+                    report_verbose_write_error(verbose_removed_directory(path, options));
+                    return false;
                 }
                 // If we can't read the directory AND can't remove it,
                 // show permission denied error for GNU compatibility

src/uu/rm/src/rm.rs

@@ -18,6 +18,7 @@ use std::os::unix::ffi::OsStrExt;
 use std::os::unix::fs::PermissionsExt;
 use std::path::MAIN_SEPARATOR;
 use std::path::Path;
+use std::sync::atomic::{AtomicBool, Ordering};
 use thiserror::Error;
 use uucore::display::Quotable;
 use uucore::error::{FromIo, UError, UResult, USimpleError, strip_errno};
@@ -87,12 +88,18 @@ fn verbose_removed_directory(path: &Path, options: &Options) -> UResult<()> {
     Ok(())
 }
 
-/// Helper function to report a verbose output write error and return error status
-fn show_verbose_write_error(result: UResult<()>) -> bool {
-    if let Err(e) = &result {
+/// Set once a verbose write failure has happened. Like GNU, a broken standard
+/// output neither interrupts the removal nor is reported more than once, but it
+/// does make `rm` exit with a failure status.
+static VERBOSE_WRITE_FAILED: AtomicBool = AtomicBool::new(false);
+
+/// Helper function to report a verbose output write error, at most once
+fn report_verbose_write_error(result: UResult<()>) {
+    if let Err(e) = result
+        && !VERBOSE_WRITE_FAILED.swap(true, Ordering::Relaxed)
+    {
         show_error!("{e}");
     }
-    result.is_err()
 }
 
 /// Helper function to show error with context and return error status
@@ -116,7 +123,10 @@ fn show_permission_denied_error(path: &Path) -> bool {
 /// Helper function to remove a directory and handle results
 fn remove_dir_with_feedback(path: &Path, options: &Options) -> bool {
     match fs::remove_dir(path) {
-        Ok(_) => show_verbose_write_error(verbose_removed_directory(path, options)),
+        Ok(_) => {
+            report_verbose_write_error(verbose_removed_directory(path, options));
+            false
+        }
         Err(e) => show_removal_error(e, path),
     }
 }
@@ -315,7 +325,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
     }
 
-    if remove(&files, &options) {
+    if remove(&files, &options) || VERBOSE_WRITE_FAILED.load(Ordering::Relaxed) {
         return Err(1.into());
     }
 
@@ -740,9 +750,7 @@ fn remove_dir_recursive(
                 // show another error message as we return from each level
                 // of the recursion.
             }
-            Ok(_) => {
-                error = error || show_verbose_write_error(verbose_removed_directory(path, options));
-            }
+            Ok(_) => report_verbose_write_error(verbose_removed_directory(path, options)),
         }
 
         error
@@ -867,7 +875,7 @@ fn remove_file(path: &Path, options: &Options, progress_bar: Option<&ProgressBar
         // Fallback method for non-Unix, Redox, or when safe traversal is unavailable
         match fs::remove_file(path) {
             Ok(_) => {
-                return show_verbose_write_error(verbose_removed_file(path, options));
+                report_verbose_write_error(verbose_removed_file(path, options));
             }
             Err(e) => {
                 if e.kind() == io::ErrorKind::PermissionDenied {

tests/by-util/test_rm.rs

@@ -383,7 +383,43 @@ fn test_verbose_write_error_does_not_panic_dir() {
         .stderr_contains("No space left on device")
         .stderr_does_not_contain("panicked");
 
+    // A broken stdout must not stop the removal itself.
     assert!(!at.file_exists(file));
+    assert!(!at.dir_exists(dir));
+}
+
+// A broken standard output must be reported once, not once per removed entry.
+#[test]
+#[cfg(target_os = "linux")]
+fn test_verbose_write_error_reported_once() {
+    use std::fs::OpenOptions;
+
+    let (at, mut ucmd) = at_and_ucmd!();
+    let dir = "test_rm_verbose_write_error_once";
+
+    at.mkdir(dir);
+    for name in ["alpha", "bravo", "charlie", "delta"] {
+        at.touch(format!("{dir}/{name}"));
+    }
+
+    let dev_full = OpenOptions::new().write(true).open("/dev/full").unwrap();
+
+    let result = ucmd
+        .arg("-r")
+        .arg("-v")
+        .arg(dir)
+        .set_stdout(dev_full)
+        .fails();
+    result.code_is(1);
+    assert_eq!(
+        result
+            .stderr_str()
+            .matches("No space left on device")
+            .count(),
+        1
+    );
+
+    assert!(!at.dir_exists(dir));
 }
 
 #[test]