1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] hw/arm: Use object_initialize_child for correct reference counting

Philippe Mathieu-Daudé posted 1 patch 6 years, 8 months ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190221183829.31019-1-philmd@redhat.com
Maintainers: Andrew Jeffery <andrew@aj.id.au>, Antony Pavlov <antonynpavlov@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Andrew Baumann <Andrew.Baumann@microsoft.com>, "Cédric Le Goater" <clg@kaod.org>, Joel Stanley <joel@jms.id.au>, Peter Maydell <peter.maydell@linaro.org>
hw/arm/aspeed_soc.c          | 43 ++++++++++++++++++------------------
hw/arm/bcm2835_peripherals.c | 41 +++++++++++++++++-----------------
hw/arm/digic.c               |  4 ++--
3 files changed, 45 insertions(+), 43 deletions(-)
[Qemu-devel] [PATCH] hw/arm: Use object_initialize_child for correct reference counting
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
As Thomas Huth explained:
"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."

This patch was generated using the following Coccinelle script:

 @use_object_initialize_child@
 identifier parent_obj;
 expression child;
 expression propname;
 expression child_type;
 expression errp;
 @@
 (
 -    object_initialize(&child, sizeof(child), child_type);
 -    object_property_add_child(parent_obj, propname, OBJECT(&child), NULL);
 +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
 +                            child_type, &error_abort, NULL);
 |
 -    object_initialize(&child, sizeof(child), child_type);
 -    object_property_add_child(parent_obj, propname, OBJECT(&child), errp);
 +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
 +                            child_type, errp, NULL);
 )

and a bit of manual fix-up for overly long lines.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/aspeed_soc.c          | 43 ++++++++++++++++++------------------
 hw/arm/bcm2835_peripherals.c | 41 +++++++++++++++++-----------------
 hw/arm/digic.c               |  4 ++--
 3 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index a27233d487..81665f2948 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -106,11 +106,11 @@ static void aspeed_soc_init(Object *obj)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
-    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
+                            sc->info->cpu_type, &error_abort, NULL);
 
-    object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
-    object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
+    object_initialize_child(obj, "scu", &s->scu, sizeof(s->scu),
+                            TYPE_ASPEED_SCU, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
     qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
                          sc->info->silicon_rev);
@@ -121,35 +121,35 @@ static void aspeed_soc_init(Object *obj)
     object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
                               "hw-prot-key", &error_abort);
 
-    object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC);
-    object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL);
+    object_initialize_child(obj, "vic", &s->vic, sizeof(s->vic),
+                            TYPE_ASPEED_VIC, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default());
 
-    object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
-    object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL);
+    object_initialize_child(obj, "timerctrl", &s->timerctrl,
+                            sizeof(s->timerctrl), TYPE_ASPEED_TIMER,
+                            &error_abort, NULL);
     object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
                                    OBJECT(&s->scu), &error_abort);
     qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default());
 
-    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
-    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
+    object_initialize_child(obj, "i2c", &s->i2c, sizeof(s->i2c),
+                            TYPE_ASPEED_I2C, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
 
-    object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
-    object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
+    object_initialize_child(obj, "fmc", &s->fmc, sizeof(s->fmc),
+                            sc->info->fmc_typename, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
     object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
                               &error_abort);
 
     for (i = 0; i < sc->info->spis_num; i++) {
-        object_initialize(&s->spi[i], sizeof(s->spi[i]),
-                          sc->info->spi_typename[i]);
-        object_property_add_child(obj, "spi[*]", OBJECT(&s->spi[i]), NULL);
+        object_initialize_child(obj, "spi[*]", &s->spi[i], sizeof(s->spi[i]),
+                                sc->info->spi_typename[i], &error_abort, NULL);
         qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
     }
 
-    object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
-    object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
+    object_initialize_child(obj, "sdmc", &s->sdmc, sizeof(s->sdmc),
+                            TYPE_ASPEED_SDMC, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
     qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
                          sc->info->silicon_rev);
@@ -159,15 +159,16 @@ static void aspeed_soc_init(Object *obj)
                               "max-ram-size", &error_abort);
 
     for (i = 0; i < sc->info->wdts_num; i++) {
-        object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
-        object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL);
+        object_initialize_child(obj, "wdt[*]", &s->wdt[i], sizeof(s->wdt[i]),
+                                TYPE_ASPEED_WDT, &error_abort, NULL);
         qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default());
         qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev",
                                     sc->info->silicon_rev);
     }
 
