Re: [Qemu-devel] [PATCH 3/3] hw/machine: Free old values of string properties

[Marcel Apfelbaum]

On Mon, 2014-06-02 at 14:52 +0200, Markus Armbruster wrote:
> Marcel Apfelbaum <address@hidden> writes:
> 
> > On Mon, 2014-06-02 at 13:51 +0200, Markus Armbruster wrote:
> >> Marcel Apfelbaum <address@hidden> writes:
> >> 
> >> > On Fri, 2014-05-30 at 17:02 -0300, Eduardo Habkost wrote:
> >> >> Signed-off-by: Eduardo Habkost <address@hidden>
> >> >> ---
> >> >> Cc: Marcel Apfelbaum <address@hidden>
> >> >> Cc: Andreas Färber <address@hidden>
> >> >> ---
> >> >>  hw/core/machine.c | 8 ++++++++
> >> >>  1 file changed, 8 insertions(+)
> >> >> 
> >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c
> >> >> index cbba679..df612bb 100644
> >> >> --- a/hw/core/machine.c
> >> >> +++ b/hw/core/machine.c
> >> >> @@ -24,6 +24,7 @@ static void machine_set_accel(Object *obj, const char 
> >> >> *value, Error **errp)
> >> >>  {
> >> >>      MachineState *ms = MACHINE(obj);
> >> >>  
> >> >> +    g_free(ms->accel);
> >> > I am not sure if in QMP is not caller's responsibility to free the
> >> > input string.
> >> > If I think about it, I ask an object to set "my" string and it
> >> > deletes it :(...
> >> > Same for the others.
> >> >
> >> > Added Markus and Luiz, maybe they have an opinion on that.
> >> >
> >> >>      ms->accel = g_strdup(value);
> >> >>  }
> >> >>  
> >> 
> >> Misunderstanding?  Eduardo's patch frees the old value before it
> >> overwrites it.  It doesn't free "the input string", assuming by "the
> >> input string" you mean argument value.
> >
> > You are right! My bad, for some reason I saw g_free(value), but it
> > was me not reading it right :(.
> 
> Happens :)
> 
> Reviewed-by: Markus Armbruster <address@hidden>

Yes indeed.
Reviewed-by: Marcel Apfelbaum <address@hidden>