shred: keep going when a rename fails during --remove

src/uu/shred/locales/en-US.ftl

@@ -63,10 +63,9 @@ shred-removing = {$file}: removing
 shred-removed = {$file}: removed
 shred-renamed-to = renamed to
 shred-pass-progress = {$file}: pass
-shred-couldnt-rename = {$file}: Couldn't rename to {$new_name}: {$error}
 shred-failed-to-open-for-writing = {$file}: failed to open for writing
 shred-file-write-pass-failed = {$file}: File write pass failed
-shred-failed-to-remove-file = {$file}: failed to remove file
+shred-failed-to-remove-file = {$file}: failed to remove
 
 # File I/O error messages
 shred-failed-to-clone-file-handle = failed to clone file handle

src/uu/shred/locales/fr-FR.ftl

@@ -62,10 +62,9 @@ shred-removing = {$file} : suppression
 shred-removed = {$file} : supprimé
 shred-renamed-to = renommé en
 shred-pass-progress = {$file}: passage
-shred-couldnt-rename = {$file} : Impossible de renommer en {$new_name} : {$error}
 shred-failed-to-open-for-writing = {$file} : impossible d'ouvrir pour l'écriture
 shred-file-write-pass-failed = {$file} : Échec du passage d'écriture de fichier
-shred-failed-to-remove-file = {$file} : impossible de supprimer le fichier
+shred-failed-to-remove-file = {$file} : impossible de supprimer
 
 # Messages d'erreur E/S de fichier
 shred-failed-to-clone-file-handle = échec du clonage du descripteur de fichier

src/uu/shred/src/shred.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (words) wipesync prefill couldnt fillpattern
+// spell-checker:ignore (words) wipesync prefill fillpattern
 
 use clap::{Arg, ArgAction, Command};
 #[cfg(unix)]
@@ -18,7 +18,7 @@ use std::os::unix::prelude::PermissionsExt;
 use std::path::{Path, PathBuf};
 use uucore::diagnostics::OptionValue;
 use uucore::display::Quotable;
-use uucore::error::{FromIo, UResult, USimpleError, UUsageError, strip_errno};
+use uucore::error::{FromIo, UResult, USimpleError, UUsageError};
 use uucore::parser::parse_size::parse_size_u64;
 use uucore::parser::shortcut_value_parser::ShortcutValueParser;
 use uucore::{format_usage, show_error, show_if_err};
@@ -842,8 +842,9 @@ fn do_pass(
 }
 
 /// Repeatedly renames the file with strings of decreasing length (most likely all 0s)
-/// Return the path of the file after its last renaming or None in case of an error
-fn wipe_name(orig_path: &Path, verbose: bool, remove_method: RemoveMethod) -> PathBuf {
+/// Return the path of the file after its last successful renaming.
+/// Stops at the first failed rename.
+fn wipe_name(orig_path: &Path, verbose: bool, remove_method: RemoveMethod) -> io::Result<PathBuf> {
     let file_name_len = orig_path.file_name().unwrap().len();
 
     let mut last_path = PathBuf::from(orig_path);
@@ -871,27 +872,20 @@ fn wipe_name(orig_path: &Path, verbose: bool, remove_method: RemoveMethod) -> Pa
 
                     if remove_method == RemoveMethod::WipeSync {
                         // Sync every file rename
-                        let new_file = OpenOptions::new()
-                            .write(true)
-                            .open(new_path.clone())
-                            .expect("Failed to open renamed file for syncing");
-                        new_file.sync_all().expect("Failed to sync renamed file");
+                        let new_file = OpenOptions::new().write(true).open(new_path.clone())?;
+                        new_file.sync_all()?;
                     }
 
                     last_path = new_path;
                     break;
                 }
-                Err(e) => {
-                    let msg = translate_text!("shred-couldnt-rename", "file" => last_path.maybe_quote(), "new_name" => new_path.quote(), "error" => strip_errno(&e));
-                    show_error!("{msg}");
-                    // TODO: replace with our error management
-                    std::process::exit(1);
-                }
+                // Ignore the error and stop here: the unlink in do_remove reports it
+                Err(_) => return Ok(last_path),
             }
         }
     }
 
-    last_path
+    Ok(last_path)
 }
 
 fn do_remove(path: &Path, verbose: bool, remove_method: RemoveMethod) -> Result<(), io::Error> {
@@ -905,7 +899,7 @@ fn do_remove(path: &Path, verbose: bool, remove_method: RemoveMethod) -> Result<
     let remove_path = if remove_method == RemoveMethod::Unlink {
         path.to_path_buf()
     } else {
-        wipe_name(path, verbose, remove_method)
+        wipe_name(path, verbose, remove_method)?
     };
 
     fs::remove_file(remove_path)?;

tests/by-util/test_shred.rs

@@ -217,19 +217,25 @@ fn test_shred_fail_no_perm() {
     let dir = "dir";
 
     let file = "test_shred_remove_a";
+    let other = "test_shred_remove_b";
 
     let binding = Path::new("dir").join(file);
     let path = binding.to_str().unwrap();
     at.mkdir(dir);
     at.touch(path);
+    at.touch(other);
     scene.ccmd("chmod").arg("a-w").arg(dir).succeeds();
 
     scene
         .ucmd()
         .arg("-uv")
         .arg(path)
+        .arg(other)
         .fails()
-        .stderr_contains("Couldn't rename to");
+        .stderr_contains(format!("{path}: failed to remove: Permission denied\n"));
+
+    assert!(at.file_exists(path));
+    assert!(!at.file_exists(other));
 }
 
 #[test]
@@ -583,7 +589,7 @@ fn test_couldnt_rename() {
     new_ucmd!()
         .args(&["-u", "/proc/self/mem"])
         .fails_with_code(1)
-        .stderr_is("shred: /proc/self/mem: Couldn't rename to '/proc/self/000': No such file or directory\n");
+        .stderr_is("shred: /proc/self/mem: failed to remove: Operation not permitted\n");
 }
 
 #[rstest::rstest]