Repository navigation
shred: keep going when a rename fails during --remove - #16044
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
Opened #16073 for this and updated the description. |
| @@ -843,7 +843,7 @@ 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 | |||
There was a problem hiding this comment.
please update this doc, it never returns None and now stops at the first failed rename
| // TODO: replace with our error management | ||
| std::process::exit(1); | ||
| } | ||
| Err(_) => return Ok(last_path), |
There was a problem hiding this comment.
maybe add a short comment why the error is ignored here (the unlink reports it)
|
|
||
| #[test] | ||
| #[cfg(unix)] | ||
| fn test_shred_remove_in_unwritable_dir_keeps_going() { |
There was a problem hiding this comment.
almost the same as test_shred_fail_no_perm above, could you just add the second operand there? :)
A failed rename made shred exit immediately, so with -u a file in a directory without write permission aborted the run and later operands were never processed. Stop renaming at the first failure and let the unlink report the error as "failed to remove: <error>".
129510c to
fae3152
Compare
|
Thanks for the review. I updated the wipe_name doc comment and added a short note on why the rename error is ignored, since the unlink reports it. I also dropped the separate test and added a second operand to test_shred_fail_no_perm instead, which now checks that the first file stays and the second one gets removed. |
|
Thanks for your PR |
Closes #16073
When
shred -ucouldn't rename a file, for example in an unwritable directory, it printed "Couldn't rename to ..." and exited, so the file was never unlinked and later operands were skipped.Now renaming stops at the first failure and the last name that worked is unlinked. If that fails too, the error is
shred: shr/f: failed to remove: Permission deniedand the run goes on with the next operand, still exiting 1. The WipeSync path returns the I/O error instead of panicking. The unusedshred-couldnt-renamestring is removed andfailed to remove fileis nowfailed to removein en-US and fr-FR. Two existing tests that checked the old rename message are updated.The new test
test_shred_remove_in_unwritable_dir_keeps_goingfails on main and passes with this change.