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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list