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,40 +430,43 @@ 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
-        for _ in 0..n_passes {
-            pass_sequence.push(PassType::Random);
-        }
-    } else {
-        // First fill it with Patterns, shuffle it, then evenly distribute Random
-        let n_full_arrays = n_passes / PATTERNS.len(); // How many times can we go through all the patterns?
-        let remainder = n_passes % PATTERNS.len(); // How many do we get through on our last time through?
+        if n_passes <= 3 {
+            // Only random passes if n_passes <= 3
+            for _ in 0..n_passes {
+                pass_sequence.push(PassType::Random);
+            }
+        } else {
+            // First fill it with Patterns, shuffle it, then evenly distribute Random
+            let n_full_arrays = n_passes / PATTERNS.len(); // How many times can we go through all the patterns?
+            let remainder = n_passes % PATTERNS.len(); // How many do we get through on our last time through?
+
+            for _ in 0..n_full_arrays {
+                for p in PATTERNS {
+                    pass_sequence.push(PassType::Pattern(p));
+                }
+            }
+            for pattern in PATTERNS.into_iter().take(remainder) {
+                pass_sequence.push(PassType::Pattern(pattern));
+            }
+            let mut rng = rand::thread_rng();
+            pass_sequence.shuffle(&mut rng); // randomize the order of application
 
-        for _ in 0..n_full_arrays {
-            for p in PATTERNS {
-                pass_sequence.push(PassType::Pattern(p));
+            let n_random = 3 + n_passes / 10; // Minimum 3 random passes; ratio of 10 after
+                                              // Evenly space random passes; ensures one at the beginning and end
+            for i in 0..n_random {
+                pass_sequence[i * (n_passes - 1) / (n_random - 1)] = PassType::Random;
             }
         }
-        for pattern in PATTERNS.into_iter().take(remainder) {
-            pass_sequence.push(PassType::Pattern(pattern));
-        }
-        let mut rng = rand::thread_rng();
-        pass_sequence.shuffle(&mut rng); // randomize the order of application
 
-        let n_random = 3 + n_passes / 10; // Minimum 3 random passes; ratio of 10 after
-                                          // Evenly space random passes; ensures one at the beginning and end
-        for i in 0..n_random {
-            pass_sequence[i * (n_passes - 1) / (n_random - 1)] = PassType::Random;
+        // --zero specifies whether we want one final pass of 0x00 on our file
+        if zero {
+            pass_sequence.push(PassType::Pattern(PATTERNS[0]));
         }
     }
 
-    // --zero specifies whether we want one final pass of 0x00 on our file
-    if zero {
-        pass_sequence.push(PassType::Pattern(PATTERNS[0]));
-    }
-
     let total_passes = pass_sequence.len();
     let mut file = OpenOptions::new()
         .write(true)
@@ -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 {}/{} ({})...",
-                    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
-                );
-            }
+            show_error!(
+                "{}: pass {:2}/{} ({})...",
+                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");
+}