On Fri, Aug 1, 2014 at 8:22 PM, Chen Fan <chen.fan.f...@cn.fujitsu.com> wrote:
> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com>
> ---
> hmp.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 2414cc7..0b1c830 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1685,13 +1685,14 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> MemdevList *memdev_list = qmp_query_memdev(&err);
> - MemdevList *m = memdev_list;
> + MemdevList *m;
> StringOutputVisitor *ov;
> char *str;
> int i = 0;
>
>
> - while (m) {
> + while (memdev_list) {
> + m = memdev_list;
> ov = string_output_visitor_new(false);
> visit_type_uint16List(string_output_get_visitor(ov),
> &m->value->host_nodes, NULL, NULL);
> @@ -1710,7 +1711,9 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
>
> string_output_visitor_cleanup(ov);
> g_free(str);
> - m = m->next;
> + memdev_list = memdev_list->next;
> + g_free(m->value);
> + g_free(m);
This doesnt feel quite right. You are piecewise freeing an entire data
structure as allocated by a single function (qmp_query_memdev). I
think you should create a function that cleans up MemdevList (just
loops and frees) so that any and all callers of qmp_query_memdev can
cleanup in one single action.
Note that qmp_query_memdev already has the code you need in it's error path:
while (list) {
m = list;
list = list->next;
g_free(m->value);
g_free(m);
}
So you can split this out into it's own fn and call it both here and
in that error path.
Regards,
Peter
> i++;
> }
>
> --
> 1.9.3
>
>
