Re: [PATCH] Use xattr (Linux) in copy-acl.c

[Bruno Haible <bruno@clisp.org>]

Ondrej Valousek wrote:
> Hope it looks fine now.

This looks better indeed.

> +#ifdef USE_XATTR

Since USE_XATTR is always defined (to 0 or 1), you need to test it
with #if, not #ifdef.

> @@ -42,10 +56,29 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
>    struct permission_context ctx;

This variable is only used in the #else case. To avoid compiler warnings
(I hope you always test with -Wall, right?) this variable declaration
should be moved into the #else block.

> +  AC_SUBST([LIB_XATTR])

I don't see any use of $(LIB_XATTR) so far. AFAICS, it ought to be
used
  - in the 'Link' section of modules/qcopy-acl,
  - in the 'Link' section of modules/acl, modules/copy-file, modules/qacl,
    modules/supersede — since these modules depend on qcopy-acl.
  - for the programs created in modules/acl-tests, modules/copy-file-tests,
    modules/supersede-tests.
Preferably each time right after $(LIB_ACL).
(One needs to lists the libraries explicitly, because you have no guarantee
that the distro uses shared libraries with implicit dependencies. It could
also be shared libraries without explicit dependencies, or static libraries.)

Bruno