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

[or-cvs] r15803: Stop using __attribute__((nonnull)): It gets us occcasional (in tor/trunk: . src/common)



Author: nickm
Date: 2008-07-09 11:23:23 -0400 (Wed, 09 Jul 2008)
New Revision: 15803

Modified:
   tor/trunk/ChangeLog
   tor/trunk/src/common/compat.h
Log:
Stop using __attribute__((nonnull)): It gets us occcasional warnings when we do something so foolish it can be detected without dataflow analysis, but it also eliminates some of our error checking code.  Suggested by Peter Gutmann.

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2008-07-09 15:16:04 UTC (rev 15802)
+++ tor/trunk/ChangeLog	2008-07-09 15:23:23 UTC (rev 15803)
@@ -13,6 +13,9 @@
     - Change the contrib/tor.logrotate script so it makes the new
       logs as "_tor:_tor" rather than the default, which is generally
       "root:wheel". Fixes bug 676, reported by Serge Koksharov.
+    - Stop using __attribute__((nonnull)) with GCC: it can give us useful
+      warnings (occasionally), but it can also cause the compiler to
+      eliminate error-checking code.  Suggested by Peter Gutmann.
 
 
 Changes in version 0.2.0.29-rc - 2008-07-08

Modified: tor/trunk/src/common/compat.h
===================================================================
--- tor/trunk/src/common/compat.h	2008-07-09 15:16:04 UTC (rev 15802)
+++ tor/trunk/src/common/compat.h	2008-07-09 15:23:23 UTC (rev 15803)
@@ -122,7 +122,17 @@
 #define ATTR_CONST __attribute__((const))
 #define ATTR_MALLOC __attribute__((malloc))
 #define ATTR_NORETURN __attribute__((noreturn))
-#define ATTR_NONNULL(x) __attribute__((nonnull x))
+/* Alas, nonnull is not at present a good idea for us.  We'd like to get
+ * warnings when we pass NULL where we shouldn't (which nonnull does, albeit
+ * spottily), but we don't want to tell the compiler to make optimizations
+ * with the assumption that the argument can't be NULL (since this would make
+ * many of our checks go away, and make our code less robust against
+ * programming errors).  Unfortunately, nonnull currently does both of these
+ * things, and there's no good way to split them up.
+ *
+ * #define ATTR_NONNULL(x) __attribute__((nonnull x)) */
+#define ATTR_NONNULL(x)
+
 /** Macro: Evaluates to <b>exp</b> and hints the compiler that the value
  * of <b>exp</b> will probably be true. */
 #define PREDICT_LIKELY(exp) __builtin_expect((exp), 1)