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

[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
-- 
1.7.1


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