Adding Rich, since he recently raised the topic on the libguestfs list and pointed to libvirt as precedence:
On 12/18/18 9:47 AM, Daniel P. Berrangé wrote: > On Tue, Dec 18, 2018 at 10:39:20AM -0500, John Ferlan wrote: >> Commit 912c6b22 modified the virCommandAddArg and virCommandAddArgPair >> to perform NULL argument checking; however, no corresponding change >> to the prototypes was made to remove the ATTRIBUTE_NONNULL, so the >> Coverity build failed. Adjust the prototypes accordingly. > > This sounds wrong or at least different from the past. > > We have historically still added nonnull annotations even when > having checks for NULL in the impl. We had to explicitly disabble > the nonnull annotation when building under GCC to prevent it from > optimizing out the NULL checks. We left it enabled for STATIC_ANALYSIS > so that coverity would still report if any callers passed a NULL into > the methods. Looking at the gcc bug and following some of the links mentioned therein, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01103.html It looks like modern gcc has added -Wnonnull-compare that noisily informs us any time we have an ATTRIBUTE_NONNULL mismatch with the body of a function comparing that parameter against NULL after all. And gnulib's manywarnings module turns that on automatically (at least for gcc 8.2.1 on Fedora 29). That is sufficient to fix the bug that led us to historically disable ATTRIBUTE_NONNULL under gcc (commit eefb881), so maybe it's time to just blindly enable ATTRIBUTE_NONNULL everywhere on the grounds that we have enough developers and CI coverage to now immediately catch inconsistent attributes rather than risking silently mis-compiled code that omits the branch after a NULL comparison as dead code. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
