| Message ID | SA1PR11MB67606FB284BF14ED9F11D436F51CA@SA1PR11MB6760.namprd11.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | softmmu: Fix dirtylimit memory leak | expand |
On Wed, Aug 23, 2023 at 3:48 PM <alloc.young@outlook.com> wrote: > From: "alloc.young" <alloc.young@outlook.com> > > Fix memory leak in hmp_info_vcpu_dirty_limit,use g_autoptr > handle memory deallocation, alse use g_free to match g_malloc > && g_new functions. > > Signed-off-by: alloc.young <alloc.young@outlook.com> > --- > softmmu/dirtylimit.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > [...] > diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c > index 3c275ee55b..fa959d7743 100644 > --- a/softmmu/dirtylimit.c > +++ b/softmmu/dirtylimit.c > @@ -100,7 +100,7 @@ static void vcpu_dirty_rate_stat_collect(void) > stat.rates[i].dirty_rate; > } > > - free(stat.rates); > + g_free(stat.rates); > } > > Code optimization. > static void *vcpu_dirty_rate_stat_thread(void *opaque) > @@ -171,10 +171,10 @@ void vcpu_dirty_rate_stat_initialize(void) > > void vcpu_dirty_rate_stat_finalize(void) > { > - free(vcpu_dirty_rate_stat->stat.rates); > + g_free(vcpu_dirty_rate_stat->stat.rates); > vcpu_dirty_rate_stat->stat.rates = NULL; > > - free(vcpu_dirty_rate_stat); > + g_free(vcpu_dirty_rate_stat); > vcpu_dirty_rate_stat = NULL; > } > > Likewise... > @@ -220,10 +220,10 @@ void dirtylimit_state_initialize(void) > > void dirtylimit_state_finalize(void) > { > - free(dirtylimit_state->states); > + g_free(dirtylimit_state->states); > dirtylimit_state->states = NULL; > > - free(dirtylimit_state); > + g_free(dirtylimit_state); > dirtylimit_state = NULL; > > Likewise... > trace_dirtylimit_state_finalize(); > @@ -653,7 +653,8 @@ struct DirtyLimitInfoList > *qmp_query_vcpu_dirty_limit(Error **errp) > > void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict) > { > - DirtyLimitInfoList *limit, *head, *info = NULL; > + DirtyLimitInfoList *info; > + g_autoptr(DirtyLimitInfoList) head = NULL; > Error *err = NULL; > > if (!dirtylimit_in_service()) { > @@ -661,20 +662,17 @@ void hmp_info_vcpu_dirty_limit(Monitor *mon, const > QDict *qdict) > return; > } > > - info = qmp_query_vcpu_dirty_limit(&err); > + head = qmp_query_vcpu_dirty_limit(&err); > if (err) { > hmp_handle_error(mon, err); > return; > } > > - head = info; > - for (limit = head; limit != NULL; limit = limit->next) { > + for (info = head; info != NULL; info = info->next) { > monitor_printf(mon, "vcpu[%"PRIi64"], limit rate %"PRIi64 " > (MB/s)," > " current rate %"PRIi64 " (MB/s)\n", > - limit->value->cpu_index, > - limit->value->limit_rate, > - limit->value->current_rate); > + info->value->cpu_index, > + info->value->limit_rate, > + info->value->current_rate); > } > - > - g_free(info); > Fix memory leak. > } > -- > 2.39.3 > > I'll choose the memory leak modifications to keep the patch focused on a single independent issue. Anyway, Reviewed-by: Hyman Huang(黄勇) <yong.huang@smartx.com>
23.08.2023 10:47, alloc.young@outlook.com wrote: > From: "alloc.young" <alloc.young@outlook.com> > > Fix memory leak in hmp_info_vcpu_dirty_limit,use g_autoptr > handle memory deallocation, alse use g_free to match g_malloc > && g_new functions. "..use g_autoptr TO handle.." ("to" is missing). "alse" - I guess should be "Also". I think it is better to split this into two parts, one fixing the memleak and another converting to g_free(). /mjt
On 2023/8/24 13:16, Michael Tokarev wrote: > 23.08.2023 10:47, alloc.young@outlook.com wrote: >> From: "alloc.young" <alloc.young@outlook.com> >> >> Fix memory leak in hmp_info_vcpu_dirty_limit,use g_autoptr >> handle memory deallocation, alse use g_free to match g_malloc >> && g_new functions. > > "..use g_autoptr TO handle.." ("to" is missing). > "alse" - I guess should be "Also". > I'll fix these typos in next patch. > I think it is better to split this into two parts, one fixing > the memleak and another converting to g_free(). Ok, I'll make another patch, Thx. > > > /mjt
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index 3c275ee55b..fa959d7743 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -100,7 +100,7 @@ static void vcpu_dirty_rate_stat_collect(void) stat.rates[i].dirty_rate; } - free(stat.rates); + g_free(stat.rates); } static void *vcpu_dirty_rate_stat_thread(void *opaque) @@ -171,10 +171,10 @@ void vcpu_dirty_rate_stat_initialize(void) void vcpu_dirty_rate_stat_finalize(void) { - free(vcpu_dirty_rate_stat->stat.rates); + g_free(vcpu_dirty_rate_stat->stat.rates); vcpu_dirty_rate_stat->stat.rates = NULL; - free(vcpu_dirty_rate_stat); + g_free(vcpu_dirty_rate_stat); vcpu_dirty_rate_stat = NULL; } @@ -220,10 +220,10 @@ void dirtylimit_state_initialize(void) void dirtylimit_state_finalize(void) { - free(dirtylimit_state->states); + g_free(dirtylimit_state->states); dirtylimit_state->states = NULL; - free(dirtylimit_state); + g_free(dirtylimit_state); dirtylimit_state = NULL; trace_dirtylimit_state_finalize(); @@ -653,7 +653,8 @@ struct DirtyLimitInfoList *qmp_query_vcpu_dirty_limit(Error **errp) void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict) { - DirtyLimitInfoList *limit, *head, *info = NULL; + DirtyLimitInfoList *info; + g_autoptr(DirtyLimitInfoList) head = NULL; Error *err = NULL; if (!dirtylimit_in_service()) { @@ -661,20 +662,17 @@ void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict) return; } - info = qmp_query_vcpu_dirty_limit(&err); + head = qmp_query_vcpu_dirty_limit(&err); if (err) { hmp_handle_error(mon, err); return; } - head = info; - for (limit = head; limit != NULL; limit = limit->next) { + for (info = head; info != NULL; info = info->next) { monitor_printf(mon, "vcpu[%"PRIi64"], limit rate %"PRIi64 " (MB/s)," " current rate %"PRIi64 " (MB/s)\n", - limit->value->cpu_index, - limit->value->limit_rate, - limit->value->current_rate); + info->value->cpu_index, + info->value->limit_rate, + info->value->current_rate); } - - g_free(info); }