-    object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
-    object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
+    object_initialize_child(obj, "ftgmac100", &s->ftgmac100,
+                            sizeof(s->ftgmac100), TYPE_FTGMAC100,
+                            &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
 }
 
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6be7660e8c..0355a41d8b 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -41,8 +41,8 @@ static void bcm2835_peripherals_init(Object *obj)
                        MBOX_CHAN_COUNT << MBOX_AS_CHAN_SHIFT);
 
     /* Interrupt Controller */
-    object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
-    object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
+    object_initialize_child(obj, "ic", &s->ic, sizeof(s->ic), TYPE_BCM2835_IC,
+                            &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
 
     /* UART0 */
@@ -51,21 +51,21 @@ static void bcm2835_peripherals_init(Object *obj)
     qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
 
     /* AUX / UART1 */
-    object_initialize(&s->aux, sizeof(s->aux), TYPE_BCM2835_AUX);
-    object_property_add_child(obj, "aux", OBJECT(&s->aux), NULL);
+    object_initialize_child(obj, "aux", &s->aux, sizeof(s->aux),
+                            TYPE_BCM2835_AUX, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->aux), sysbus_get_default());
 
     /* Mailboxes */
-    object_initialize(&s->mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
-    object_property_add_child(obj, "mbox", OBJECT(&s->mboxes), NULL);
+    object_initialize_child(obj, "mbox", &s->mboxes, sizeof(s->mboxes),
+                            TYPE_BCM2835_MBOX, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->mboxes), sysbus_get_default());
 
     object_property_add_const_link(OBJECT(&s->mboxes), "mbox-mr",
                                    OBJECT(&s->mbox_mr), &error_abort);
 
     /* Framebuffer */
-    object_initialize(&s->fb, sizeof(s->fb), TYPE_BCM2835_FB);
-    object_property_add_child(obj, "fb", OBJECT(&s->fb), NULL);
+    object_initialize_child(obj, "fb", &s->fb, sizeof(s->fb), TYPE_BCM2835_FB,
+                            &error_abort, NULL);
     object_property_add_alias(obj, "vcram-size", OBJECT(&s->fb), "vcram-size",
                               &error_abort);
     qdev_set_parent_bus(DEVICE(&s->fb), sysbus_get_default());
@@ -74,8 +74,9 @@ static void bcm2835_peripherals_init(Object *obj)
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* Property channel */
-    object_initialize(&s->property, sizeof(s->property), TYPE_BCM2835_PROPERTY);
-    object_property_add_child(obj, "property", OBJECT(&s->property), NULL);
+    object_initialize_child(obj, "property", &s->property,
+                            sizeof(s->property), TYPE_BCM2835_PROPERTY,
+                            &error_abort, NULL);
     object_property_add_alias(obj, "board-rev", OBJECT(&s->property),
                               "board-rev", &error_abort);
     qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
@@ -86,31 +87,31 @@ static void bcm2835_peripherals_init(Object *obj)
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* Random Number Generator */
-    object_initialize(&s->rng, sizeof(s->rng), TYPE_BCM2835_RNG);
-    object_property_add_child(obj, "rng", OBJECT(&s->rng), NULL);
+    object_initialize_child(obj, "rng", &s->rng, sizeof(s->rng),
+                            TYPE_BCM2835_RNG, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->rng), sysbus_get_default());
 
     /* Extended Mass Media Controller */
-    object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
-    object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
+    object_initialize_child(obj, "sdhci", &s->sdhci, sizeof(s->sdhci),
+                            TYPE_SYSBUS_SDHCI, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
 
     /* SDHOST */
-    object_initialize(&s->sdhost, sizeof(s->sdhost), TYPE_BCM2835_SDHOST);
-    object_property_add_child(obj, "sdhost", OBJECT(&s->sdhost), NULL);
+    object_initialize_child(obj, "sdhost", &s->sdhost, sizeof(s->sdhost),
+                            TYPE_BCM2835_SDHOST, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->sdhost), sysbus_get_default());
 
     /* DMA Channels */
-    object_initialize(&s->dma, sizeof(s->dma), TYPE_BCM2835_DMA);
-    object_property_add_child(obj, "dma", OBJECT(&s->dma), NULL);
+    object_initialize_child(obj, "dma", &s->dma, sizeof(s->dma),
+                            TYPE_BCM2835_DMA, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->dma), sysbus_get_default());
 
     object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* GPIO */
