Re: [PATCH 2/5] virCapabilitiesHostNUMAInitReal: Free @cpus properly

On 5/5/21 4:02 AM, Michal Privoznik wrote: > The @cpus variable is an array of structs in which each item > contains a virBitmap member. As such it is not enough to just > VIR_FREE() the array - each bitmap has to be freed too. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- >   src/conf/capabilities.c | 1 + >   1 file changed, 1 insertion(+) > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > index 084e09286d..4d509a6d28 100644 > --- a/src/conf/capabilities.c > +++ b/src/conf/capabilities.c > @@ -1648,6 +1648,7 @@ virCapabilitiesHostNUMAInitReal(virCapsHostNUMA > *caps) >      cleanup: >       virBitmapFree(cpumap); > +    virCapabilitiesClearHostNUMACellCPUTopology(cpus, ncpus); There's some coding axiom about for every bug you fix you introduce another ;-)... Anyway Coverity notes you can get to cleanup from within the for loop when ncpus < 0 and that will not be very good for this call. Yes, -1 can no longer be returned, but -2 can be and we could fall to cleanup (at least theoretically). Another tweak could be to only check -2 and continue in the if statement since -1 is no longer possible.