[libvirt] [PATCH] build: force correct gcc syntax for attribute_nonnull

[Laine Stump]

On 02/18/2013 03:42 PM, Eric Blake wrote:
> Gcc lets you do:
>
> int ATTRIBUTE_NONNULL(1) foo(void *param);
> int foo(void *param) ATTRIBUTE_NONNULL(1);
> int ATTRIBUTE_NONNULL(1) foo(void *param) { ... }
>
> but chokes on:
>
> int foo(void *param) ATTRIBUTE_NONNULL(1) { ... }
>
> However, since commit eefb881, we have intentionally been disabling
> ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that
> is, gcc doesn't do decent warning reporting, then compiles code that
> mysteriously fails if you break the contract of the attribute, which
> is surprisingly easy to do), leaving it on only for Coverity (which
> does a much better job of improved static analysis when the attribute
> is present).
>
> But completely eliding the macro makes it too easy to write code that
> uses the fourth syntax option, if you aren't using Coverity.  So this
> patch forces us to avoid syntax errors, even when not using the
> attribute under gcc.  It also documents WHY we disable the warning
> under gcc, rather than forcing you to find the commit log.
>
> * src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute,
> rather than nothing, when on gcc.
> ---
>  src/internal.h |   17 +++++++++++++++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/src/internal.h b/src/internal.h
> index ebc91c7..2cf4731 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -181,9 +181,22 @@
>  #   endif
>  #  endif
>
> +/* gcc's handling of attribute nonnull is less than stellar - it does
> + * NOT improve diagnostics, and merely allows gcc to optimize away
> + * null code checks even when the caller manages to pass null in spite
> + * of the attribute, leading to weird crashes.  Coverity, on the other
> + * hand, knows how to do better static analysis based on knowing
> + * whether a parameter is nonnull.  Make this attribute conditional
> + * based on whether we are compiling for real or for analysis, while
> + * still requiring correct gcc syntax when it is turned off.  See also
> + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 */
>  #  ifndef ATTRIBUTE_NONNULL
> -#   if __GNUC_PREREQ (3, 3) && STATIC_ANALYSIS
> -#    define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m)))
> +#   if __GNUC_PREREQ (3, 3)
> +#    if STATIC_ANALYSIS
> +#     define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m)))
> +#    else
> +#     define ATTRIBUTE_NONNULL(m) __attribute__(())
> +#    endif
>  #   else
>  #    define ATTRIBUTE_NONNULL(m)
>  #   endif

ACK.