Re: [libvirt] [PATCH] build: add compiler attributes to virUUIDParse
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).
But fine, ACK
I've gone ahead and pushed this.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org