diff mbox series

[PULL,4/8] bus: do not unref the added child bus on realize

Message ID 20180612104430.25745-5-kraxel@redhat.com
State New
Headers show
Series [PULL,1/8] usb: update docs | expand

Commit Message

Gerd Hoffmann June 12, 2018, 10:44 a.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

When the parent bus removes the child property, it takes care of
removing the added reference, in object_finalize_child_property().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180531195119.22021-2-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/core/bus.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Paolo Bonzini June 13, 2018, 4:05 p.m. UTC | #1 
On 12/06/2018 12:44, Gerd Hoffmann wrote:
> 
> When the parent bus removes the child property, it takes care of
> removing the added reference, in object_finalize_child_property().
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-id: 20180531195119.22021-2-marcandre.lureau@redhat.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/core/bus.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index 4651f24486..ad0c9df335 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -102,7 +102,6 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
>          QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
>          bus->parent->num_child_bus++;
>          object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
> -        object_unref(OBJECT(bus));

This is wrong.  object_finalize_child_property()'s unref balances the
ref in object_property_add_child().  qbus_realize's unref balances the
ref that was initially placed by object_new/object_initialize.

So you're introducing a leak.

Paolo
Marc-André Lureau June 13, 2018, 4:21 p.m. UTC | #2 
Hi

On Wed, Jun 13, 2018 at 6:05 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/06/2018 12:44, Gerd Hoffmann wrote:
>>
>> When the parent bus removes the child property, it takes care of
>> removing the added reference, in object_finalize_child_property().
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Message-id: 20180531195119.22021-2-marcandre.lureau@redhat.com
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  hw/core/bus.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/core/bus.c b/hw/core/bus.c
>> index 4651f24486..ad0c9df335 100644
>> --- a/hw/core/bus.c
>> +++ b/hw/core/bus.c
>> @@ -102,7 +102,6 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
>>          QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
>>          bus->parent->num_child_bus++;
>>          object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
>> -        object_unref(OBJECT(bus));
>
> This is wrong.  object_finalize_child_property()'s unref balances the
> ref in object_property_add_child().  qbus_realize's unref balances the
> ref that was initially placed by object_new/object_initialize.
>
> So you're introducing a leak.

Oops, too bad you didn't review earlier. Hmm.. I vote for reverting
the 4 patches.
diff mbox series

Patch

diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f24486..ad0c9df335 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -102,7 +102,6 @@  static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
         QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
         bus->parent->num_child_bus++;
         object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
-        object_unref(OBJECT(bus));
     } else if (bus != sysbus_get_default()) {
         /* TODO: once all bus devices are qdevified,
            only reset handler for main_system_bus should be registered here. */