diff lib/set-mode-acl.c @ 16382:cc05dad27529

acl: Don't use GETACLCNT and similar ops, since they are unreliable. - There were several instances of this pattern: for (;;) { n = acl (f, GETACLCNT, 0, NULL); [ allocate an array A of size N ] if (acl (f, GETACL, n, a) == n) break; } This loop might never terminate if some other process is constantly manipulating the file's ACL. The loop should be rewritten to terminate. - The acl (... GETACLNT ...) call is merely an optimization; its value is merely a hint as to how big to make the array. A better optimization is to avoid the acl (... GETACLNT ...) call entirely, and just guess a reasonably-big size, growing the size and trying again if it's not large enough. This guarantees termination, and saves a system call. * lib/acl-internal.h: Include <limits.h>. (MIN, SIZE_MAX): New macros. * lib/file-has-acl.c (file_has_acl) [Solaris]: Read the entries into a stack-allocated buffer, and use malloc if it does not fit. Don't use GETACLCNT. * lib/set-mode-acl.c (qset_acl) [Solaris]: Likewise.
author Paul Eggert <eggert@cs.ucla.edu>
date Mon, 20 Feb 2012 01:12:06 +0100
parents fc5c37ccbece
children 498a2211d839
line wrap: on
line diff
--- a/lib/set-mode-acl.c
+++ b/lib/set-mode-acl.c
@@ -197,7 +197,7 @@
   return chmod_or_fchmod (name, desc, mode);
 #  endif
 
-# elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */
+# elif HAVE_FACL && defined GETACL /* Solaris, Cygwin, not HP-UX */
 
   int done_setacl = 0;
 
@@ -214,55 +214,60 @@
   int convention;
 
   {
+    /* Initially, try to read the entries into a stack-allocated buffer.
+       Use malloc if it does not fit.  */
+    enum
+      {
+        alloc_init = 4000 / sizeof (ace_t), /* >= 3 */
+        alloc_max = MIN (INT_MAX, SIZE_MAX / sizeof (ace_t))
+      };
+    ace_t buf[alloc_init];
+    size_t alloc = alloc_init;
+    ace_t *entries = buf;
+    ace_t *malloced = NULL;
     int count;
-    ace_t *entries;
 
     for (;;)
       {
-        int ret;
-
-        if (desc != -1)
-          count = facl (desc, ACE_GETACLCNT, 0, NULL);
-        else
-          count = acl (name, ACE_GETACLCNT, 0, NULL);
-        if (count <= 0)
-          {
-            convention = -1;
-            break;
-          }
-        entries = (ace_t *) malloc (count * sizeof (ace_t));
-        if (entries == NULL)
-          {
-            errno = ENOMEM;
-            return -1;
-          }
-        ret = (desc != -1
-               ? facl (desc, ACE_GETACL, count, entries)
-               : acl (name, ACE_GETACL, count, entries));
-        if (ret < 0)
+        count = (desc != -1
+                 ? facl (desc, ACE_GETACL, alloc, entries)
+                 : acl (name, ACE_GETACL, alloc, entries));
+        if (count < 0 && errno == ENOSPC)
           {
-            free (entries);
-            convention = -1;
-            break;
+            /* Increase the size of the buffer.  */
+            free (malloced);
+            if (alloc > alloc_max / 2)
+              {
+                errno = ENOMEM;
+                return -1;
+              }
+            alloc = 2 * alloc; /* <= alloc_max */
+            entries = malloced = (ace_t *) malloc (alloc * sizeof (ace_t));
+            if (entries == NULL)
+              {
+                errno = ENOMEM;
+                return -1;
+              }
+            continue;
           }
-        if (ret == count)
-          {
-            int i;
+        break;
+      }
 
-            convention = 0;
-            for (i = 0; i < count; i++)
-              if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER))
-                {
-                  convention = 1;
-                  break;
-                }
-            free (entries);
-            break;
-          }
-        /* Huh? The number of ACL entries changed since the last call.
-           Repeat.  */
-        free (entries);
+    if (count <= 0)
+      convention = -1;
+    else
+      {
+        int i;
+
+        convention = 0;
+        for (i = 0; i < count; i++)
+          if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER))
+            {
+              convention = 1;
+              break;
+            }
       }
+    free (malloced);
   }
 
   if (convention >= 0)