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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list