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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list