[Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Eduardo Habkost]

This helps us avoid memory leaks when using object_property_add_alias(), as it
is not practical for callers to save target_name to free it later.

Eduardo Habkost (2):
  qom: strdup() target property name on object_property_add_alias()
  qdev: Free property names after registering gpio aliases

 hw/core/qdev.c | 2 ++
 qom/object.c   | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] qom: strdup() target property name on object_property_add_alias()

[Eduardo Habkost]

With this, object_property_add_alias() callers can safely free the
target property name, like what already happens with the 'name' argument
to all object_property_add*() functions.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qom/object.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index b8dff43..550dd48 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1689,7 +1689,7 @@ void object_property_add_uint64_ptr(Object *obj, const char *name,
 
 typedef struct {
     Object *target_obj;
-    const char *target_name;
+    char *target_name;
 } AliasProperty;
 
 static void property_get_alias(Object *obj, struct Visitor *v, void *opaque,
@@ -1720,6 +1720,7 @@ static void property_release_alias(Object *obj, const char *name, void *opaque)
 {
     AliasProperty *prop = opaque;
 
+    g_free(prop->target_name);
     g_free(prop);
 }
 
@@ -1747,7 +1748,7 @@ void object_property_add_alias(Object *obj, const char *name,
 
     prop = g_malloc(sizeof(*prop));
     prop->target_obj = target_obj;
-    prop->target_name = target_name;
+    prop->target_name = g_strdup(target_name);
 
     op = object_property_add(obj, name, prop_type,
                              property_get_alias,
-- 
2.1.0

[Qemu-devel] [PATCH 2/2] qdev: Free property names after registering gpio aliases

[Eduardo Habkost]

Now that object_property_add_alias() strdup()s target_name, we can free
the property names in qdev_pass_gpios().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..6dca6cb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -563,6 +563,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
         object_property_add_alias(OBJECT(container), propname,
                                   OBJECT(dev), propname,
                                   &error_abort);
+        g_free(propname);
     }
     for (i = 0; i < ngl->num_out; i++) {
         const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
@@ -571,6 +572,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
         object_property_add_alias(OBJECT(container), propname,
                                   OBJECT(dev), propname,
                                   &error_abort);
+        g_free(propname);
     }
     QLIST_REMOVE(ngl, node);
     QLIST_INSERT_HEAD(&container->gpios, ngl, node);
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Paolo Bonzini]

On 09/04/2015 21:57, Eduardo Habkost wrote:
> This helps us avoid memory leaks when using object_property_add_alias(), as it
> is not practical for callers to save target_name to free it later.
> 
> Eduardo Habkost (2):
>   qom: strdup() target property name on object_property_add_alias()
>   qdev: Free property names after registering gpio aliases
> 
>  hw/core/qdev.c | 2 ++
>  qom/object.c   | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 

Good idea!

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Andreas Färber]

Am 09.04.2015 um 22:22 schrieb Paolo Bonzini:
> On 09/04/2015 21:57, Eduardo Habkost wrote:
>> This helps us avoid memory leaks when using object_property_add_alias(), as it
>> is not practical for callers to save target_name to free it later.
>>
>> Eduardo Habkost (2):
>>   qom: strdup() target property name on object_property_add_alias()
>>   qdev: Free property names after registering gpio aliases
>>
>>  hw/core/qdev.c | 2 ++
>>  qom/object.c   | 5 +++--
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
> 
> Good idea!
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good to me in general, I'm just surprised that there are not more
cleanups resulting out of this? At least my pc87312 series will need
changes then. Will take a deeper look another day.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

On Thu, Apr 09, 2015 at 04:57:28PM -0300, Eduardo Habkost wrote:
> This helps us avoid memory leaks when using object_property_add_alias(), as it
> is not practical for callers to save target_name to free it later.
> 
> Eduardo Habkost (2):
>   qom: strdup() target property name on object_property_add_alias()
>   qdev: Free property names after registering gpio aliases
> 
>  hw/core/qdev.c | 2 ++
>  qom/object.c   | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> -- 
> 2.1.0
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Andreas Färber]

Am 10.04.2015 um 00:48 schrieb Andreas Färber:
> Am 09.04.2015 um 22:22 schrieb Paolo Bonzini:
>> On 09/04/2015 21:57, Eduardo Habkost wrote:
>>> This helps us avoid memory leaks when using object_property_add_alias(), as it
>>> is not practical for callers to save target_name to free it later.
>>>
>>> Eduardo Habkost (2):
>>>   qom: strdup() target property name on object_property_add_alias()
>>>   qdev: Free property names after registering gpio aliases
>>>
>>>  hw/core/qdev.c | 2 ++
>>>  qom/object.c   | 5 +++--
>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>
>> Good idea!
>>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Looks good to me in general, I'm just surprised that there are not more
> cleanups resulting out of this? At least my pc87312 series will need
> changes then. Will take a deeper look another day.

Thanks, applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

[Eduardo Habkost]

On Tue, Apr 28, 2015 at 05:16:35AM +0200, Andreas Färber wrote:
> Am 10.04.2015 um 00:48 schrieb Andreas Färber:
> > Am 09.04.2015 um 22:22 schrieb Paolo Bonzini:
> >> On 09/04/2015 21:57, Eduardo Habkost wrote:
> >>> This helps us avoid memory leaks when using object_property_add_alias(), as it
> >>> is not practical for callers to save target_name to free it later.
> >>>
> >>> Eduardo Habkost (2):
> >>>   qom: strdup() target property name on object_property_add_alias()
> >>>   qdev: Free property names after registering gpio aliases
> >>>
> >>>  hw/core/qdev.c | 2 ++
> >>>  qom/object.c   | 5 +++--
> >>>  2 files changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>
> >> Good idea!
> >>
> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > Looks good to me in general, I'm just surprised that there are not more
> > cleanups resulting out of this? At least my pc87312 series will need
> > changes then. Will take a deeper look another day.
> 
> Thanks, applied to qom-next:
> https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks. Is it expected to appear in a pull request soon? This is needed
for the X86CPU properties patch that's on the x86 queue.

-- 
Eduardo