Re: [libvirt] [PATCH 6/9] util: Refactor 'virResctrlMonitorFreeStats'
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(a)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