GNU bug report logs - #20310
[COREUTILS 0/2] Improved acl handling

Previous Next

Full log

Message #10 received at 20310 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Andreas Gruenbacher <andreas.gruenbacher <at> gmail.com>, 
 bug-gnulib <at> gnu.org, 20310 <at> debbugs.gnu.org
Subject: Re: [COREUTILS 2/2] ls: Don't treat lack of acl support as an error
Date: Tue, 21 Apr 2015 04:40:00 +0100

On 12/04/15 15:37, Andreas Gruenbacher wrote:
> * src/ls.c (file_has_acl_cache): When a file system doesn't support
> acls, fail with errno set to ENOTSUP.
> (gobble_file): Don't treat lack of acl support as an error.
> ---
>  src/ls.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ls.c b/src/ls.c
> index b308dd3..884e042 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -2866,7 +2866,7 @@ getfilecon_cache (char const *file, struct fileinfo *f, bool deref)
>  
>  /* Cache file_has_acl failure, when it's trivial to do.
>     Like file_has_acl, but when F's st_dev says it's on a file
> -   system lacking ACL support, return 0 with ENOTSUP immediately.  */
> +   system lacking ACL support, fail with ENOTSUP immediately.  */
>  static int
>  file_has_acl_cache (char const *file, struct fileinfo *f)
>  {
> @@ -2877,14 +2877,11 @@ file_has_acl_cache (char const *file, struct fileinfo *f)
>    if (f->stat.st_dev == unsupported_device)
>      {
>        errno = ENOTSUP;
> -      return 0;
> +      return -1;
>      }
>  
> -  /* Zero errno so that we can distinguish between two 0-returning cases:
> -     "has-ACL-support, but only a default ACL" and "no ACL support". */
> -  errno = 0;
>    int n = file_has_acl (file, &f->stat);
> -  if (n <= 0 && errno_unsupported (errno))
> +  if (n < 0 && errno_unsupported (errno))
>      unsupported_device = f->stat.st_dev;
>    return n;
>  }
> @@ -3076,7 +3073,7 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
>            if (err == 0 && format == long_format)
>              {
>                int n = file_has_acl_cache (absolute_name, f);
> -              err = (n < 0);
> +              err = (n < 0 && ! errno_unsupported (errno));
>                have_acl = (0 < n);
>              }

I dislike this change actually.
Or more accurately, the gnulib change that changed the file_has_acl()
interface, requiring this change.

Previously in gnulib we mapped ENOTSUP to return 0 using:
http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/acl-errno-valid.c

Since we've now changed file_has_acl() to return -1 in this case,
all gnulib users may now be printing erroneous errors etc.

Is there any reason not to use the same gnulib acl_errno_valid() logic
in the newly added "avoiding libacl" path?

thanks,
Pádraig.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.