Re: [PATCH] Bug fixes in acl module.

[Bruno Haible]

David Bartley wrote:
> The acl_get_entry related ones fix the loop logic;
> before the loop would infinitely loop (observable on freebsd via 'ls
> -ldv foo' where foo has a default acl set).

Thanks for reporting this.

Can you please also provide an augmented unit test that exhibits the bug?
(Additional code in tests/test-file-has-acl* ?)

However, the patch leads to a test failure on MacOS X:

file_has_acl("tmpfile0") returned no, expected yes
FAIL: test-file-has-acl.sh

The reason is that acl_get_entry has different return value conventions
on Linux and FreeBSD than on MacOS X.

On Linux [1] and FreeBSD [2], the function returns 1 if successful and 0 at
the end.

On MacOS X [3], the function returns 0 if successful and -1/EINVAL at the end.

[1] http://linux.die.net/man/3/acl_get_entry
[2] http://fuse4bsd.creo.hu/localcgi/man-cgi.cgi?acl_get_entry+3
[3] 
http://developer.apple.com/documentation/Darwin/Reference/ManPages/man3/acl_get_entry.3.html

I'm applying this, which passes "make check" on Linux and MacOS X.


2009-05-04  David Bartley  <address@hidden>
            Bruno Haible  <address@hidden>

        acl: Fix infinite loop on FreeBSD.
        * lib/acl_entries.c (acl_entries) [Linux, FreeBSD]: Fix interpretation
        of return value from acl_get_entry.
        * lib/file-has-acl.c (acl_access_nontrivial) [Linux, FreeBSD]:
        Likewise.

*** lib/acl_entries.c.orig      2009-05-04 03:25:20.000000000 +0200
--- lib/acl_entries.c   2009-05-04 03:24:07.000000000 +0200
***************
*** 35,47 ****
    if (acl != NULL)
      {
  #if HAVE_ACL_FIRST_ENTRY /* Linux, FreeBSD, MacOS X */
        acl_entry_t ace;
!       int at_end;
  
!       for (at_end = acl_get_entry (acl, ACL_FIRST_ENTRY, &ace);
!          !at_end;
!          at_end = acl_get_entry (acl, ACL_NEXT_ENTRY, &ace))
        count++;
  #else /* IRIX, Tru64 */
  # if HAVE_ACL_TO_SHORT_TEXT /* IRIX */
        /* Don't use acl_get_entry: it is undocumented.  */
--- 35,63 ----
    if (acl != NULL)
      {
  #if HAVE_ACL_FIRST_ENTRY /* Linux, FreeBSD, MacOS X */
+ # if HAVE_ACL_TYPE_EXTENDED /* MacOS X */
+       /* acl_get_entry returns 0 when it successfully fetches an entry,
+        and -1/EINVAL at the end.  */
        acl_entry_t ace;
!       int got_one;
  
!       for (got_one = acl_get_entry (acl, ACL_FIRST_ENTRY, &ace);
!          got_one >= 0;
!          got_one = acl_get_entry (acl, ACL_NEXT_ENTRY, &ace))
        count++;
+ # else /* Linux, FreeBSD */
+       /* acl_get_entry returns 1 when it successfully fetches an entry,
+        and 0 at the end.  */
+       acl_entry_t ace;
+       int got_one;
+ 
+       for (got_one = acl_get_entry (acl, ACL_FIRST_ENTRY, &ace);
+          got_one > 0;
+          got_one = acl_get_entry (acl, ACL_NEXT_ENTRY, &ace))
+       count++;
+       if (got_one < 0)
+       return -1;
+ # endif
  #else /* IRIX, Tru64 */
  # if HAVE_ACL_TO_SHORT_TEXT /* IRIX */
        /* Don't use acl_get_entry: it is undocumented.  */
*** lib/file-has-acl.c.orig     2009-05-04 03:25:20.000000000 +0200
--- lib/file-has-acl.c  2009-05-04 02:36:44.000000000 +0200
***************
*** 1,6 ****
  /* Test whether a file has a nontrivial access control list.
  
!    Copyright (C) 2002-2003, 2005-2008 Free Software Foundation, Inc.
  
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
--- 1,6 ----
  /* Test whether a file has a nontrivial access control list.
  
!    Copyright (C) 2002-2003, 2005-2009 Free Software Foundation, Inc.
  
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
***************
*** 55,65 ****
  #  if HAVE_ACL_FIRST_ENTRY /* Linux, FreeBSD */
  
    acl_entry_t ace;
!   int at_end;
  
!   for (at_end = acl_get_entry (acl, ACL_FIRST_ENTRY, &ace);
!        !at_end;
!        at_end = acl_get_entry (acl, ACL_NEXT_ENTRY, &ace))
      {
        acl_tag_t tag;
        if (acl_get_tag_type (ace, &tag) < 0)
--- 55,65 ----
  #  if HAVE_ACL_FIRST_ENTRY /* Linux, FreeBSD */
  
    acl_entry_t ace;
!   int got_one;
  
!   for (got_one = acl_get_entry (acl, ACL_FIRST_ENTRY, &ace);
!        got_one > 0;
!        got_one = acl_get_entry (acl, ACL_NEXT_ENTRY, &ace))
      {
        acl_tag_t tag;
        if (acl_get_tag_type (ace, &tag) < 0)
***************
*** 67,73 ****
        if (!(tag == ACL_USER_OBJ || tag == ACL_GROUP_OBJ || tag == ACL_OTHER))
        return 1;
      }
!   return 0;
  
  #  else /* IRIX, Tru64 */
  #   if HAVE_ACL_TO_SHORT_TEXT /* IRIX */
--- 67,73 ----
        if (!(tag == ACL_USER_OBJ || tag == ACL_GROUP_OBJ || tag == ACL_OTHER))
        return 1;
      }
!   return got_one;
  
  #  else /* IRIX, Tru64 */
  #   if HAVE_ACL_TO_SHORT_TEXT /* IRIX */