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

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