Re: acl, copy-file: small refactorings

[Jim Meyering]

Bruno Haible wrote:
> Here's a proposed series of tweaks and alignments to the 'acl' and 'copy-file'
> modules, to make Reuben's proposed change to 'copy-file' easier.
>
> The first change is not backward compatible, but the only users of copy_acl()
> are coreutils and 'copy-file' in gnulib, and I've verified that these callers
> don't care whether the return value is -1 or -2.
>
> For the third change, I considered exporting just one function copy_acl
> with a null_stderr argument, like in the module 'wait-process', rather than
> two function qcopy_acl and copy_acl. But I find the approach with two
> functions more maintainable: It would make it easier to move the code
> that does not emit error message to an LGPLed module.
>
> Objections?

Those changes look fine.
Only suggestion is to make a log-summary more descriptive:

> 2012-01-10  Bruno Haible  <address@hidden>
>
>       copy-file: Use 'quote' module consistently.
>       * lib/copy-file.c (copy_file_preserving): Use quote().
>
>       copy-file: Refactor.
>       * lib/copy-file.c: Include quote.h.
>       (copy_file_preserving): Call qcopy_acl instead of copy_acl. Emit error
>       message here.
>       * modules/copy-file (Depends-on): Add quote.
>
>       acl: Export qcopy_acl.
>       * lib/acl.h (qcopy_acl): New declaration.
>       * lib/copy-acl.c (qcopy_acl): Make non-static.
>
>       acl: Tweak.

How about this?

    acl: rename a local variable to be consistent with similar code

>       * lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
>
>       acl: Align return values of copy_acl and qcopy_acl.
>       * lib/copy-acl.c (copy_acl): Return the same value as qcopy_acl,
>       maybe < -1.
...
> Subject: [PATCH 2/5] acl: Tweak.
>
> * lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
...
> diff --git a/lib/set-mode-acl.c b/lib/set-mode-acl.c
> index b9d202e..a81b321 100644
> --- a/lib/set-mode-acl.c
> +++ b/lib/set-mode-acl.c
> @@ -677,8 +677,8 @@ qset_acl (char const *name, int desc, mode_t mode)
>  int
>  set_acl (char const *name, int desc, mode_t mode)
>  {
> -  int r = qset_acl (name, desc, mode);
> -  if (r != 0)
> +  int ret = qset_acl (name, desc, mode);
> +  if (ret != 0)
>      error (0, errno, _("setting permissions for %s"), quote (name));
> -  return r;
> +  return ret;
>  }
> --