On 03/16/2012 11:26 AM, Eric Blake wrote:
+ + if (!nparams) { + if ((cpu = PyDict_New()) == NULL) { + error = NULL;
Initialize error to NULL at the front, and you won't have to set it here.
+ goto failed; + } + + if (PyList_Append(ret, cpu) < 0) { + Py_DECREF(cpu); + error = NULL; + goto failed; + } + + Py_DECREF(cpu); + return ret;
So why are we populating the list with a single empty dictionary? Shouldn't we instead be populating it with one empty dictionary per cpu? That is, this code returns [{}], but if ncpus is 4, it should return [{},{},{},{}].
In fact, instead of returning early here, you can just say: if (nparams) { // get parameters, via for loop over...
+ } + sumparams = nparams * ncpus; + + if (VIR_ALLOC_N(params, sumparams) < 0) { + error = PyErr_NoMemory(); + goto failed; + } + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetCPUStats(domain, params, nparams, 0, ncpus, flags);
This will fail if ncpus > 128. You have to do this in a for loop, grabbing only 128 cpus per iteration.
+ LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + error = VIR_PY_NONE; + goto cleanup; + }
} else { i_retval = 0; }
+ + for (i = 0; i < ncpus; i++) { + if (params[i * nparams].type == 0) + continue;
Technically, this is only possible if you called virDomainGetCPUStats with ncpus larger than what the hypervisor already told you to use. But I guess it doesn't hurt to leave these two lines in.
then drop these two lines after all (since in the i_retval==0 case, params might be NULL and this would be a NULL deref),
+ + cpuparams = ¶ms[i * nparams];
This is just an address computation, so it doesn't matter if params is NULL.
+ if ((cpu = getPyVirTypedParameter(cpuparams, nparams)) == NULL) {
Here, you want to iterate over the number of returned parameters, not the number of array slots you passed in. s/nparams/i_retval/
and this will properly create your empty dictionary, since getPyVirTypedParameter does the right thing if i_retval is 0. That way, your PyList_Append() code can be reused to populate the correct return value for ncpus regardless of whether nparams was 0. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org