[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Fix treatment of --enable-assert in AC_HEADER_ASSERT

From: Eric Blake
Subject: Re: [PATCH] Fix treatment of --enable-assert in AC_HEADER_ASSERT
Date: Sun, 07 Dec 2008 07:30:00 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20081105 Thunderbird/ Mnenhy/

Hash: SHA1

According to William Pursell on 12/7/2008 12:11 AM:
> +     Fix AC_HEADER_ASSERT to honor --enable-assert, rather than
> +     treat --enable-assert and --disable-assert equivalently.
> +     * lib/autoconf/headers.m4
> +

Thanks for the idea; it is certainly a step in the right direction.
However, would you mind resubmitting with some additional nits fixed?

>    AC_MSG_CHECKING([whether to enable assertions])
>    AC_ARG_ENABLE([assert],
>      [  --disable-assert        turn off assertions],

We should be promoting the use of AS_HELP_STRING on this line,

> -    [AC_MSG_RESULT([no])
> -     AC_DEFINE(NDEBUG, 1, [Define to 1 if assertions should be disabled.])],
> +    [ if test "x$enableval" != xyes; then

this could be done with AS_IF, although that is not a necessity,

> +    AC_MSG_RESULT([no])
> +     AC_DEFINE(NDEBUG, 1, [Define to 1 if assertions should be disabled.])

we should promote proper m4 quoting on this line,

> +    else
> +    AC_MSG_RESULT([yes])
> +    fi ],

and trailing whitespace is smashed by autom4te, so it doesn't make sense
to document it.

>      [AC_MSG_RESULT(yes)])

You might want to simplify by doing a single
AC_MSG_RESULT([$enable_assert]) after the conclusion of AC_ARG_ENABLE,
rather than repeating a hardcoded the yes/no in each branch.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at
Comment: Using GnuPG with Mozilla -


reply via email to

[Prev in Thread] Current Thread [Next in Thread]