Re: [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok()

[Greg Kurz]

On Tue, 6 Jun 2017 16:28:26 +0100
Peter Maydell <address@hidden> wrote:

> On 6 June 2017 at 16:22, Greg Kurz <address@hidden> wrote:
> > The string returned by object_property_get_str() is dynamically allocated.
> >
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> >  target/ppc/kvm.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 88817620766c..f2f7c531bc7b 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char 
> > *obj_path)
> >
> >      if (mempath) {
> >          pagesize = qemu_mempath_getpagesize(mempath);
> > +        g_free(mempath);
> >      } else {
> >          pagesize = getpagesize();
> >      }  
> 
> Huh, I wasn't expecting this function to free its argument.

Hmm... mempath isn't an argument, it is computed locally:

bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
{
    Object *mem_obj = object_resolve_path(obj_path, NULL);
    char *mempath = object_property_get_str(mem_obj, "mem-path", NULL);
    long pagesize;

    if (mempath) {
        pagesize = qemu_mempath_getpagesize(mempath);
        g_free(mempath);
    } else {
        pagesize = getpagesize();
    }

    return pagesize >= max_cpu_page_size;
}

> If we take that API then don't we also need to change the
> two other implementations of it in the tree? Having the
> single caller do the g_free() seems simpler...
> 
> thanks
> -- PMM

pgpFlLqlDAbu9.pgp
Description: OpenPGP digital signature