file-has-acl: Fix performance regression on FreeBSD, Cygwin

[Bruno Haible]

The recent changes in module file-has-acl can lead to extra system calls
executed on FreeBSD and Cygwin for the file_has_acl() function.

Namely, the caller of this function calls lstat(), and if the directory
entry is of some unusual type, the IFTODT macro returns DT_UNKNOWN, which
loses the information that the directory entry does not name a directory.
Then, the file_has_aclinfo() function calls
   acl_get_file (name, ACL_TYPE_DEFAULT)
although this makes only sense for directories.

This patch restores the previous behaviour that once we have called lstat()
we don't need to determine the ACL_TYPE_DEFAULT ACL unless it's a directory.


2024-10-07  Bruno Haible  <bruno@clisp.org>

        file-has-acl: Fix performance regression on FreeBSD, Cygwin.
        * lib/dirent.in.h (_GL_DT_NOTDIR): New macro.
        * lib/acl.h (ACL_SYMLINK_FOLLOW): Increase value.
        * lib/file-has-acl.c (file_has_aclinfo): Don't call
        acl_get_file (name, ACL_TYPE_DEFAULT) if we know that name does not
        denote a directory.
        (file_has_acl): Extract from *SB the information that NAME is not a
        directory.

diff --git a/lib/acl.h b/lib/acl.h
index 29ec2496fa..68a421a093 100644
--- a/lib/acl.h
+++ b/lib/acl.h
@@ -32,9 +32,9 @@
 extern "C" {
 #endif
 
-/* 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 };
+/* Follow symlinks when getting an ACL.  This is a bitmask that is guaranteed
+   not to collide with any <dirent.h> DT_* or _GL_DT_* value.  */
+enum { ACL_SYMLINK_FOLLOW = 0x10000 };
 
 /* Information about an ACL.  */
 struct aclinfo
diff --git a/lib/dirent.in.h b/lib/dirent.in.h
index c8c39c3e6b..a0ac39b496 100644
--- a/lib/dirent.in.h
+++ b/lib/dirent.in.h
@@ -54,6 +54,7 @@ struct dirent
    but not (yet) DT_MQ, DT_SEM, DT_SHM, DT_TMO.
    These macros can be useful even on platforms that do not support
    d_type or the corresponding file types.
+   The values of these macros are all in the 'unsigned char' range.
    Default to the Linux values which are also popular elsewhere,
    and check that all macros have distinct values.  */
 #ifndef DT_UNKNOWN
@@ -97,6 +98,9 @@ static_assert (DT_UNKNOWN != DT_FIFO && DT_UNKNOWN != DT_CHR
                && DT_LNK != DT_SOCK && DT_LNK != DT_WHT
                && DT_SOCK != DT_WHT);
 
+/* Other optional information about a directory entry.  */
+#define _GL_DT_NOTDIR 0x100   /* Not a directory */
+
 /* Conversion between S_IF* and DT_* file types.  */
 #if ! (defined IFTODT && defined DTTOIF)
 # include <sys/stat.h>
@@ -111,6 +115,7 @@ static_assert (DT_UNKNOWN != DT_FIFO && DT_UNKNOWN != DT_CHR
 #  define _GL_DIRENT_S_IFWHT (DT_WHT << 12) /* just a guess */
 # endif
 #endif
+/* Conversion from a 'stat' mode to a DT_* value.  */
 #ifndef IFTODT
 # define IFTODT(mode) \
    (S_ISREG (mode) ? DT_REG : S_ISDIR (mode) ? DT_DIR \
@@ -119,6 +124,7 @@ static_assert (DT_UNKNOWN != DT_FIFO && DT_UNKNOWN != DT_CHR
     : S_ISSOCK (mode) ? DT_SOCK \
     : _GL_DIRENT_S_ISWHT (mode) ? DT_WHT : DT_UNKNOWN)
 #endif
+/* Conversion from a DT_* value to a 'stat' mode.  */
 #ifndef DTTOIF
 # define DTTOIF(dirtype) \
    ((dirtype) == DT_REG ? S_IFREG : (dirtype) == DT_DIR ? S_IFDIR \
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 1fc54a7687..0dfd25b52a 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -328,8 +328,10 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
    if ACLs are not supported as errno is set in that case also.
    Set *AI to ACL info regardless of return value.
    FLAGS should be a <dirent.h> d_type value, optionally ORed with
-   ACL_SYMLINK_FOLLOW; if the d_type value is not known,
-   use DT_UNKNOWN though this may be less efficient.
+     - _GL_DT_NOTDIR if it is known that NAME is not a directory,
+     - ACL_SYMLINK_FOLLOW to follow the link if NAME is a symbolic link.
+   If the d_type value is not known, use DT_UNKNOWN though this may be less
+   efficient.
    If FLAGS & ACL_SYMLINK_FOLLOW, follow symlinks when retrieving ACL info;
    otherwise do not follow them if possible.  */
 int
@@ -463,7 +465,9 @@ file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
              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 && (d_type == DT_DIR || d_type == DT_UNKNOWN))
+          if (ret == 0
+              && (d_type == DT_DIR
+                  || (d_type == DT_UNKNOWN && !(flags & _GL_DT_NOTDIR))))
             {
               acl = acl_get_file (name, ACL_TYPE_DEFAULT);
               if (acl)
@@ -851,8 +855,11 @@ file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
 int
 file_has_acl (char const *name, struct stat const *sb)
 {
+  int flags = IFTODT (sb->st_mode);
+  if (!S_ISDIR (sb->st_mode))
+    flags |= _GL_DT_NOTDIR;
   struct aclinfo ai;
-  int r = file_has_aclinfo (name, &ai, IFTODT (sb->st_mode));
+  int r = file_has_aclinfo (name, &ai, flags);
   aclinfo_free (&ai);
   return r;
 }