From a4fca2d4fc4718909658f1b0ab83734a54c8d6bc Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Thu, 2 Sep 2021 18:27:01 +0200
Subject: [PATCH] uucore/perms: remove flags in favor of enums

Part of the code was transliterated from GNU's implementation in C,
and used flags to store settings. Instead, we can use enums to avoid
magic values or binary operations to extract flags.
---
 src/uu/chown/src/chown.rs            |  1 +
 src/uucore/src/lib/features/perms.rs | 56 ++++++++++++++--------------
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs
index 20f3f8174..4abb9ac61 100644
--- a/src/uu/chown/src/chown.rs
+++ b/src/uu/chown/src/chown.rs
@@ -160,6 +160,7 @@ pub fn uu_app() -> App<'static, 'static> {
         .arg(
             Arg::with_name(options::verbosity::VERBOSE)
                 .long(options::verbosity::VERBOSE)
+                .short("v")
                 .help("output a diagnostic for every file processed"),
         )
 }
diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs
index 5f470d5a8..b605c0cdf 100644
--- a/src/uucore/src/lib/features/perms.rs
+++ b/src/uucore/src/lib/features/perms.rs
@@ -165,22 +165,26 @@ pub enum IfFrom {
     UserGroup(u32, u32),
 }
 
+#[derive(PartialEq, Eq)]
+pub enum TraverseSymlinks {
+    None,
+    First,
+    All,
+}
+
 pub struct ChownExecutor {
     pub dest_uid: Option<u32>,
     pub dest_gid: Option<u32>,
-    pub bit_flag: u8,
+    pub traverse_symlinks: TraverseSymlinks,
     pub verbosity: Verbosity,
     pub filter: IfFrom,
     pub files: Vec<String>,
     pub recursive: bool,
     pub preserve_root: bool,
+    // Must be true if traverse_symlinks is not None
     pub dereference: bool,
 }
 
-pub const FTS_COMFOLLOW: u8 = 1;
-pub const FTS_PHYSICAL: u8 = 1 << 1;
-pub const FTS_LOGICAL: u8 = 1 << 2;
-
 impl ChownExecutor {
     pub fn exec(&self) -> UResult<()> {
         let mut ret = 0;
@@ -194,9 +198,8 @@ impl ChownExecutor {
     }
 
     fn traverse<P: AsRef<Path>>(&self, root: P) -> i32 {
-        let follow_arg = self.dereference || self.bit_flag != FTS_PHYSICAL;
         let path = root.as_ref();
-        let meta = match self.obtain_meta(path, follow_arg) {
+        let meta = match self.obtain_meta(path, self.dereference) {
             Some(m) => m,
             _ => return 1,
         };
@@ -208,7 +211,7 @@ impl ChownExecutor {
         //     (argument is symlink && should follow argument && resolved to be '/')
         // )
         if self.recursive && self.preserve_root {
-            let may_exist = if follow_arg {
+            let may_exist = if self.dereference {
                 path.canonicalize().ok()
             } else {
                 let real = resolve_relative_path(path);
@@ -234,7 +237,7 @@ impl ChownExecutor {
                 &meta,
                 self.dest_uid,
                 self.dest_gid,
-                follow_arg,
+                self.dereference,
                 self.verbosity.clone(),
             ) {
                 Ok(n) => {
@@ -264,9 +267,8 @@ impl ChownExecutor {
     fn dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
         let mut ret = 0;
         let root = root.as_ref();
-        let follow = self.dereference || self.bit_flag & FTS_LOGICAL != 0;
         let mut iterator = WalkDir::new(root)
-            .follow_links(follow)
+            .follow_links(self.dereference)
             .min_depth(1)
             .into_iter();
         // We can't use a for loop because we need to manipulate the iterator inside the loop.
@@ -292,7 +294,7 @@ impl ChownExecutor {
                 Ok(entry) => entry,
             };
             let path = entry.path();
-            let meta = match self.obtain_meta(path, follow) {
+            let meta = match self.obtain_meta(path, self.dereference) {
                 Some(m) => m,
                 _ => {
                     ret = 1;
@@ -314,7 +316,7 @@ impl ChownExecutor {
                 &meta,
                 self.dest_uid,
                 self.dest_gid,
-                follow,
+                self.dereference,
                 self.verbosity.clone(),
             ) {
                 Ok(n) => {
@@ -452,31 +454,31 @@ pub fn chown_base<'a>(
     let preserve_root = matches.is_present(options::preserve_root::PRESERVE);
 
     let mut dereference = if matches.is_present(options::dereference::DEREFERENCE) {
-        1
+        Some(true)
     } else if matches.is_present(options::dereference::NO_DEREFERENCE) {
-        0
+        Some(false)
     } else {
-        -1
+        None
     };
 
-    let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) {
-        FTS_COMFOLLOW | FTS_PHYSICAL
+    let mut traverse_symlinks = if matches.is_present(options::traverse::TRAVERSE) {
+        TraverseSymlinks::First
     } else if matches.is_present(options::traverse::EVERY) {
-        FTS_LOGICAL
+        TraverseSymlinks::All
     } else {
-        FTS_PHYSICAL
+        TraverseSymlinks::None
     };
 
     let recursive = matches.is_present(options::RECURSIVE);
     if recursive {
-        if bit_flag == FTS_PHYSICAL {
-            if dereference == 1 {
+        if traverse_symlinks == TraverseSymlinks::None {
+            if dereference == Some(true) {
                 return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
             }
-            dereference = 0;
+            dereference = Some(false);
         }
     } else {
-        bit_flag = FTS_PHYSICAL;
+        traverse_symlinks = TraverseSymlinks::None;
     }
 
     let verbosity_level = if matches.is_present(options::verbosity::CHANGES) {
@@ -493,15 +495,15 @@ pub fn chown_base<'a>(
     let (dest_gid, dest_uid, filter) = parse_gid_uid_and_filter(&matches)?;
 
     let executor = ChownExecutor {
-        bit_flag,
+        traverse_symlinks,
         dest_gid,
         dest_uid,
         verbosity: Verbosity {
-            groups_only: true,
+            groups_only,
             level: verbosity_level,
         },
         recursive,
-        dereference: dereference != 0,
+        dereference: dereference.unwrap_or(true),
         preserve_root,
         files,
         filter,