[libvirt] [PATCH 03/41] qemu: Use virDomainCapsCPUModels for cpuDefinitions

[Jiri Denemark]

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