According to Jim Meyering on 3/1/2010 1:21 PM: > Clang warned about the potential NULL-dereference > via the STREQ/strcmp below. models[i] could be NULL. > Even "models" could be NULL, and the "allowed = ..." test > makes that appear to be deliberately allowed.
This same function was also listed by coverity, but only for models, not
models[i].
> The change below is the least invasive and cleanest
> I could come up with, assuming there is no need to diagnose
> (e.g., by returning -1) the condition of a NULL models[i] pointer.
>
> while (candidate != NULL) {
> bool allowed = (models == NULL);
>
> for (i = 0; i < candidate->ncpuid; i++) {
> cpuid = x86DataCpuid(data, candidate->cpuid[i].function);
> if (cpuid == NULL
> || !x86cpuidMatchMasked(cpuid, candidate->cpuid + i))
> goto next;
> }
>
> for (i = 0; i < nmodels; i++) {
> - if (STREQ(models[i], candidate->name)) {
> + if (models && models[i] && STREQ(models[i], candidate->name)) {
Isn't the intent that (models==NULL) iff (nmodels==0)? In which case,
this code is unreachable if models is NULL. But your patch certainly is
the least-invasive possible, and while it is only a false positive for
well-formed arguments, I didn't spend time checking all clients of
x86Decode to see if there is ever a possibility of bad arguments.
ACK
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
