From f18e8983b189ed62fb381c3c3ce808281daae6d2 Mon Sep 17 00:00:00 2001 From: terade <134976752+terade@users.noreply.github.com.> Date: Wed, 4 Oct 2023 16:32:30 +0200 Subject: [PATCH 1/2] rm: Refactor prompt_file, lower nesting depth Addressing issue #5345. Introduce new helper function: prompt_file_permission_read_only --- src/uu/rm/src/rm.rs | 61 +++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 87767b904..0f4d04598 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -486,7 +486,6 @@ fn prompt_dir(path: &Path, options: &Options) -> bool { } } -#[allow(clippy::cognitive_complexity)] fn prompt_file(path: &Path, options: &Options) -> bool { // If interactive is Never we never want to send prompts if options.interactive == InteractiveMode::Never { @@ -503,47 +502,43 @@ fn prompt_file(path: &Path, options: &Options) -> bool { // File::open(path) doesn't open the file in write mode so we need to use file options to open it in also write mode to check if it can written too match File::options().read(true).write(true).open(path) { Ok(file) => { - if let Ok(metadata) = file.metadata() { - if metadata.permissions().readonly() { - if metadata.len() == 0 { - prompt_yes!( - "remove write-protected regular empty file {}?", - path.quote() - ) - } else { - prompt_yes!("remove write-protected regular file {}?", path.quote()) - } - } else if options.interactive == InteractiveMode::Always { - if metadata.len() == 0 { - prompt_yes!("remove regular empty file {}?", path.quote()) - } else { - prompt_yes!("remove file {}?", path.quote()) - } + let Ok(metadata) = file.metadata() else { + return true; + }; + + if options.interactive == InteractiveMode::Always && !metadata.permissions().readonly() + { + return if metadata.len() == 0 { + prompt_yes!("remove regular empty file {}?", path.quote()) } else { - true - } - } else { - true + prompt_yes!("remove file {}?", path.quote()) + }; } + prompt_file_permission_readonly(path, Ok(metadata)) } Err(err) => { - if err.kind() == ErrorKind::PermissionDenied { - match fs::metadata(path) { - Ok(metadata) if metadata.len() == 0 => { - prompt_yes!( - "remove write-protected regular empty file {}?", - path.quote() - ) - } - _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), - } - } else { - true + if err.kind() != ErrorKind::PermissionDenied { + return true; } + prompt_file_permission_readonly(path, fs::metadata(path)) } } } +fn prompt_file_permission_readonly( + path: &Path, + metadata_or_err: Result, +) -> bool { + match metadata_or_err { + Ok(metadata) if !metadata.permissions().readonly() => true, + Ok(metadata) if metadata.len() == 0 => prompt_yes!( + "remove write-protected regular empty file {}?", + path.quote() + ), + _ => prompt_yes!("remove write-protected regular file {}?", path.quote()), + } +} + // For directories finding if they are writable or not is a hassle. In Unix we can use the built-in rust crate to to check mode bits. But other os don't have something similar afaik // Most cases are covered by keep eye out for edge cases #[cfg(unix)] From b6a6a4dc6506db94ec7dc4702314c78bff3be0ab Mon Sep 17 00:00:00 2001 From: terade <134976752+terade@users.noreply.github.com.> Date: Tue, 17 Oct 2023 17:26:27 +0200 Subject: [PATCH 2/2] rm: apply suggestion of retrieving metatada in function --- src/uu/rm/src/rm.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 0f4d04598..43a7ec774 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -514,22 +514,18 @@ fn prompt_file(path: &Path, options: &Options) -> bool { prompt_yes!("remove file {}?", path.quote()) }; } - prompt_file_permission_readonly(path, Ok(metadata)) } Err(err) => { if err.kind() != ErrorKind::PermissionDenied { return true; } - prompt_file_permission_readonly(path, fs::metadata(path)) } } + prompt_file_permission_readonly(path) } -fn prompt_file_permission_readonly( - path: &Path, - metadata_or_err: Result, -) -> bool { - match metadata_or_err { +fn prompt_file_permission_readonly(path: &Path) -> bool { + match fs::metadata(path) { Ok(metadata) if !metadata.permissions().readonly() => true, Ok(metadata) if metadata.len() == 0 => prompt_yes!( "remove write-protected regular empty file {}?",