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 <br...@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;
}
