[libvirt] [PATCH] build: add compiler attributes to virUUIDParse

Eric Blake eblake at redhat.com
Thu Oct 13 18:24:02 UTC 2011


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list