On Mon, Aug 29, 2016 at 12:32:34 -0400, John Ferlan wrote: ...
-size_t virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, - char ***names) +int +virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, + char ***names, + size_t *count) { + size_t i; + char **models = NULL; + + *count = 0; if (names) - *names = qemuCaps->cpuDefinitions; - return qemuCaps->ncpuDefinitions; + *names = NULL; + + if (!qemuCaps->cpuDefinitions) + return 0; + + if (names && VIR_ALLOC_N(models, qemuCaps->cpuDefinitions->count) < 0) + return -1; +
So if we don't have names, we don't get models and the following loop will only add to models if we've allocated it because we have names, right?
Thus could there be an optimization to set/return ->count if !names prior to this check? e.g.
It could, but only for a limited time. In the future the for loop will do a bit of filtering and thus we will have to go through it to compute @count.
if (!names) { *count = qemuCaps->cpuDefinitions->count; return 0; }
This works, but it's tough to read.
+ for (i = 0; i < qemuCaps->cpuDefinitions->count; i++) { + virDomainCapsCPUModelPtr cpu = qemuCaps->cpuDefinitions->models + i; + if (models && VIR_STRDUP(models[i], cpu->name) < 0) + goto error; + } + + if (names) + *names = models; + *count = qemuCaps->cpuDefinitions->count; + return 0; + + error: + virStringFreeListCount(models, i); + return -1; } ... @@ -4921,7 +4927,11 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, cpulist = NULL;
cleanup: - virStringFreeList(cpulist); + if (ret < 0 && cpulist) {
I think "ret < 0" is superfluous... Coverity whines too
Right. Fixed. Jirka