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