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 | 18 ++++++++ NEWS | 3 ++ lib/acl.h | 7 +-- lib/file-has-acl.c | 95 ++++++++++++++++++++------------------- modules/file-has-acl | 2 + tests/test-file-has-acl.c | 13 +++++- 6 files changed, 88 insertions(+), 50 deletions(-)
diff --git a/ChangeLog b/ChangeLog index fc6d4002ef..d3311a6ea8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2024-10-02 Paul Eggert <egg...@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 c3710825dd..bd120be0b3 100644 --- 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. diff --git a/lib/acl.h b/lib/acl.h index 1a627323ec..29ec2496fa 100644 --- 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 *) diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index f29414de67..9f4213702f 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -27,10 +27,15 @@ #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; } diff --git a/modules/file-has-acl b/modules/file-has-acl index 8adb310bd0..b5821dab00 100644 --- a/modules/file-has-acl +++ b/modules/file-has-acl @@ -9,7 +9,9 @@ m4/selinux-selinux-h.m4 Depends-on: acl-permissions +assert-h attribute +dirent errno extern-inline minmax diff --git a/tests/test-file-has-acl.c b/tests/test-file-has-acl.c index 79e80c1abf..1cced639b0 100644 --- a/tests/test-file-has-acl.c +++ b/tests/test-file-has-acl.c @@ -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 -- 2.43.0