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@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