[libvirt] [PATCH 6/9] util: Refactor 'virResctrlMonitorFreeStats'

[Huaqiang,Wang]

On 2019年05月27日 23:26, Michal Privoznik wrote:
> On 5/23/19 11:34 AM, Wang Huaqiang wrote:
>> Refactor 'virResctrlMonitorFreeStats' to let it available to free
>> the 'virResctrlMonitorStatsPtr' variable.
>>
>> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
>> ---
>>   src/qemu/qemu_driver.c | 1 +
>>   src/util/virresctrl.c  | 4 +---
>>   2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 42b1ce2..2abed86 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -19980,6 +19980,7 @@ 
>> qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata)
>>       VIR_FREE(resdata->name);
>>       VIR_FREE(resdata->vcpus);
>>       virResctrlMonitorFreeStats(resdata->stats, resdata->nstats);
>> +    VIR_FREE(resdata->stats);
>>       VIR_FREE(resdata);
>>   }
>>   diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
>> index 90532cf..0f18d2b 100644
>> --- a/src/util/virresctrl.c
>> +++ b/src/util/virresctrl.c
>> @@ -2764,7 +2764,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr 
>> monitor,
>>    cleanup:
>>       VIR_FREE(datapath);
>>       VIR_FREE(filepath);
>> -    VIR_FREE(stat);
>> +    virResctrlMonitorFreeStats(&stat, 1);
>
> How about creating a function that frees exactly one 
> virResctrlMonitorStatsPtr and then (possibly) have a wrapper that 
> would call it 1 to n times? We can then get rid of this ugly call.
>

How about change virResctrlMonitorFreeStats to: (function name have been 
changed to
virResctrlMonitorStatsFree)

```code

void
virResctrlMonitorStatsFree(virResctrlMonitorStatsPtr stat)
{
     if (!stat)
         return;

     VIR_FREE(stat->vals);
     virStringListFree(stat->features);
     VIR_FREE(stat);  /* Free virResctrlMonitorStats object */
}


```
And following code to is the way to free a list of 
@virResctrlMonitorStatsPtr objects:

```code

static void
qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata)
{
...
     size_t i = 0;

     for ( i = 0; i < resdata->nstats; i++)
         virResctrlMonitorStatsFree(resdata->stats[i]);

    virResctrlMonitorStatsFree
...
}

```

>>       VIR_DIR_CLOSE(dirp);
>>       return ret;
>>   }
>> @@ -2781,8 +2781,6 @@ 
>> virResctrlMonitorFreeStats(virResctrlMonitorStatsPtr *stats,
>>         for (i = 0; i < nstats; i++)
>>           VIR_FREE(stats[i]);
>> -
>> -    VIR_FREE(stats);
>
> This means that virResctrlMonitorGetStats() is now leaking @stat. For 
> instance, if virStrToLong_uip() fails.
>

Got. Thanks.

> Michal

Thanks for review.
Huaqiang