Re: [libvirt] [PATCH (RFC?)] Remove usage of __attribute__((nonnull))

On Wed, Apr 05, 2017 at 10:06:22AM +0100, Daniel P. Berrange wrote: > On Tue, Apr 04, 2017 at 09:51:47PM +0200, Martin Kletzander wrote: > > On Tue, Apr 04, 2017 at 04:10:59PM +0100, Daniel P. Berrange wrote: > > > On Tue, Mar 28, 2017 at 01:46:31PM +0200, Martin Kletzander wrote: > > > > The attribute (defined as ATTRIBUTE_NONNULL) was added long time > > > > ago (2009), but in 2012 (commit eefb881d4683) it was disabled for > > > > normal build, making it used only when coverity was building libvirt > > > > or on special request. It was disabled because it was being misused > > > > and misunderstood -- the attribute is there as an optimization hint > > > > for the compiler, not to enhance static analysis. > > > > > > Actually the attribute does both and the primary intention of the attribute > > > *is* build time warnings and/or static analysis warnings: > > > > > > [quote] > > > 'nonnull (ARG-INDEX, ...)' > > > The 'nonnull' attribute specifies that some function parameters > > > should be non-null pointers. For instance, the declaration: > > > > > > extern void * > > > my_memcpy (void *dest, const void *src, size_t len) > > > __attribute__((nonnull (1, 2))); > > > > > > causes the compiler to check that, in calls to 'my_memcpy', > > > arguments DEST and SRC are non-null. If the compiler determines > > > > This, however, happens only if we pass NULL directly. There's not much > > of a deeper analysis involved IIRC. > > Yep, coverity is needed to do most deeper analysis of NULL handling > > > > The use of nonnull attribute would help the compiler report > > > mistakes, but the compiler only catches some easy cases at build > > > time. > > > > > > > It would. But for now it is only enabled if STATIC_ANALYSIS=1 and that > > is only set if you are running in coverity (or explicitly specify that > > during configure, which almost nobody does). > > Easily solvable by enabling it in CI. > How about we enable it by default for build with new enough GCC (so that it reports these errors without silently dropping the non-NULL checks)?