]> Savannah Git Hosting - gnulib.git/commitdiff
file-has-acl: no need for struct stat
authorPaul Eggert <eggert@cs.ucla.edu>
Thu, 3 Oct 2024 00:40:34 +0000 (17:40 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Thu, 3 Oct 2024 00:41:28 +0000 (17:41 -0700)
Change the API of the new file_has_aclinfo function so that it no
longer needs a struct stat *.  In some cases this can help GNU ls
avoid an unnecessary ‘stat’ call for each file it lists, which
can be a significant win.
* lib/acl.h (ACL_SYMLINK_NOFOLLOW): Change from 1 to UCHAR_MAX+1
so that it can be ORed with any d_type value.
* lib/file-has-acl.c: Include dirent.h, for the DT_* macros.
Check that ACL_SYMLINK_NOFOLLOW is outside unsigned char range.
(file_has_aclinfo): Change API so that struct stat is no longer
needed.  Instead, the file type (if known) is passed as part of
the flags.  All callers changed.  Simplify NFSv4 code.
* modules/file-has-acl (Depends-on): Add assert-h for static_assert,
and dirent for DT_* macros.
* tests/test-file-has-acl.c: Include dirent.h.
(main): Adjust to file_has_aclinfo API change.
Briefly test ACL_SYMLINK_FOLLOW.

ChangeLog
NEWS
lib/acl.h
lib/file-has-acl.c
modules/file-has-acl
tests/test-file-has-acl.c

index fc6d4002efa3fb8bb690ca13c6babdfe9a51ddc9..d3311a6ea82b087e136961363043f145aca14efd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,23 @@
 2024-10-02  Paul Eggert  <eggert@cs.ucla.edu>
 
+       file-has-acl: no need for struct stat
+       Change the API of the new file_has_aclinfo function so that it no
+       longer needs a struct stat *.  In some cases this can help GNU ls
+       avoid an unnecessary ‘stat’ call for each file it lists, which
+       can be a significant win.
+       * lib/acl.h (ACL_SYMLINK_NOFOLLOW): Change from 1 to UCHAR_MAX+1
+       so that it can be ORed with any d_type value.
+       * lib/file-has-acl.c: Include dirent.h, for the DT_* macros.
+       Check that ACL_SYMLINK_NOFOLLOW is outside unsigned char range.
+       (file_has_aclinfo): Change API so that struct stat is no longer
+       needed.  Instead, the file type (if known) is passed as part of
+       the flags.  All callers changed.  Simplify NFSv4 code.
+       * modules/file-has-acl (Depends-on): Add assert-h for static_assert,
+       and dirent for DT_* macros.
+       * tests/test-file-has-acl.c: Include dirent.h.
+       (main): Adjust to file_has_aclinfo API change.
+       Briefly test ACL_SYMLINK_FOLLOW.
+
        dirent: define DT_* macros on all platforms
        * lib/dirent.in.h (DT_UNKNOWN, DT_FIFO, DT_CHR, DT_DIR, DT_BLK)
        (DT_REG, DT_LNK, DT_SOCK, DT_WHT): Define these on all platforms,
diff --git a/NEWS b/NEWS
index c3710825dd058eaf879d8a1bc5e1f4678094195e..bd120be0b3a436d61636067db82eedb6d04e09b4 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,9 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2024-10-02  file-has-acl    The file_has_aclinfo function introduced 3 days ago
+                            now has a different signature.
+
 2024-09-25  string-buffer   The function sb_append is renamed to sb_append_c.
                             The function sb_dupfree is renamed to sb_dupfree_c.
 
index 1a627323ec91d96a0e88bf50a3e0471f78d4c6f5..29ec2496fa45bf7064e551ea7565a49a3090f96c 100644 (file)
--- a/lib/acl.h
+++ b/lib/acl.h
@@ -32,8 +32,9 @@
 extern "C" {
 #endif
 
-/* Follow symlinks when getting an ACL.  */
-enum { ACL_SYMLINK_FOLLOW = 1 };
+/* Follow symlinks when getting an ACL.  This is greater than any
+   unsigned char so that it can be ORed with any <dirent.h> DT_* value.  */
+enum { ACL_SYMLINK_FOLLOW = 1 + (unsigned char) -1 };
 
 /* Information about an ACL.  */
 struct aclinfo
@@ -75,7 +76,7 @@ struct aclinfo
 
 bool acl_errno_valid (int) _GL_ATTRIBUTE_CONST;
 int file_has_acl (char const *, struct stat const *);
-int file_has_aclinfo (char const *, struct stat const *, struct aclinfo *, int);
+int file_has_aclinfo (char const *restrict, struct aclinfo *restrict, int);
 
 #if USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR
 bool aclinfo_has_xattr (struct aclinfo const *, char const *)
index f29414de677c7196e7e557e35188245029ff67e0..9f4213702fb99110777f5082e255c5793b014ec8 100644 (file)
 
 #include "acl.h"
 
+#include <dirent.h>
+
 #include "acl-internal.h"
 #include "attribute.h"
 #include "minmax.h"
 
+/* Check the assumption that UCHAR_MAX < INT_MAX.  */
+static_assert (ACL_SYMLINK_FOLLOW & ~ (unsigned char) -1);
+
 static char const UNKNOWN_SECURITY_CONTEXT[] = "?";
 
 #if USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR
@@ -321,16 +326,18 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
    0 if ACLs are not supported, or if NAME has no or only a base ACL,
    and -1 (setting errno) on error.  Note callers can determine
    if ACLs are not supported as errno is set in that case also.
-   SB must be set to the stat buffer of NAME,
-   obtained through stat() or lstat().
-   Set *AI to the ACL info if available.
-   If FLAGS & AT_SYMLINK_FOLLOW, SB was gotten via stat, otherwise lstat.
-   Also, if FLAGS & AT_SYMLINK_FOLLOW, follow symlinks when retrieving
-   ACL info, otherwise do not follow them if possible.  */
+   Set *AI to ACL info regardless of return value.
+   FLAGS should be a <dirent.h> d_type value, optionally ORed with
+   AT_SYMLINK_FOLLOW; if the d_type value is not known,
+   use DT_UNKNOWN though this may be less efficient.
+   If FLAGS & AT_SYMLINK_FOLLOW, follow symlinks when retrieving ACL info;
+   otherwise do not follow them if possible.  */
 int
-file_has_aclinfo (char const *name, struct stat const *sb,
-                  struct aclinfo *ai, int flags)
+file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
+                  struct aclinfo *restrict ai, int flags)
 {
+  unsigned char d_type = flags & UCHAR_MAX;
+
   /* Symbolic links lack extended attributes and ACLs on all supported
      platforms, so don't bother trying to fetch them.  If the platform
      supports not following symlinks this defends against some races
@@ -338,7 +345,7 @@ file_has_aclinfo (char const *name, struct stat const *sb,
 
      First, initialize *AI for cases known to be unsupported.  */
 
-  if (!USE_LINUX_XATTR || S_ISLNK (sb->st_mode))
+  if (!USE_LINUX_XATTR || d_type == DT_LNK)
     {
       ai->buf = ai->u.__gl_acl_ch;
       ai->size = -1;
@@ -350,7 +357,7 @@ file_has_aclinfo (char const *name, struct stat const *sb,
   /* Now set *AI for cases that might be supported, then check for ACLs.  */
 
 #if USE_ACL
-  if (! S_ISLNK (sb->st_mode))
+  if (d_type != DT_LNK)
     {
 
 # if USE_LINUX_XATTR
@@ -368,45 +375,41 @@ file_has_aclinfo (char const *name, struct stat const *sb,
              but if it has an NFSv4 ACL that's the one that matters.
              In earlier Fedora the two types of ACLs were mutually exclusive.
              Attempt to work correctly on both kinds of systems.  */
-          bool nfsv4_acl = aclinfo_has_xattr (ai, XATTR_NAME_NFSV4_ACL);
-          int ret
-            = (ai->size <= 0 ? ai->size
-               : (nfsv4_acl
-                  || aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_ACCESS)
-                  || (S_ISDIR (sb->st_mode)
-                      && aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_DEFAULT))));
-
-          /* If there is an NFSv4 ACL, check whether it is nontrivial.  */
-          if (nfsv4_acl)
-            {
-              /* A buffer large enough to hold any trivial NFSv4 ACL.
-                 The max length of a trivial NFSv4 ACL is 6 words for owner,
-                 6 for group, 7 for everyone, all times 2 because there are both
-                 allow and deny ACEs.  There are 6 words for owner because of
-                 type, flag, mask, wholen, "OWNER@"+pad and similarly for group;
-                 everyone is another word to hold "EVERYONE@".  */
-              uint32_t buf[2 * (6 + 6 + 7)];
-
-              ret = ((flags & ACL_SYMLINK_FOLLOW ? getxattr : lgetxattr)
+
+          if (!aclinfo_has_xattr (ai, XATTR_NAME_NFSV4_ACL))
+            return
+              (aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_ACCESS)
+               || ((d_type == DT_DIR || d_type == DT_UNKNOWN)
+                   && aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_DEFAULT)));
+
+          /* A buffer large enough to hold any trivial NFSv4 ACL.
+             The max length of a trivial NFSv4 ACL is 6 words for owner,
+             6 for group, 7 for everyone, all times 2 because there are both
+             allow and deny ACEs.  There are 6 words for owner because of
+             type, flag, mask, wholen, "OWNER@"+pad and similarly for group;
+             everyone is another word to hold "EVERYONE@".  */
+          uint32_t buf[2 * (6 + 6 + 7)];
+
+          int ret = ((flags & ACL_SYMLINK_FOLLOW ? getxattr : lgetxattr)
                      (name, XATTR_NAME_NFSV4_ACL, buf, sizeof buf));
+          if (ret < 0)
+            switch (errno)
+              {
+              case ENODATA: return 0;
+              case ERANGE : return 1; /* ACL must be nontrivial.  */
+              }
+          else
+            {
+              /* It looks like a trivial ACL, but investigate further.  */
+              ret = acl_nfs4_nontrivial (buf, ret);
               if (ret < 0)
-                switch (errno)
-                  {
-                  case ENODATA: return 0;
-                  case ERANGE : return 1; /* ACL must be nontrivial.  */
-                  }
-              else
                 {
-                  /* It looks like a trivial ACL, but investigate further.  */
-                  ret = acl_nfs4_nontrivial (buf, ret);
-                  if (ret < 0)
-                    {
-                      errno = EINVAL;
-                      return ret;
-                    }
-                  errno = initial_errno;
+                  errno = EINVAL;
+                  return ret;
                 }
+              errno = initial_errno;
             }
+
           if (ret < 0)
             return - acl_errno_valid (errno);
           return ret;
@@ -460,7 +463,7 @@ file_has_aclinfo (char const *name, struct stat const *sb,
              either both succeed or both fail; it depends on the
              file system.  Therefore there is no point in making the second
              call if the first one already failed.  */
-          if (ret == 0 && S_ISDIR (sb->st_mode))
+          if (ret == 0 && (d_type == DT_DIR || d_type == DT_UNKNOWN))
             {
               acl = acl_get_file (name, ACL_TYPE_DEFAULT);
               if (acl)
@@ -849,7 +852,7 @@ int
 file_has_acl (char const *name, struct stat const *sb)
 {
   struct aclinfo ai;
-  int r = file_has_aclinfo (name, sb, &ai, 0);
+  int r = file_has_aclinfo (name, &ai, IFTODT (sb->st_mode));
   aclinfo_free (&ai);
   return r;
 }
index 8adb310bd0ed334e3d024229af5c7dc352bbc088..b5821dab005505fdd6c66fc66fc1b98c29ba2898 100644 (file)
@@ -9,7 +9,9 @@ m4/selinux-selinux-h.m4
 
 Depends-on:
 acl-permissions
+assert-h
 attribute
+dirent
 errno
 extern-inline
 minmax
index 79e80c1abff3d5c9580f5f1c5ce86e7ffe83982e..1cced639b0ea8b0c170c8ad7d4c9fdea01f8b7ce 100644 (file)
@@ -20,6 +20,7 @@
 
 #include "acl.h"
 
+#include <dirent.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -66,7 +67,7 @@ main (int argc, char *argv[])
       }
 
     struct aclinfo ai;
-    int reti = file_has_aclinfo (file, &statbuf, &ai, 0);
+    int reti = file_has_aclinfo (file, &ai, DT_UNKNOWN);
     if (reti != ret)
       {
         fprintf (stderr, "file_has_aclinfo failed for \"%s\"\n", file);
@@ -74,6 +75,16 @@ main (int argc, char *argv[])
       }
     aclinfo_free (&ai);
 
+    int retj = file_has_aclinfo (file, &ai, ACL_SYMLINK_FOLLOW | DT_UNKNOWN);
+    if (retj != ret)
+      {
+        fprintf (stderr,
+                 "file_has_aclinfo with ACL_SYMLINK_FOLLOW failed for \"%s\"\n",
+                 file);
+        exit (EXIT_FAILURE);
+      }
+    aclinfo_free (&ai);
+
     printf ("%s\n", ret ? "yes" : "no");
   }
 #else