bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L


From: Jim Meyering
Subject: Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Date: Mon, 03 Oct 2011 15:00:46 +0200

Kamil Dudka wrote:
> The commit 95f7c57 introduced an unintended change in behavior of ls -L.
> I am attaching a patch that restores the old behavior.  Thanks in advance
> for considering the patch!

Thanks again, Kamil,

Here's a version of your patch that I find more readable:
  - prefer if (CONDITION) over #if CONDITION, when possible
  - document the code more in comments, less in the commit log
Is this ok with you?


>From 628f1d14424c8356d5b1cfce690bbe544c048177 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <address@hidden>
Date: Mon, 3 Oct 2011 12:17:22 +0200
Subject: [PATCH] file-has-acl: revert unintended change in behavior of ls -L

* lib/file-has-acl.c (acl_extended_file_wrap): New function,
derived from...
(file_has_acl): ...code here.  Call it.
This problem was introduced with 2011-07-22 commit 95f7c57f,
"file-has-acl: use acl_extended_file_nofollow if available".
See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
---
 ChangeLog          |   10 ++++++++++
 lib/file-has-acl.c |   38 ++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a6d363a..140f5e1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-10-03  Kamil Dudka  <address@hidden>
+
+       file-has-acl: revert unintended change in behavior of ls -L
+       * lib/file-has-acl.c (acl_extended_file_wrap): New function,
+       derived from...
+       (file_has_acl): ...code here.  Call it.
+       This problem was introduced with 2011-07-22 commit 95f7c57f,
+       "file-has-acl: use acl_extended_file_nofollow if available".
+       See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
+
 2011-10-01  Jim Meyering  <address@hidden>

        maint.mk: adjust a release-related rule not to require use of gzip
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 1b7e392..db733ac 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -437,6 +437,32 @@ acl_nontrivial (int count, struct acl *entries)
 #endif


+/* Call acl_extended_file_nofollow when it is available and when
+   NAME is a symlink.  Otherwise, merely call acl_extended_file.
+   When acl_extended_file is not available, always return -1.  */
+
+static int
+acl_extended_file_wrap (char const *name)
+{
+  if ( ! HAVE_ACL_EXTENDED_FILE)
+    return -1;
+
+  if (HAVE_ACL_EXTENDED_FILE_NOFOLLOW)
+    {
+      struct stat sb;
+      if (! lstat (name, &sb) && ! S_ISLNK (sb.st_mode))
+        /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
+           unnecessary mounts, but it returns the same result as we already
+           know that NAME is not a symbolic link at this point (modulo the
+           TOCTTOU race condition).  */
+        return acl_extended_file_nofollow (name);
+    }
+
+  /* fallback for symlinks and old versions of libacl */
+  return acl_extended_file (name);
+}
+
+
 /* Return 1 if NAME has a nontrivial access control list, 0 if NAME
    only has no or a base access control list, and -1 (setting errno)
    on error.  SB must be set to the stat buffer of FILE.  */
@@ -453,20 +479,12 @@ file_has_acl (char const *name, struct stat const *sb)
       /* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
       int ret;

-      if (HAVE_ACL_EXTENDED_FILE || HAVE_ACL_EXTENDED_FILE_NOFOLLOW) /* Linux 
*/
+      if (HAVE_ACL_EXTENDED_FILE) /* Linux */
         {
-#  if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
-          /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
-             unnecessary mounts, but it returns the same result as we already
-             know that NAME is not a symbolic link at this point (modulo the
-             TOCTTOU race condition).  */
-          ret = acl_extended_file_nofollow (name);
-#  else
           /* On Linux, acl_extended_file is an optimized function: It only
              makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for
              ACL_TYPE_DEFAULT.  */
-          ret = acl_extended_file (name);
-#  endif
+          ret = acl_extended_file_wrap (name);
         }
       else /* FreeBSD, MacOS X, IRIX, Tru64 */
         {
--
1.7.7.rc0.362.g5a14



reply via email to

[Prev in Thread] Current Thread [Next in Thread]