The string returned by object_property_get_str() is dynamically allocated.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 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();
     }

On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote:
> The string returned by object_property_get_str() is dynamically allocated.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  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.
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

On Tue, 6 Jun 2017 16:28:26 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote:
> > The string returned by object_property_get_str() is dynamically allocated.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  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

On 6 June 2017 at 16:41, Greg Kurz <groug@kaod.org> wrote:
> On Tue, 6 Jun 2017 16:28:26 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote:
>> > The string returned by object_property_get_str() is dynamically allocated.
>> >
>> > Signed-off-by: Greg Kurz <groug@kaod.org>
>> > ---
>> >  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:

Oops; sorry, I misread the patch.

thanks
-- PMM

On 06.06.2017 17:22, Greg Kurz wrote:
> The string returned by object_property_get_str() is dynamically allocated.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  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();
>      }
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>