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

[Michal Privoznik]

On 5/28/19 10:32 AM, Huaqiang,Wang wrote:
> 
> 
> 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
> ...
> }

Yes, this is exactly what I had on mind.

Michal