Skip to content

Commit f1c3436

Browse files
committed
refactor(rm): extract one-fs and root preservation checks to helper
Introduce `check_and_report_one_fs` to encapsulate the logic for verifying filesystem boundaries (`-x`) and root preservation. This removes code duplication between `remove_dir_recursive` and `handle_dir`, ensuring consistent error reporting for "different device" skips and `--preserve-root=all` violations.
1 parent 2617af7 commit f1c3436

1 file changed

Lines changed: 23 additions & 21 deletions

File tree

src/uu/rm/src/rm.rs

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,27 @@ fn is_writable_metadata(_metadata: &Metadata) -> bool {
586586
true
587587
}
588588

589+
/// Helper function to check fs and report errors if necessary.
590+
/// Returns true if the operation should be skipped/returned (i.e., on error).
591+
fn check_and_report_one_fs(path: &Path, options: &Options) -> bool {
592+
if let Err(additional_reason) = check_one_fs(path, options) {
593+
if !additional_reason.is_empty() {
594+
show_error!("{}", additional_reason);
595+
}
596+
show_error!(
597+
"skipping {}, since it's on a different device",
598+
path.quote()
599+
);
600+
601+
if options.preserve_root == PreserveRoot::YesAll {
602+
show_error!("and --preserve-root=all is in effect");
603+
}
604+
605+
return true;
606+
}
607+
false
608+
}
609+
589610
/// Recursively remove the directory tree rooted at the given path.
590611
///
591612
/// If `path` is a file or a symbolic link, just remove it. If it is a
@@ -608,14 +629,7 @@ fn remove_dir_recursive(
608629
}
609630

610631
// Base case 2: check if a path is on the same file system
611-
if let Err(additional_reason) = check_one_fs(path, options) {
612-
if !additional_reason.is_empty() {
613-
show_error!("{}", additional_reason);
614-
}
615-
show_error!(
616-
"skipping {}, since it's on a different device",
617-
path.quote()
618-
);
632+
if check_and_report_one_fs(path, options) {
619633
return true;
620634
}
621635

@@ -772,19 +786,7 @@ fn check_one_fs(path: &Path, options: &Options) -> Result<(), String> {
772786
fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
773787
let mut had_err = false;
774788

775-
if let Err(additional_reason) = check_one_fs(path, options) {
776-
if !additional_reason.is_empty() {
777-
show_error!("{}", additional_reason);
778-
}
779-
show_error!(
780-
"skipping {}, since it's on a different device",
781-
path.quote()
782-
);
783-
784-
if options.preserve_root == PreserveRoot::YesAll {
785-
show_error!("and --preserve-root=all is in effect");
786-
}
787-
789+
if check_and_report_one_fs(path, options) {
788790
return true;
789791
}
790792

0 commit comments

Comments
 (0)