On Thu, Oct 13, 2011 at 12:24:02PM -0600, Eric Blake wrote: > On 10/13/2011 02:55 AM, Daniel Veillard wrote: > >On Wed, Oct 12, 2011 at 05:27:40PM -0600, Eric Blake wrote: > >>Coverity complained that most, but not all, clients of virUUIDParse > >>were checking for errors. Silence those coverity warnings by > >>explicitly marking the cases where we trust the input, and fixing > >>one instance that really should have been checking. In particular, > >>this silences about half of the 46 warnings I saw on my most > >>recent Coverity analysis run. > >> > > >>@@ -129,9 +129,6 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { > >> const char *cur; > >> int i; > >> > >>- if ((uuidstr == NULL) || (uuid == NULL)) > >>- return(-1); > >>- > > > > ACK, but I'm always a bit distressed at the idea that a perfectly > > valid runtime check is being replaced by a static one which is > > compiler specific. Can we keep this chunk without Coverity complaining ? > > Unfortunately, no. Keeping this code will make Coverity warn about > dead code (the ATTRIBUTE_NONNULL implies that the check for NULL > will always fail). All callers were already passing valid pointers; > directly passing NULL will make gcc warn, and there's an open BZ > against gcc to make it someday do the same analysis as Coverity to > warn about any other obvious passing of NULL. But in spite of gcc's > weakness, both clang and coverity catch a lot more cases of passing > unintentional NULL, and get run frequently enough that I'm not too > worried about dropping the argument check (it's an internal > function, so we should be calling it correctly in the first place).
Also note that if you have ATTRIBUTE_NONNULL, then GCC will actually *remove* those two lines during optimization phase anyway. So by leaving them you are giving yourself a misleading view of reality. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list