diff mbox series

[3/6] hw/arm: Use sysbus_init_child_obj for correct reference counting

Message ID 20190701123108.12493-4-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child() | expand

Commit Message

Philippe Mathieu-Daudé July 1, 2019, 12:31 p.m. UTC
As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/exynos4_boards.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Maydell July 29, 2019, 1:03 p.m. UTC | #1
On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> As explained in commit aff39be0ed97:
>
>   Both functions, object_initialize() and object_property_add_child()
>   increase the reference counter of the new object, so one of the
>   references has to be dropped afterwards to get the reference
>   counting right. Otherwise the child object will not be properly
>   cleaned up when the parent gets destroyed.
>   Thus let's use now object_initialize_child() instead to get the
>   reference counting here right.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/exynos4_boards.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index ac0b0dc2a9..5dd53d2a23 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
>      exynos4_boards_init_ram(s, get_system_memory(),
>                              exynos4_board_ram_size[board_type]);
>
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
> -    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
> +    sysbus_init_child_obj(OBJECT(machine), "soc",
> +                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>      object_property_set_bool(OBJECT(&s->soc), true, "realized",
>                               &error_fatal);

I suspect the code change here is correct but it doesn't seem
to match the commit message -- the old code is not calling
object_property_add_child() at all, and the new code does not
call object_initialize_child()...

thanks
-- PMM
Philippe Mathieu-Daudé Aug. 12, 2019, 12:14 p.m. UTC | #2
On 7/29/19 3:03 PM, Peter Maydell wrote:
> On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> As explained in commit aff39be0ed97:
>>
>>   Both functions, object_initialize() and object_property_add_child()
>>   increase the reference counter of the new object, so one of the
>>   references has to be dropped afterwards to get the reference
>>   counting right. Otherwise the child object will not be properly
>>   cleaned up when the parent gets destroyed.
>>   Thus let's use now object_initialize_child() instead to get the
>>   reference counting here right.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/arm/exynos4_boards.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
>> index ac0b0dc2a9..5dd53d2a23 100644
>> --- a/hw/arm/exynos4_boards.c
>> +++ b/hw/arm/exynos4_boards.c
>> @@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
>>      exynos4_boards_init_ram(s, get_system_memory(),
>>                              exynos4_board_ram_size[board_type]);
>>
>> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>> -    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
>> +    sysbus_init_child_obj(OBJECT(machine), "soc",
>> +                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>>      object_property_set_bool(OBJECT(&s->soc), true, "realized",
>>                               &error_fatal);
> 
> I suspect the code change here is correct but it doesn't seem
> to match the commit message -- the old code is not calling
> object_property_add_child() at all, and the new code does not
> call object_initialize_child()...

OK, will improve, thanks!
diff mbox series

Patch

diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index ac0b0dc2a9..5dd53d2a23 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -129,8 +129,8 @@  exynos4_boards_init_common(MachineState *machine,
     exynos4_boards_init_ram(s, get_system_memory(),
                             exynos4_board_ram_size[board_type]);
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
-    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
+    sysbus_init_child_obj(OBJECT(machine), "soc",
+                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
     object_property_set_bool(OBJECT(&s->soc), true, "realized",
                              &error_fatal);