Re: Incorrect use of USE_XATTR in coreutils-8.4

[Jim Meyering]

[Cc'd autoconf for a suggestion below]

Pádraig Brady wrote:
> On 18/01/10 10:32, Pádraig Brady wrote:
>> On 17/01/10 08:03, Jim Meyering wrote:
>>> Thanks!
>>> That would fix it, but please retain the 0/1 semantics, in case
>>> we ever want to use USE_XATTR in a C (as opposed to cpp) expression.
>>>
>>> # Map yes,no to 1,0.
>>> AC_DEFINE_UNQUOTED([USE_XATTR],
>>> [`test $use_xattr = yes&& echo 1 || echo 0`],
>>> [Define if you want extended attribute support.])
>>>
>>> I know this is not the norm for USE_* variables e.g., in gnulib,
>>> but I have come to appreciate being able to use 0/1 cpp symbols
>>> in C code (albeit not often), for readability.
>>>
>>
>> Also if all variables were defined to something then
>> we could enable -Wundef which would catch this case:
>>
>> #define USE_XATTR yes
>> #if XATTR
>
> Seeing as we're not using -Wundef, I was wondering
> whether I could add a syntax-check to catch cases like this.
> How about the following? Would it be general enough for
> gnulib's maint.mk ?
>
> cheers,
> Pádraig.
>
> diff --git a/cfg.mk b/cfg.mk
> index b669961..e14155f 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -245,6 +245,14 @@ sc_prohibit_sleep:
>         msg='prefer xnanosleep over other sleep interfaces'             \
>           $(_prohibit_regexp)
>
> +# "#if HAVE_WHATEVER" will evaluate to false for any non numeric string.
> +# That would be flagged by using -Wundef, however gnulib currently
> +# tests many undefined macros, and so we can't enable that option.
> +# So at least preclude common boolean strings as macro values.
> +sc_Wundef_boolean:
> +       @grep -Ei '^#define.*(yes|no|true|false)$$' lib/config.h &&     \
> +         { echo 'Please use 0 or 1 for macro values' 1>&2; exit 1; }
> +

I like it.

However, it'd sure be nice to use something more generic than
lib/config.h.  IMHO, autoconf should make configure AC_SUBST its
currently-internal-only CONFIG_HEADERS variable.  While we wait,
I suppose we can kludge it by extracting the first file name
from the use of AC_CONFIG_HEADER(S)? in configure.ac.

autoconf's exporting CONFIG_HEADERS would also save coreutils
from having to hard-code lib/config.h here:

    tests/check.mk:  CONFIG_HEADER='$(abs_top_builddir)/lib/config.h' \