Merge pull request #6086 from sylvestre/shred-v

shred: some small improvements

src/uu/shred/src/shred.rs

@@ -365,6 +365,7 @@ fn get_size(size_str_opt: Option<String>) -> Option<u64> {
         .or_else(|| {
             if let Some(size) = size_str_opt {
                 show_error!("invalid file size: {}", size.quote());
+                // TODO: replace with our error management
                 std::process::exit(1);
             }
             None
@@ -406,9 +407,10 @@ fn wipe_file(
         ));
     }
 
+    let metadata = fs::metadata(path).map_err_context(String::new)?;
+
     // If force is true, set file permissions to not-readonly.
     if force {
-        let metadata = fs::metadata(path).map_err_context(String::new)?;
         let mut perms = metadata.permissions();
         #[cfg(unix)]
         #[allow(clippy::useless_conversion, clippy::unnecessary_cast)]
@@ -428,6 +430,8 @@ fn wipe_file(
 
     // Fill up our pass sequence
     let mut pass_sequence = Vec::new();
+    if metadata.len() != 0 {
+        // Only add passes if the file is non-empty
 
         if n_passes <= 3 {
             // Only random passes if n_passes <= 3
@@ -461,6 +465,7 @@ fn wipe_file(
         if zero {
             pass_sequence.push(PassType::Pattern(PATTERNS[0]));
         }
+    }
 
     let total_passes = pass_sequence.len();
     let mut file = OpenOptions::new()
@@ -471,29 +476,19 @@ fn wipe_file(
 
     let size = match size {
         Some(size) => size,
-        None => get_file_size(path)?,
+        None => metadata.len(),
     };
 
     for (i, pass_type) in pass_sequence.into_iter().enumerate() {
         if verbose {
             let pass_name = pass_name(&pass_type);
-            if total_passes < 10 {
             show_error!(
-                    "{}: pass {}/{} ({})...",
+                "{}: pass {:2}/{} ({})...",
                 path.maybe_quote(),
                 i + 1,
                 total_passes,
                 pass_name
             );
-            } else {
-                show_error!(
-                    "{}: pass {:2.0}/{:2.0} ({})...",
-                    path.maybe_quote(),
-                    i + 1,
-                    total_passes,
-                    pass_name
-                );
-            }
         }
         // size is an optional argument for exactly how many bytes we want to shred
         // Ignore failed writes; just keep trying
@@ -539,10 +534,6 @@ fn do_pass(
     Ok(())
 }
 
-fn get_file_size(path: &Path) -> Result<u64, io::Error> {
-    Ok(fs::metadata(path)?.len())
-}
-
 // 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 if error
 fn wipe_name(orig_path: &Path, verbose: bool, remove_method: RemoveMethod) -> Option<PathBuf> {
@@ -589,7 +580,8 @@ fn wipe_name(orig_path: &Path, verbose: bool, remove_method: RemoveMethod) -> Op
                         new_path.quote(),
                         e
                     );
-                    return None;
+                    // TODO: replace with our error management
+                    std::process::exit(1);
                 }
             }
         }

tests/by-util/test_shred.rs

@@ -143,3 +143,60 @@ fn test_hex() {
 
     ucmd.arg("--size=0x10").arg(file).succeeds();
 }
+
+#[test]
+fn test_shred_empty() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let file_a = "test_shred_remove_a";
+
+    at.touch(file_a);
+
+    // Shred file_a and verify that, as it is empty, it doesn't have "pass 1/3 (random)"
+    scene
+        .ucmd()
+        .arg("-uv")
+        .arg(file_a)
+        .succeeds()
+        .stderr_does_not_contain("1/3 (random)");
+
+    assert!(!at.file_exists(file_a));
+
+    // if the file isn't empty, we should have random
+    at.touch(file_a);
+    at.write(file_a, "1");
+    scene
+        .ucmd()
+        .arg("-uv")
+        .arg(file_a)
+        .succeeds()
+        .stderr_contains("1/3 (random)");
+
+    assert!(!at.file_exists(file_a));
+}
+
+#[test]
+#[cfg(all(unix, feature = "chmod"))]
+fn test_shred_fail_no_perm() {
+    use std::path::Path;
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let dir = "dir";
+
+    let file = "test_shred_remove_a";
+
+    let binding = Path::new("dir").join(file);
+    let path = binding.to_str().unwrap();
+    at.mkdir(dir);
+    at.touch(path);
+    scene.ccmd("chmod").arg("a-w").arg(dir).succeeds();
+
+    scene
+        .ucmd()
+        .arg("-uv")
+        .arg(path)
+        .fails()
+        .stderr_contains("Couldn't rename to");
+}