On Wed, Sep 28, 2022 at 06:25:33PM +0100, Richard W.M. Jones wrote: > v2: > https://listman.redhat.com/archives/libguestfs/2022-September/030014.html > > I didn't think this would need a v3, but here we are. > > The first patch (also a new patch) appears to fix a bug in Eric's > earlier series to do with meta queries. It's not possible to call the > new APIs with queries == NULL, and this becomes obvious when you use > attribute((nonnull)) and enable GCC warnings. I tried to fix this, > but two tests still fail for reasons I'm not clear about: > > FAIL: opt-list-meta-queries > FAIL: opt-set-meta-queries > > The third patch is also new, and extends attribute((nonnull)) > annotations to many internal functions. No actual errors found by > this, but it seems worth it to avoid future problems, assuming that > GCC won't start adding undefined behaviour. I wonder aloud if we > should only enable attribute((nonnull)) for developer builds, ie. tie > it to ./configure --enable-gcc-warnings somehow. (This series does > not do this.)
At this point, I think we're safe. We've disabled the attribute for the one generated .c file where we really want to prevent the compiler from being too smart at eliding our intentional NULL checks, as well as in the testsuite files where we prove that we can trigger the EFAULT behavior when the attribute is suppressed. Everywhere else, clang's handling of nonnull is better than gcc such that CI tests should catch if we start passing NULL where we shouldn't (callers), or checking for NULL after documenting that we didn't need to (implementations). > > I added comments as suggested by Laszlo to patch 2, and > picked up his R-b's and Acks. I have some suggestions, but the series is now close enough that you don't need to post a v4 on my behalf. Acked-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
