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).