From 1a0f41e4ccbe59f7ddfbd003dffbf1d3e28a2cda Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Mon, 30 Dec 2024 14:35:36 +0100
Subject: [PATCH] chmod/chown/chgrp: deduplicate some arg management

---
 src/uu/chmod/src/chmod.rs            | 35 +-------------
 src/uucore/src/lib/features/perms.rs | 68 ++++++++++++++++------------
 2 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs
index c51de50f9..8da715e39 100644
--- a/src/uu/chmod/src/chmod.rs
+++ b/src/uu/chmod/src/chmod.rs
@@ -16,7 +16,7 @@ use uucore::fs::display_permissions_unix;
 use uucore::libc::mode_t;
 #[cfg(not(windows))]
 use uucore::mode;
-use uucore::perms::TraverseSymlinks;
+use uucore::perms::{configure_symlink_and_recursion, TraverseSymlinks};
 use uucore::{format_usage, help_about, help_section, help_usage, show, show_error};
 
 const ABOUT: &str = help_about!("chmod.md");
@@ -34,11 +34,6 @@ mod options {
     pub const RECURSIVE: &str = "recursive";
     pub const MODE: &str = "MODE";
     pub const FILE: &str = "FILE";
-    // TODO remove duplication with perms.rs
-    pub mod dereference {
-        pub const DEREFERENCE: &str = "dereference";
-        pub const NO_DEREFERENCE: &str = "no-dereference";
-    }
 }
 
 /// Extract negative modes (starting with '-') from the rest of the arguments.
@@ -143,33 +138,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         return Err(UUsageError::new(1, "missing operand".to_string()));
     }
 
-    let mut dereference = if matches.get_flag(options::dereference::DEREFERENCE) {
-        Some(true) // Follow symlinks
-    } else if matches.get_flag(options::dereference::NO_DEREFERENCE) {
-        Some(false) // Do not follow symlinks
-    } else {
-        None // Default behavior
-    };
-
-    let mut traverse_symlinks = if matches.get_flag("L") {
-        TraverseSymlinks::All
-    } else if matches.get_flag("H") {
-        TraverseSymlinks::First
-    } else {
-        TraverseSymlinks::None
-    };
-
-    let recursive = matches.get_flag(options::RECURSIVE);
-    if recursive {
-        if traverse_symlinks == TraverseSymlinks::None {
-            if dereference == Some(true) {
-                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-            }
-            dereference = Some(false);
-        }
-    } else {
-        traverse_symlinks = TraverseSymlinks::None;
-    }
+    let (recursive, dereference, traverse_symlinks) = configure_symlink_and_recursion(&matches)?;
 
     let chmoder = Chmoder {
         changes,
diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs
index 73b84be72..dcb797341 100644
--- a/src/uucore/src/lib/features/perms.rs
+++ b/src/uucore/src/lib/features/perms.rs
@@ -516,6 +516,45 @@ pub struct GidUidOwnerFilter {
 }
 type GidUidFilterOwnerParser = fn(&ArgMatches) -> UResult<GidUidOwnerFilter>;
 
+/// Determines symbolic link traversal and recursion settings based on flags.
+/// Returns the updated `dereference` and `traverse_symlinks` values.
+pub fn configure_symlink_and_recursion(
+    matches: &ArgMatches,
+) -> Result<(bool, Option<bool>, TraverseSymlinks), Box<dyn crate::error::UError>> {
+    let mut dereference = if matches.get_flag(options::dereference::DEREFERENCE) {
+        Some(true) // Follow symlinks
+    } else if matches.get_flag(options::dereference::NO_DEREFERENCE) {
+        Some(false) // Do not follow symlinks
+    } else {
+        None // Default behavior
+    };
+
+    let mut traverse_symlinks = if matches.get_flag("L") {
+        TraverseSymlinks::All
+    } else if matches.get_flag("H") {
+        TraverseSymlinks::First
+    } else {
+        TraverseSymlinks::None
+    };
+
+    let recursive = matches.get_flag(options::RECURSIVE);
+    if recursive {
+        if traverse_symlinks == TraverseSymlinks::None {
+            if dereference == Some(true) {
+                return Err(USimpleError::new(
+                    1,
+                    "-R --dereference requires -H or -L".to_string(),
+                ));
+            }
+            dereference = Some(false);
+        }
+    } else {
+        traverse_symlinks = TraverseSymlinks::None;
+    }
+
+    Ok((recursive, dereference, traverse_symlinks))
+}
+
 /// Base implementation for `chgrp` and `chown`.
 ///
 /// An argument called `add_arg_if_not_reference` will be added to `command` if
@@ -571,34 +610,7 @@ pub fn chown_base(
         .unwrap_or_default();
 
     let preserve_root = matches.get_flag(options::preserve_root::PRESERVE);
-
-    let mut dereference = if matches.get_flag(options::dereference::DEREFERENCE) {
-        Some(true)
-    } else if matches.get_flag(options::dereference::NO_DEREFERENCE) {
-        Some(false)
-    } else {
-        None
-    };
-
-    let mut traverse_symlinks = if matches.get_flag(options::traverse::TRAVERSE) {
-        TraverseSymlinks::First
-    } else if matches.get_flag(options::traverse::EVERY) {
-        TraverseSymlinks::All
-    } else {
-        TraverseSymlinks::None
-    };
-
-    let recursive = matches.get_flag(options::RECURSIVE);
-    if recursive {
-        if traverse_symlinks == TraverseSymlinks::None {
-            if dereference == Some(true) {
-                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-            }
-            dereference = Some(false);
-        }
-    } else {
-        traverse_symlinks = TraverseSymlinks::None;
-    }
+    let (recursive, dereference, traverse_symlinks) = configure_symlink_and_recursion(&matches)?;
 
     let verbosity_level = if matches.get_flag(options::verbosity::CHANGES) {
         VerbosityLevel::Changes