uucore: refactor - reduce duplicate code related to fs::display_permissions

This is a refactor to reduce duplicate code, it affects chmod/ls/stat.
* merge `stat/src/fsext::pretty_access` into `uucore/src/lib/feature/fs::display_permissions_unix`
* move tests for `fs::display_permissions` from `test_stat::test_access` to `uucore/src/lib/features/fs::test_display_permissions`
* adjust `uu_chmod`, `uu_ls` and `uu_stat` to use `uucore::fs::display_permissions`

src/uu/chmod/src/chmod.rs

@@ -15,6 +15,7 @@ use std::fs;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
 use std::path::Path;
 use uucore::fs::display_permissions_unix;
+use uucore::libc::mode_t;
 #[cfg(not(windows))]
 use uucore::mode;
 use uucore::InvalidEncodingHandling;
@@ -306,7 +307,7 @@ impl Chmoder {
                     "mode of '{}' retained as {:04o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                 );
             }
             Ok(())
@@ -319,9 +320,9 @@ impl Chmoder {
                     "failed to change mode of file '{}' from {:o} ({}) to {:o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                     mode,
-                    display_permissions_unix(mode)
+                    display_permissions_unix(mode as mode_t, false)
                 );
             }
             Err(1)
@@ -331,9 +332,9 @@ impl Chmoder {
                     "mode of '{}' changed from {:o} ({}) to {:o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                     mode,
-                    display_permissions_unix(mode)
+                    display_permissions_unix(mode as mode_t, false)
                 );
             }
             Ok(())

src/uu/ls/src/ls.rs

@@ -1480,9 +1480,8 @@ fn display_item_long(
 
     let _ = write!(
         out,
-        "{}{} {}",
+        "{} {}",
-        display_file_type(md.file_type()),
+        display_permissions(&md, true),
-        display_permissions(&md),
         pad_left(display_symlink_count(&md), max_links),
     );
 
@@ -1668,16 +1667,6 @@ fn display_size(len: u64, config: &Config) -> String {
     }
 }
 
-fn display_file_type(file_type: FileType) -> char {
-    if file_type.is_dir() {
-        'd'
-    } else if file_type.is_symlink() {
-        'l'
-    } else {
-        '-'
-    }
-}
-
 #[cfg(unix)]
 fn file_is_executable(md: &Metadata) -> bool {
     // Mode always returns u32, but the flags might not be, based on the platform

src/uu/stat/Cargo.toml

@@ -17,7 +17,7 @@ path = "src/stat.rs"
 [dependencies]
 clap = "2.33"
 time = "0.1.40"
-uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "libc"] }
+uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "libc", "fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 
 [[bin]]

src/uu/stat/src/fsext.rs

@@ -41,13 +41,6 @@ impl BirthTime for Metadata {
     }
 }
 
-#[macro_export]
-macro_rules! has {
-    ($mode:expr, $perm:expr) => {
-        $mode & $perm != 0
-    };
-}
-
 pub fn pretty_time(sec: i64, nsec: i64) -> String {
     // sec == seconds since UNIX_EPOCH
     // nsec == nanoseconds since (UNIX_EPOCH + sec)
@@ -81,65 +74,6 @@ pub fn pretty_filetype<'a>(mode: mode_t, size: u64) -> &'a str {
     }
 }
 
-pub fn pretty_access(mode: mode_t) -> String {
-    let mut result = String::with_capacity(10);
-    result.push(match mode & S_IFMT {
-        S_IFDIR => 'd',
-        S_IFCHR => 'c',
-        S_IFBLK => 'b',
-        S_IFREG => '-',
-        S_IFIFO => 'p',
-        S_IFLNK => 'l',
-        S_IFSOCK => 's',
-        // TODO: Other file types
-        _ => '?',
-    });
-
-    result.push(if has!(mode, S_IRUSR) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWUSR) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISUID as mode_t) {
-        if has!(mode, S_IXUSR) {
-            's'
-        } else {
-            'S'
-        }
-    } else if has!(mode, S_IXUSR) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result.push(if has!(mode, S_IRGRP) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWGRP) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISGID as mode_t) {
-        if has!(mode, S_IXGRP) {
-            's'
-        } else {
-            'S'
-        }
-    } else if has!(mode, S_IXGRP) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result.push(if has!(mode, S_IROTH) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWOTH) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISVTX as mode_t) {
-        if has!(mode, S_IXOTH) {
-            't'
-        } else {
-            'T'
-        }
-    } else if has!(mode, S_IXOTH) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result
-}
-
 use std::borrow::Cow;
 use std::convert::{AsRef, From};
 use std::ffi::CString;