-    object_initialize(&s->gpio, sizeof(s->gpio), TYPE_BCM2835_GPIO);
-    object_property_add_child(obj, "gpio", OBJECT(&s->gpio), NULL);
+    object_initialize_child(obj, "gpio", &s->gpio, sizeof(s->gpio),
+                            TYPE_BCM2835_GPIO, &error_abort, NULL);
     qdev_set_parent_bus(DEVICE(&s->gpio), sysbus_get_default());
 
     object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhci",
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 726abb9b48..14409ef080 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -35,8 +35,8 @@ static void digic_init(Object *obj)
     DeviceState *dev;
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
-    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
+                            "arm946-" TYPE_ARM_CPU, &error_abort, NULL);
 
     for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
 #define DIGIC_TIMER_NAME_MLEN    11
-- 
2.20.1
Re: [Qemu-devel] [PATCH] hw/arm: Use object_initialize_child for correct reference counting
Posted by Markus Armbruster 6 years, 8 months ago 
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> As Thomas Huth explained:
> "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."
>
> This patch was generated using the following Coccinelle script:
>
>  @use_object_initialize_child@
>  identifier parent_obj;
>  expression child;
>  expression propname;
>  expression child_type;
>  expression errp;
>  @@
>  (
>  -    object_initialize(&child, sizeof(child), child_type);
>  -    object_property_add_child(parent_obj, propname, OBJECT(&child), NULL);
>  +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
>  +                            child_type, &error_abort, NULL);
>  |
>  -    object_initialize(&child, sizeof(child), child_type);
>  -    object_property_add_child(parent_obj, propname, OBJECT(&child), errp);
>  +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
>  +                            child_type, errp, NULL);
>  )
>
> and a bit of manual fix-up for overly long lines.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Inspired-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/aspeed_soc.c          | 43 ++++++++++++++++++------------------
>  hw/arm/bcm2835_peripherals.c | 41 +++++++++++++++++-----------------
>  hw/arm/digic.c               |  4 ++--
>  3 files changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index a27233d487..81665f2948 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -106,11 +106,11 @@ static void aspeed_soc_init(Object *obj)
>      AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>      int i;
>  
> -    object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
> -    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +                            sc->info->cpu_type, &error_abort, NULL);

This flips from "ignore errors" to "abort on error".  Quite probably an
improvement, but should be mentioned and justified in the commit
message.

