Re: [libvirt] [PATCH 03/41] qemu: Use virDomainCapsCPUModels for cpuDefinitions
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