src/uu/stat/src/stat.rs

@@ -7,13 +7,13 @@
 
 // spell-checker:ignore (ToDO) mtab fsext showfs otype fmtstr prec ftype blocksize nlink rdev fnodes fsid namelen blksize inodes fstype iosize statfs gnulib NBLOCKSIZE
 
-#[macro_use]
 mod fsext;
 pub use crate::fsext::*;
 
 #[macro_use]
 extern crate uucore;
 use uucore::entries;
+use uucore::fs::display_permissions;
 
 use clap::{App, Arg, ArgMatches};
 use std::borrow::Cow;
@@ -575,7 +575,7 @@ impl Stater {
                                     }
                                     // access rights in human readable form
                                     'A' => {
-                                        arg = pretty_access(meta.mode() as mode_t);
+                                        arg = display_permissions(&meta, true);
                                         otype = OutputType::Str;
                                     }
                                     // number of blocks allocated (see %B)

src/uucore/src/lib/features/fs.rs

@@ -8,8 +8,9 @@
 
 #[cfg(unix)]
 use libc::{
-    mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_ISGID, S_ISUID, S_ISVTX, S_IWGRP, S_IWOTH, S_IWUSR,
+    mode_t, S_IFBLK, S_IFCHR, S_IFDIR, S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK, S_IRGRP,
-    S_IXGRP, S_IXOTH, S_IXUSR,
+    S_IROTH, S_IRUSR, S_ISGID, S_ISUID, S_ISVTX, S_IWGRP, S_IWOTH, S_IWUSR, S_IXGRP, S_IXOTH,
+    S_IXUSR,
 };
 use std::borrow::Cow;
 use std::env;
@@ -23,9 +24,10 @@ use std::os::unix::fs::MetadataExt;
 use std::path::{Component, Path, PathBuf};
 
 #[cfg(unix)]
+#[macro_export]
 macro_rules! has {
     ($mode:expr, $perm:expr) => {
-        $mode & ($perm as u32) != 0
+        $mode & $perm != 0
     };
 }
 
