On Wed, Apr 05, 2017 at 12:38:23PM +0200, Martin Kletzander wrote: > 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)? Yes, it seems that we can use -fno-delete-null-pointer-checks to prevent GCC from optimizing away the NULL pointer checks, while having nonnull attributes enabled all the time. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
