On 07/05/19 11:34, Philippe Mathieu-Daudé 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.
>
> This patch was generated using the following Coccinelle script
> (with a bit of manual fix-up for overly long lines):
>
> @use_object_initialize_child@
> expression parent_obj;
> expression child_ptr;
> expression child_name;
> expression child_type;
> expression child_size;
> expression errp;
> @@
> (
> - object_initialize(child_ptr, child_size, child_type);
> + object_initialize_child(parent_obj, child_name, child_ptr, child_size,
> + child_type, &error_abort, NULL);
> ... when != parent_obj
> - object_property_add_child(parent_obj, child_name, OBJECT(child_ptr),
> NULL);
> ...
> ?- object_unref(OBJECT(child_ptr));
> |
> - object_initialize(child_ptr, child_size, child_type);
> + object_initialize_child(parent_obj, child_name, child_ptr, child_size,
> + child_type, errp, NULL);
> ... when != parent_obj
> - object_property_add_child(parent_obj, child_name, OBJECT(child_ptr),
> errp);
> ...
> ?- object_unref(OBJECT(child_ptr));
> )
>
> @use_sysbus_init_child_obj@
> expression parent_obj;
> expression dev;
> expression child_ptr;
> expression child_name;
> expression child_type;
> expression child_size;
> expression errp;
> @@
> (
> - object_initialize_child(parent_obj, child_name, child_ptr, child_size,
> - child_type, errp, NULL);
> + sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
> + child_type);
> ...
> - qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
> |
> - object_initialize_child(parent_obj, child_name, child_ptr, child_size,
> - child_type, errp, NULL);
> + sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
> + child_type);
> - dev = DEVICE(child_ptr);
> - qdev_set_parent_bus(dev, sysbus_get_default());
> )
>
> While the object_initialize() function doesn't take an
> 'Error *errp' argument, the object_initialize_child() does.
> Since this code is used when a machine is created (and is not
> yet running), we deliberately choose to use the &error_abort
> argument instead of ignoring errors if an object creation failed.
> This choice also matches when using sysbus_init_child_obj(),
> since its code is:
>
> void sysbus_init_child_obj(Object *parent,
> const char *childname, void *child,
> size_t childsize, const char *childtype)
> {
> object_initialize_child(parent, childname, child, childsize,
> childtype, &error_abort, NULL);
>
> qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
> }
>
> Suggested-by: Eduardo Habkost <ehabk...@redhat.com>
> Inspired-by: Thomas Huth <th...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> ---
> hw/arm/bcm2835_peripherals.c | 53 ++++++++++++++----------------------
> 1 file changed, 20 insertions(+), 33 deletions(-)
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 2931a82a25a..0fb54c7964e 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -41,44 +41,36 @@ 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);
> - qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
> + sysbus_init_child_obj(obj, "ic", &s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
>
> /* UART0 */
> - object_initialize(&s->uart0, sizeof(s->uart0), TYPE_PL011);
> - object_property_add_child(obj, "uart0", OBJECT(&s->uart0), NULL);
> - qdev_set_parent_bus(DEVICE(&s->uart0), sysbus_get_default());
> + sysbus_init_child_obj(obj, "uart0", &s->uart0, sizeof(s->uart0),
> + TYPE_PL011);
>
> /* AUX / UART1 */
> - object_initialize(&s->aux, sizeof(s->aux), TYPE_BCM2835_AUX);
> - object_property_add_child(obj, "aux", OBJECT(&s->aux), NULL);
> - qdev_set_parent_bus(DEVICE(&s->aux), sysbus_get_default());
> + sysbus_init_child_obj(obj, "aux", &s->aux, sizeof(s->aux),
> + TYPE_BCM2835_AUX);
>
> /* Mailboxes */
> - object_initialize(&s->mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
> - object_property_add_child(obj, "mbox", OBJECT(&s->mboxes), NULL);
> - qdev_set_parent_bus(DEVICE(&s->mboxes), sysbus_get_default());
> + sysbus_init_child_obj(obj, "mbox", &s->mboxes, sizeof(s->mboxes),
> + TYPE_BCM2835_MBOX);
>
> 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);
> + sysbus_init_child_obj(obj, "fb", &s->fb, sizeof(s->fb), TYPE_BCM2835_FB);
> object_property_add_alias(obj, "vcram-size", OBJECT(&s->fb),
> "vcram-size",
> &error_abort);
> - qdev_set_parent_bus(DEVICE(&s->fb), sysbus_get_default());
>
> object_property_add_const_link(OBJECT(&s->fb), "dma-mr",
> 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);
> + sysbus_init_child_obj(obj, "property", &s->property, sizeof(s->property),
> + TYPE_BCM2835_PROPERTY);
> object_property_add_alias(obj, "board-rev", OBJECT(&s->property),
> "board-rev", &error_abort);
> - qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
>
> object_property_add_const_link(OBJECT(&s->property), "fb",
> OBJECT(&s->fb), &error_abort);
> @@ -86,32 +78,27 @@ 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);
> - qdev_set_parent_bus(DEVICE(&s->rng), sysbus_get_default());
> + sysbus_init_child_obj(obj, "rng", &s->rng, sizeof(s->rng),
> + TYPE_BCM2835_RNG);
>
> /* Extended Mass Media Controller */
> - object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
> - object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
> - qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
> + sysbus_init_child_obj(obj, "sdhci", &s->sdhci, sizeof(s->sdhci),
> + TYPE_SYSBUS_SDHCI);
>
> /* SDHOST */
> - object_initialize(&s->sdhost, sizeof(s->sdhost), TYPE_BCM2835_SDHOST);
> - object_property_add_child(obj, "sdhost", OBJECT(&s->sdhost), NULL);
> - qdev_set_parent_bus(DEVICE(&s->sdhost), sysbus_get_default());
> + sysbus_init_child_obj(obj, "sdhost", &s->sdhost, sizeof(s->sdhost),
> + TYPE_BCM2835_SDHOST);
>
> /* DMA Channels */
> - object_initialize(&s->dma, sizeof(s->dma), TYPE_BCM2835_DMA);
> - object_property_add_child(obj, "dma", OBJECT(&s->dma), NULL);
> - qdev_set_parent_bus(DEVICE(&s->dma), sysbus_get_default());
> + sysbus_init_child_obj(obj, "dma", &s->dma, sizeof(s->dma),
> + TYPE_BCM2835_DMA);
>
> 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);
> - qdev_set_parent_bus(DEVICE(&s->gpio), sysbus_get_default());
> + sysbus_init_child_obj(obj, "gpio", &s->gpio, sizeof(s->gpio),
> + TYPE_BCM2835_GPIO);
>
> object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhci",
> OBJECT(&s->sdhci.sdbus), &error_abort);
>
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