@@ -240,22 +242,42 @@ pub fn is_stderr_interactive() -> bool {
 
 #[cfg(not(unix))]
 #[allow(unused_variables)]
-pub fn display_permissions(metadata: &fs::Metadata) -> String {
+pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> String {
+    if display_file_type {
+        return String::from("----------");
+    }
     String::from("---------")
 }
 
 #[cfg(unix)]
-pub fn display_permissions(metadata: &fs::Metadata) -> String {
+pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> String {
     let mode: mode_t = metadata.mode() as mode_t;
-    display_permissions_unix(mode as u32)
+    display_permissions_unix(mode, display_file_type)
 }
 
 #[cfg(unix)]
-pub fn display_permissions_unix(mode: u32) -> String {
+pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String {
-    let mut result = String::with_capacity(9);
+    let mut result;
+    if display_file_type {
+        result = String::with_capacity(10);
+        result.push(match mode & S_IFMT {
+            S_IFDIR => 'd',
+            S_IFCHR => 'c',
+            S_IFBLK => 'b',
+            S_IFREG => '-',
+            S_IFIFO => 'p',
+            S_IFLNK => 'l',
+            S_IFSOCK => 's',
+            // TODO: Other file types
+            _ => '?',
+        });
+    } else {
+        result = String::with_capacity(9);
+    }
+
     result.push(if has!(mode, S_IRUSR) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWUSR) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISUID) {
+    result.push(if has!(mode, S_ISUID as mode_t) {
         if has!(mode, S_IXUSR) {
             's'
         } else {
@@ -269,7 +291,7 @@ pub fn display_permissions_unix(mode: u32) -> String {
 
     result.push(if has!(mode, S_IRGRP) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWGRP) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISGID) {
+    result.push(if has!(mode, S_ISGID as mode_t) {
         if has!(mode, S_IXGRP) {
             's'
         } else {
@@ -283,7 +305,7 @@ pub fn display_permissions_unix(mode: u32) -> String {
 
     result.push(if has!(mode, S_IROTH) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWOTH) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISVTX) {
+    result.push(if has!(mode, S_ISVTX as mode_t) {
         if has!(mode, S_IXOTH) {
             't'
         } else {
@@ -355,4 +377,57 @@ mod tests {
             );
         }
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_display_permissions() {
+        assert_eq!(
+            "drwxr-xr-x",
+            display_permissions_unix(S_IFDIR | 0o755, true)
+        );
+        assert_eq!(
+            "rwxr-xr-x",
+            display_permissions_unix(S_IFDIR | 0o755, false)
+        );
+        assert_eq!(
+            "-rw-r--r--",
+            display_permissions_unix(S_IFREG | 0o644, true)
+        );
+        assert_eq!(
+            "srw-r-----",
+            display_permissions_unix(S_IFSOCK | 0o640, true)
+        );
+        assert_eq!(
+            "lrw-r-xr-x",
+            display_permissions_unix(S_IFLNK | 0o655, true)
+        );
+        assert_eq!("?rw-r-xr-x", display_permissions_unix(0o655, true));
+
+        assert_eq!(
+            "brwSr-xr-x",
+            display_permissions_unix(S_IFBLK | S_ISUID as mode_t | 0o655, true)
+        );
+        assert_eq!(
+            "brwsr-xr-x",
+            display_permissions_unix(S_IFBLK | S_ISUID as mode_t | 0o755, true)
+        );
+
+        assert_eq!(
+            "prw---sr--",
+            display_permissions_unix(S_IFIFO | S_ISGID as mode_t | 0o614, true)
+        );
+        assert_eq!(
+            "prw---Sr--",
+            display_permissions_unix(S_IFIFO | S_ISGID as mode_t | 0o604, true)
+        );
+
+        assert_eq!(
+            "c---r-xr-t",
+            display_permissions_unix(S_IFCHR | S_ISVTX as mode_t | 0o055, true)
+        );
+        assert_eq!(
+            "c---r-xr-T",
+            display_permissions_unix(S_IFCHR | S_ISVTX as mode_t | 0o054, true)
+        );
+    }
 }

tests/by-util/test_stat.rs

@@ -9,42 +9,6 @@ pub use self::stat::*;
 mod test_fsext {
     use super::*;
 
-    #[test]
-    fn test_access() {
-        assert_eq!("drwxr-xr-x", pretty_access(S_IFDIR | 0o755));
-        assert_eq!("-rw-r--r--", pretty_access(S_IFREG | 0o644));
-        assert_eq!("srw-r-----", pretty_access(S_IFSOCK | 0o640));
-        assert_eq!("lrw-r-xr-x", pretty_access(S_IFLNK | 0o655));
-        assert_eq!("?rw-r-xr-x", pretty_access(0o655));
-
-        assert_eq!(
-            "brwSr-xr-x",
-            pretty_access(S_IFBLK | S_ISUID as mode_t | 0o655)
-        );
-        assert_eq!(
-            "brwsr-xr-x",
-            pretty_access(S_IFBLK | S_ISUID as mode_t | 0o755)
-        );
-
-        assert_eq!(
-            "prw---sr--",
-            pretty_access(S_IFIFO | S_ISGID as mode_t | 0o614)
-        );
-        assert_eq!(
-            "prw---Sr--",
-            pretty_access(S_IFIFO | S_ISGID as mode_t | 0o604)
-        );
-
-        assert_eq!(
-            "c---r-xr-t",
-            pretty_access(S_IFCHR | S_ISVTX as mode_t | 0o055)
-        );
-        assert_eq!(
-            "c---r-xr-T",
-            pretty_access(S_IFCHR | S_ISVTX as mode_t | 0o054)
-        );
-    }
-
     #[test]
     fn test_file_type() {
         assert_eq!("block special file", pretty_filetype(S_IFBLK, 0));