qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] hmp: fix MemdevList memory leak


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH 3/3] hmp: fix MemdevList memory leak
Date: Fri, 1 Aug 2014 23:38:56 +1000

On Fri, Aug 1, 2014 at 8:22 PM, Chen Fan <address@hidden> wrote:
> Signed-off-by: Chen Fan <address@hidden>
> ---
>  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
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]