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:
> >>@@ -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).
Okay an example scenario I have in mind is someone developping
a driver for a OS where gcc is not the compiler (say ppc64 lpar for AIX
and using xlc as a example) then even if the code is internal it would
never be compiled with gcc, and one would require Coverity to actually
find the problem, internal function or not.
To be honnest I dislike the idea that we must rely on a proprietary
code in the future to make sure the code is correct, and even if gcc
was adding some support for this, this is still not an absolute
guarantee...
Still the ACK stands, but I prefer to not completely generalize
a static only check approach to our code base,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list