On 5/11/21 12:37 PM, John Ferlan wrote: > > > 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@xxxxxxxxxx> >> --- >> 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. Unless I'm missing something that's just theoretical issue. Because virCapabilitiesClearHostNUMACellCPUTopology() does check for cpus == NULL and only if it is not NULL then it looks at ncpus. And I don't see how cpus could be !NULL and ncpus < 0 at the same time. But let me see if there's something I can do. Michal
