On Tue, May 07, 2019 at 06:34:01PM +0200, 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)); > ) > > 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. > > Suggested-by: Eduardo Habkost <ehabk...@redhat.com> > Inspired-by: Thomas Huth <th...@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Acked-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/ppc/pnv.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index dfb4ea5742c..31aa20ee25d 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -994,14 +994,12 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9,
> Error **errp)
> PnvCore *pnv_core = PNV_CORE(chip->cores + (i * 4) * typesize);
> int core_id = CPU_CORE(pnv_core)->core_id;
>
> - object_initialize(eq, sizeof(*eq), TYPE_PNV_QUAD);
> snprintf(eq_name, sizeof(eq_name), "eq[%d]", core_id);
> + object_initialize_child(OBJECT(chip), eq_name, eq, sizeof(*eq),
> + TYPE_PNV_QUAD, &error_fatal, NULL);
>
> - object_property_add_child(OBJECT(chip), eq_name, OBJECT(eq),
> - &error_fatal);
> object_property_set_int(OBJECT(eq), core_id, "id", &error_fatal);
> object_property_set_bool(OBJECT(eq), true, "realized", &error_fatal);
> - object_unref(OBJECT(eq));
>
> pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->id),
> &eq->xscom_regs);
> @@ -1165,10 +1163,9 @@ static void pnv_chip_core_realize(PnvChip *chip, Error
> **errp)
> continue;
> }
>
> - object_initialize(pnv_core, typesize, typename);
> snprintf(core_name, sizeof(core_name), "core[%d]", core_hwid);
> - object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
> - &error_fatal);
> + object_initialize_child(OBJECT(chip), core_name, pnv_core, typesize,
> + typename, &error_fatal, NULL);
> object_property_set_int(OBJECT(pnv_core), smp_threads, "nr-threads",
> &error_fatal);
> object_property_set_int(OBJECT(pnv_core), core_hwid,
> @@ -1180,7 +1177,6 @@ static void pnv_chip_core_realize(PnvChip *chip, Error
> **errp)
> OBJECT(chip), &error_fatal);
> object_property_set_bool(OBJECT(pnv_core), true, "realized",
> &error_fatal);
> - object_unref(OBJECT(pnv_core));
>
> /* Each core has an XSCOM MMIO region */
> if (!pnv_chip_is_power9(chip)) {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
