ls: Fix excessive ls -al syscalls for ACL metadata

src/uu/ls/src/colors.rs

@@ -168,7 +168,8 @@ pub(crate) fn color_name(
         let has_capabilities = if capabilities.is_none() {
             false
         } else {
-            uucore::fsxattr::has_acl(path.p_buf.as_path())
+            //uucore::fsxattr::has_capability(&path.p_buf)
+            path.has_capability()
         };
 
         // If the file has capabilities, use a specific style for `ca` (capabilities)

src/uu/ls/src/ls.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly nohash strtime
+// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly nohash strtime getxattr
 
 #[cfg(unix)]
 use std::collections::HashMap;
@@ -39,7 +39,9 @@ use thiserror::Error;
 #[cfg(unix)]
 use uucore::entries;
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-use uucore::fsxattr::has_acl;
+use uucore::fsxattr::{
+    POSIX_ACL_ACCESS_KEY, POSIX_ACL_DEFAULT_KEY, SET_CAPABILITY_KEY, retrieve_xattrs,
+};
 #[cfg(unix)]
 use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
 #[cfg(any(
@@ -1768,10 +1770,13 @@ pub fn uu_app() -> Command {
 /// Any data that will be reused several times makes sense to be added to this structure.
 /// Caching data here helps eliminate redundant syscalls to fetch same information.
 #[derive(Debug)]
+#[allow(unused)]
 struct PathData {
     // Result<MetaData> got from symlink_metadata() or metadata() based on config
     md: OnceCell<Option<Metadata>>,
     ft: OnceCell<Option<FileType>>,
+    #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+    xattrs: OnceCell<Option<HashMap<OsString, Vec<u8>>>>,
     // can be used to avoid reading the metadata. Can be also called d_type:
     // https://www.gnu.org/software/libc/manual/html_node/Directory-Entries.html
     de: Option<DirEntry>,
@@ -1846,6 +1851,7 @@ impl PathData {
                 OnceCell::new()
             }
         }
+
         let ft = match de {
             Some(ref de) => get_file_type(de, &p_buf, must_dereference),
             None => OnceCell::new(),
@@ -1856,6 +1862,8 @@ impl PathData {
         Self {
             md: OnceCell::new(),
             ft,
+            #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+            xattrs: OnceCell::new(),
             de,
             display_name,
             p_buf,
@@ -1909,6 +1917,36 @@ impl PathData {
             .get_or_init(|| self.get_metadata(out).map(|md| md.file_type()))
             .as_ref()
     }
+
+    #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+    fn xattrs(&self) -> Option<&HashMap<OsString, Vec<u8>>> {
+        self.xattrs
+            .get_or_init(
+                || match retrieve_xattrs(&self.p_buf, self.must_dereference) {
+                    Ok(map) => Some(map),
+                    Err(_) => fs::read_link(&self.p_buf)
+                        .ok()
+                        .and_then(|file| retrieve_xattrs(file, false).ok()),
+                },
+            )
+            .as_ref()
+    }
+
+    #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+    fn has_acl(&self) -> bool {
+        self.xattrs().as_ref().is_some_and(|map| {
+            map.contains_key(OsStr::new(POSIX_ACL_ACCESS_KEY))
+                || map.contains_key(OsStr::new(POSIX_ACL_DEFAULT_KEY))
+        })
+    }
+
+    #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+    fn has_capability(&self) -> bool {
+        // don't use exacl here, it is doing more getxattr call then needed
+        self.xattrs()
+            .as_ref()
+            .is_some_and(|map| map.contains_key(OsStr::new(SET_CAPABILITY_KEY)))
+    }
 }
 
 /// Show the directory name in the case where several arguments are given to ls
@@ -2705,15 +2743,16 @@ fn display_item_long(
     if let Some(md) = item.get_metadata(&mut state.out) {
         #[cfg(any(not(unix), target_os = "android", target_os = "macos"))]
         // TODO: See how Mac should work here
-        let is_acl_set = false;
+        let has_acl = false;
         #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-        let is_acl_set = has_acl(item.display_name.as_os_str());
+        let has_acl = { item.has_acl() };
+
         output_display.extend(display_permissions(md, true).as_bytes());
         if item.security_context.len() > 1 {
             // GNU `ls` uses a "." character to indicate a file with a security context,
             // but not other alternate access method.
             output_display.extend(b".");
-        } else if is_acl_set {
+        } else if has_acl {
             output_display.extend(b"+");
         }
         output_display.extend(b" ");

src/uu/mv/src/mv.rs

@@ -950,7 +950,7 @@ fn rename_dir_fallback(
     };
 
     #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
-    let xattrs = fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| HashMap::new());
+    let xattrs = fsxattr::retrieve_xattrs(from, false).unwrap_or_else(|_| HashMap::new());
 
     // Use directory copying (with or without hardlink support)
     let result = copy_dir_contents(

src/uucore/src/lib/features/fsxattr.rs

@@ -7,9 +7,13 @@
 
 //! Set of functions to manage xattr on files and dirs
 use std::collections::HashMap;
-use std::ffi::OsString;
+use std::ffi::{OsStr, OsString};
 use std::path::Path;
 
+pub static POSIX_ACL_ACCESS_KEY: &str = "system.posix_acl_access";
+pub static POSIX_ACL_DEFAULT_KEY: &str = "system.posix_acl_default";
+pub static SET_CAPABILITY_KEY: &str = "security.capability";
+
 /// Copies extended attributes (xattrs) from one file or directory to another.
 ///
 /// # Arguments
@@ -38,9 +42,19 @@ pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
 /// # Returns
 ///
 /// A result containing a HashMap of attributes names and values, or an error.
-pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
+pub fn retrieve_xattrs<P: AsRef<Path>>(
+    source: P,
+    must_dereference: bool,
+) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
     let mut attrs = HashMap::new();
-    for attr_name in xattr::list(&source)? {
+
+    let iter = if must_dereference {
+        xattr::list_deref(&source)?
+    } else {
+        xattr::list(&source)?
+    };
+
+    for attr_name in iter {
         if let Some(value) = xattr::get(&source, &attr_name)? {
             attrs.insert(attr_name, value);
         }
@@ -79,10 +93,32 @@ pub fn apply_xattrs<P: AsRef<Path>>(
 /// `true` if the file has extended attributes (indicating an ACL), `false` otherwise.
 pub fn has_acl<P: AsRef<Path>>(file: P) -> bool {
     // don't use exacl here, it is doing more getxattr call then needed
-    xattr::list_deref(file).is_ok_and(|acl| {
-        // if we have extra attributes, we have an acl
-        acl.count() > 0
-    })
+    xattr::get_deref(&file, OsStr::new(POSIX_ACL_ACCESS_KEY))
+        .ok()
+        .flatten()
+        .or_else(|| {
+            xattr::get_deref(&file, POSIX_ACL_DEFAULT_KEY)
+                .ok()
+                .flatten()
+        })
+        .is_some_and(|vec| !vec.is_empty())
+}
+
+/// Checks if a file has an Capability set based on its extended attributes.
+///
+/// # Arguments
+///
+/// * `file` - A reference to the path of the file.
+///
+/// # Returns
+///
+/// `true` if the file has a capability extended attribute, `false` otherwise.
+pub fn has_capability<P: AsRef<Path>>(file: P) -> bool {
+    // don't use exacl here, it is doing more getxattr call then needed
+    xattr::get_deref(&file, OsStr::new(SET_CAPABILITY_KEY))
+        .ok()
+        .flatten()
+        .is_some_and(|vec| !vec.is_empty())
 }
 
 /// Returns the permissions bits of a file or directory which has Access Control List (ACL) entries based on its
@@ -101,9 +137,9 @@ pub fn get_acl_perm_bits_from_xattr<P: AsRef<Path>>(source: P) -> u32 {
 
     // Only default acl entries get inherited by objects under the path i.e. if child directories
     // will have their permissions modified.
-    if let Ok(entries) = retrieve_xattrs(source) {
+    if let Ok(entries) = retrieve_xattrs(source, true) {
         let mut perm: u32 = 0;
-        if let Some(value) = entries.get(&OsString::from("system.posix_acl_default")) {
+        if let Some(value) = entries.get(OsStr::new(POSIX_ACL_DEFAULT_KEY)) {
             // value is xattr byte vector
             // value follows a starts with a 4 byte header, and then has posix_acl_entries, each
             // posix_acl_entry is separated by a u32 sequence i.e. 0xFFFFFFFF
@@ -177,7 +213,7 @@ mod tests {
         test_xattrs.insert(OsString::from(test_attr), test_value.to_vec());
         apply_xattrs(&file_path, test_xattrs).unwrap();
 
-        let retrieved_xattrs = retrieve_xattrs(&file_path).unwrap();
+        let retrieved_xattrs = retrieve_xattrs(&file_path, true).unwrap();
         assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str()));
         assert_eq!(
             retrieved_xattrs
@@ -242,9 +278,12 @@ mod tests {
 
         assert!(!has_acl(&file_path));
 
-        let test_attr = "user.test_acl";
-        let test_value = b"test value";
-        xattr::set(&file_path, test_attr, test_value).unwrap();
+        let test_attr = "system.posix_acl_access";
+        let test_value = "invalid_test_value";
+        // perhaps can't set actual ACL in test environment? if so, return early
+        let Ok(_) = xattr::set(&file_path, test_attr, test_value.as_bytes()) else {
+            return;
+        };
 
         assert!(has_acl(&file_path));
     }