Skip to content

Commit 1ffd0a0

Browse files
ChrisDrydenArcterus
authored andcommitted
rm: fix error reporting for -r on Linux fixing uutils#9011 (uutils#10111)
* rm: fix error reporting for -r on Linux * rm: add stricter assertion for error tracking test --------- Co-authored-by: Alex Lyon <dev@arct.rs>
1 parent 27be899 commit 1ffd0a0

File tree

2 files changed

+10
-9
lines changed

2 files changed

+10
-9
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
404404
let entry_stat = match dir_fd.stat_at(&entry_name, false) {
405405
Ok(stat) => stat,
406406
Err(e) => {
407-
error = handle_error_with_force(e, &entry_path, options);
407+
error |= handle_error_with_force(e, &entry_path, options);
408408
continue;
409409
}
410410
};
@@ -428,21 +428,21 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
428428
// If we can't open the subdirectory for safe traversal,
429429
// try to handle it as best we can with safe operations
430430
if e.kind() == std::io::ErrorKind::PermissionDenied {
431-
error = handle_permission_denied(
431+
error |= handle_permission_denied(
432432
dir_fd,
433433
entry_name.as_ref(),
434434
&entry_path,
435435
options,
436436
);
437437
} else {
438-
error = handle_error_with_force(e, &entry_path, options);
438+
error |= handle_error_with_force(e, &entry_path, options);
439439
}
440440
continue;
441441
}
442442
};
443443

444444
let child_error = safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options);
445-
error = error || child_error;
445+
error |= child_error;
446446

447447
// Ask user permission if needed for this subdirectory
448448
if !child_error
@@ -454,12 +454,12 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
454454

455455
// Remove the now-empty subdirectory using safe unlinkat
456456
if !child_error {
457-
error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options);
457+
error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options);
458458
}
459459
} else {
460460
// Remove file - check if user wants to remove it first
461461
if prompt_file_with_stat(&entry_path, &entry_stat, options) {
462-
error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options);
462+
error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options);
463463
}
464464
}
465465
}

tests/by-util/test_rm.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,9 +1140,10 @@ fn test_rm_directory_not_writable() {
11401140

11411141
// Check for expected error message
11421142
// When the parent directory (b/a) doesn't have write permission,
1143-
// we get "Permission denied" when trying to remove the subdirectory
1144-
let stderr = result.stderr_str();
1145-
assert!(stderr.contains("rm: cannot remove 'b/a/p': Permission denied"));
1143+
// we get "Permission denied" when trying to remove the subdirectory.
1144+
// The error tracking must be correct so we don't attempt to remove the parent
1145+
// directory after child failure (which would produce extra "Directory not empty" errors).
1146+
result.stderr_only("rm: cannot remove 'b/a/p': Permission denied\n");
11461147

11471148
// Check which directories still exist
11481149
assert!(at.dir_exists("b/a/p")); // Should still exist (parent not writable)

0 commit comments

Comments
 (0)