[...]
Re: [Qemu-devel] [PATCH] hw/arm: Use object_initialize_child for correct reference counting
Posted by Thomas Huth 6 years, 8 months ago 
On 21/02/2019 19.38, Philippe Mathieu-Daudé wrote:
> As Thomas Huth explained:
> "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."
> 
> This patch was generated using the following Coccinelle script:
> 
>  @use_object_initialize_child@
>  identifier parent_obj;
>  expression child;
>  expression propname;
>  expression child_type;
>  expression errp;
>  @@
>  (
>  -    object_initialize(&child, sizeof(child), child_type);
>  -    object_property_add_child(parent_obj, propname, OBJECT(&child), NULL);
>  +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
>  +                            child_type, &error_abort, NULL);
>  |
>  -    object_initialize(&child, sizeof(child), child_type);
>  -    object_property_add_child(parent_obj, propname, OBJECT(&child), errp);
>  +    object_initialize_child(parent_obj, propname,  &child, sizeof(child),
>  +                            child_type, errp, NULL);
>  )
> 
> and a bit of manual fix-up for overly long lines.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Inspired-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/aspeed_soc.c          | 43 ++++++++++++++++++------------------
>  hw/arm/bcm2835_peripherals.c | 41 +++++++++++++++++-----------------
>  hw/arm/digic.c               |  4 ++--
>  3 files changed, 45 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index a27233d487..81665f2948 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -106,11 +106,11 @@ static void aspeed_soc_init(Object *obj)
>      AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>      int i;
>  
> -    object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
> -    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +                            sc->info->cpu_type, &error_abort, NULL);
>  
> -    object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
> -    object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
> +    object_initialize_child(obj, "scu", &s->scu, sizeof(s->scu),
> +                            TYPE_ASPEED_SCU, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
>      qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
>                           sc->info->silicon_rev);
> @@ -121,35 +121,35 @@ static void aspeed_soc_init(Object *obj)
>      object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
>                                "hw-prot-key", &error_abort);
>  
> -    object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC);
> -    object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL);
> +    object_initialize_child(obj, "vic", &s->vic, sizeof(s->vic),
> +                            TYPE_ASPEED_VIC, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default());
>  
> -    object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
> -    object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL);
> +    object_initialize_child(obj, "timerctrl", &s->timerctrl,
> +                            sizeof(s->timerctrl), TYPE_ASPEED_TIMER,
> +                            &error_abort, NULL);
>      object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
>                                     OBJECT(&s->scu), &error_abort);
>      qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default());
>  
> -    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
> -    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
> +    object_initialize_child(obj, "i2c", &s->i2c, sizeof(s->i2c),
> +                            TYPE_ASPEED_I2C, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
>  
> -    object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
> -    object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
> +    object_initialize_child(obj, "fmc", &s->fmc, sizeof(s->fmc),
> +                            sc->info->fmc_typename, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
>      object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
>                                &error_abort);
>  
>      for (i = 0; i < sc->info->spis_num; i++) {
> -        object_initialize(&s->spi[i], sizeof(s->spi[i]),
> -                          sc->info->spi_typename[i]);
> -        object_property_add_child(obj, "spi[*]", OBJECT(&s->spi[i]), NULL);
> +        object_initialize_child(obj, "spi[*]", &s->spi[i], sizeof(s->spi[i]),
> +                                sc->info->spi_typename[i], &error_abort, NULL);
>          qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
>      }
>  
> -    object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
> -    object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
> +    object_initialize_child(obj, "sdmc", &s->sdmc, sizeof(s->sdmc),
> +                            TYPE_ASPEED_SDMC, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
>      qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
>                           sc->info->silicon_rev);
> @@ -159,15 +159,16 @@ static void aspeed_soc_init(Object *obj)
>                                "max-ram-size", &error_abort);
>  
>      for (i = 0; i < sc->info->wdts_num; i++) {
> -        object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
> -        object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL);
> +        object_initialize_child(obj, "wdt[*]", &s->wdt[i], sizeof(s->wdt[i]),
> +                                TYPE_ASPEED_WDT, &error_abort, NULL);
>          qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default());
>          qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev",
>                                      sc->info->silicon_rev);
>      }
>  
> -    object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
> -    object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
> +    object_initialize_child(obj, "ftgmac100", &s->ftgmac100,
> +                            sizeof(s->ftgmac100), TYPE_FTGMAC100,
> +                            &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
>  }
>  
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 6be7660e8c..0355a41d8b 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -41,8 +41,8 @@ static void bcm2835_peripherals_init(Object *obj)
>                         MBOX_CHAN_COUNT << MBOX_AS_CHAN_SHIFT);
>  
>      /* Interrupt Controller */
> -    object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
> -    object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
> +    object_initialize_child(obj, "ic", &s->ic, sizeof(s->ic), TYPE_BCM2835_IC,
> +                            &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
>  
>      /* UART0 */
> @@ -51,21 +51,21 @@ static void bcm2835_peripherals_init(Object *obj)
>      qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
>  
>      /* AUX / UART1 */
> -    object_initialize(&s->aux, sizeof(s->aux), TYPE_BCM2835_AUX);
> -    object_property_add_child(obj, "aux", OBJECT(&s->aux), NULL);
> +    object_initialize_child(obj, "aux", &s->aux, sizeof(s->aux),
> +                            TYPE_BCM2835_AUX, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->aux), sysbus_get_default());
>  
>      /* Mailboxes */
> -    object_initialize(&s->mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
> -    object_property_add_child(obj, "mbox", OBJECT(&s->mboxes), NULL);
> +    object_initialize_child(obj, "mbox", &s->mboxes, sizeof(s->mboxes),
> +                            TYPE_BCM2835_MBOX, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->mboxes), sysbus_get_default());
>  
>      object_property_add_const_link(OBJECT(&s->mboxes), "mbox-mr",
>                                     OBJECT(&s->mbox_mr), &error_abort);
>  
>      /* Framebuffer */
> -    object_initialize(&s->fb, sizeof(s->fb), TYPE_BCM2835_FB);
> -    object_property_add_child(obj, "fb", OBJECT(&s->fb), NULL);
> +    object_initialize_child(obj, "fb", &s->fb, sizeof(s->fb), TYPE_BCM2835_FB,
> +                            &error_abort, NULL);
>      object_property_add_alias(obj, "vcram-size", OBJECT(&s->fb), "vcram-size",
>                                &error_abort);
>      qdev_set_parent_bus(DEVICE(&s->fb), sysbus_get_default());
> @@ -74,8 +74,9 @@ static void bcm2835_peripherals_init(Object *obj)
>                                     OBJECT(&s->gpu_bus_mr), &error_abort);
>  
>      /* Property channel */
> -    object_initialize(&s->property, sizeof(s->property), TYPE_BCM2835_PROPERTY);
> -    object_property_add_child(obj, "property", OBJECT(&s->property), NULL);
> +    object_initialize_child(obj, "property", &s->property,
> +                            sizeof(s->property), TYPE_BCM2835_PROPERTY,
> +                            &error_abort, NULL);
>      object_property_add_alias(obj, "board-rev", OBJECT(&s->property),
>                                "board-rev", &error_abort);
>      qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
> @@ -86,31 +87,31 @@ static void bcm2835_peripherals_init(Object *obj)
>                                     OBJECT(&s->gpu_bus_mr), &error_abort);
>  
>      /* Random Number Generator */
> -    object_initialize(&s->rng, sizeof(s->rng), TYPE_BCM2835_RNG);
> -    object_property_add_child(obj, "rng", OBJECT(&s->rng), NULL);
> +    object_initialize_child(obj, "rng", &s->rng, sizeof(s->rng),
> +                            TYPE_BCM2835_RNG, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->rng), sysbus_get_default());
>  
>      /* Extended Mass Media Controller */
> -    object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
> -    object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
> +    object_initialize_child(obj, "sdhci", &s->sdhci, sizeof(s->sdhci),
> +                            TYPE_SYSBUS_SDHCI, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
>  
>      /* SDHOST */
> -    object_initialize(&s->sdhost, sizeof(s->sdhost), TYPE_BCM2835_SDHOST);
> -    object_property_add_child(obj, "sdhost", OBJECT(&s->sdhost), NULL);
> +    object_initialize_child(obj, "sdhost", &s->sdhost, sizeof(s->sdhost),
> +                            TYPE_BCM2835_SDHOST, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->sdhost), sysbus_get_default());
>  
>      /* DMA Channels */
> -    object_initialize(&s->dma, sizeof(s->dma), TYPE_BCM2835_DMA);
> -    object_property_add_child(obj, "dma", OBJECT(&s->dma), NULL);
> +    object_initialize_child(obj, "dma", &s->dma, sizeof(s->dma),
> +                            TYPE_BCM2835_DMA, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->dma), sysbus_get_default());
>  
>      object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
>                                     OBJECT(&s->gpu_bus_mr), &error_abort);
>  
>      /* GPIO */
> -    object_initialize(&s->gpio, sizeof(s->gpio), TYPE_BCM2835_GPIO);
> -    object_property_add_child(obj, "gpio", OBJECT(&s->gpio), NULL);
> +    object_initialize_child(obj, "gpio", &s->gpio, sizeof(s->gpio),
> +                            TYPE_BCM2835_GPIO, &error_abort, NULL);
>      qdev_set_parent_bus(DEVICE(&s->gpio), sysbus_get_default());
>  
>      object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhci",

Ack so far.

> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 726abb9b48..14409ef080 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -35,8 +35,8 @@ static void digic_init(Object *obj)
>      DeviceState *dev;
>      int i;
>  
> -    object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
> -    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +                            "arm946-" TYPE_ARM_CPU, &error_abort, NULL);
>  
>      for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
>  #define DIGIC_TIMER_NAME_MLEN    11
> 

I think digic_init() needs some more manual tweaking. You can see two
more instances of object_initialize() + add_child() later in that
function, just with some code in between. That either needs to be
re-arranged a little bit, or we should add an object_unref() each time
at the end there.

Also there is one more spot in xlnx_zynqmp_create_rpu() in xlnx-zynqmp.c
that should also be fixed, I think.

Eduardo also listed the machine init functions last year:

 https://patchwork.ozlabs.org/patch/943333/#1953608

Do we want to fix them, too? I think it's currently not really necessary
there since we don't clean up machines anyway, but maybe it would still
be a good idea to avoid that people copy-n-paste bad code snippets...
Well, maybe something for a separate patch, at least.

 Thomas

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.