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.
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/ppc/pnv.c | 12 ++++++------
hw/ppc/pnv_psi.c | 4 ++--
hw/ppc/spapr.c | 6 +++---
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index da54086..9e03e9c 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
{
Pnv8Chip *chip8 = PNV8_CHIP(obj);
- object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
- object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
+ object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
+ TYPE_PNV_PSI, &error_abort, NULL);
object_property_add_const_link(OBJECT(&chip8->psi), "xics",
OBJECT(qdev_get_machine()), &error_abort);
- object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
- object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
+ object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
+ TYPE_PNV_LPC, &error_abort, NULL);
object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
OBJECT(&chip8->psi), &error_abort);
- object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
- object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
+ object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
+ TYPE_PNV_OCC, &error_abort, NULL);
object_property_add_const_link(OBJECT(&chip8->occ), "psi",
OBJECT(&chip8->psi), &error_abort);
}
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 8ced095..44bc0cb 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
{
PnvPsi *psi = PNV_PSI(obj);
- object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
- object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
+ object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
+ TYPE_ICS_SIMPLE, &error_abort, NULL);
}
static const uint8_t irq_to_xivr[] = {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index abf9ebc..6c58dca 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
static void spapr_rtc_create(sPAPRMachineState *spapr)
{
- object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
- object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
- &error_fatal);
+ object_initialize_child(OBJECT(spapr), "rtc",
+ &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
+ &error_fatal, NULL);
object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
&error_fatal);
object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
--
1.8.3.1
On Thu, Feb 21, 2019 at 12:24:48PM +0100, Thomas Huth wrote:
> 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.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Nice cleanup for a common pattern, even if it weren't fixing a bug. Applied.
> ---
> hw/ppc/pnv.c | 12 ++++++------
> hw/ppc/pnv_psi.c | 4 ++--
> hw/ppc/spapr.c | 6 +++---
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index da54086..9e03e9c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
> {
> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>
> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
> + TYPE_PNV_PSI, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> OBJECT(qdev_get_machine()), &error_abort);
>
> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
> + TYPE_PNV_LPC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
> OBJECT(&chip8->psi), &error_abort);
>
> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
> + TYPE_PNV_OCC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> OBJECT(&chip8->psi), &error_abort);
> }
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 8ced095..44bc0cb 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
> {
> PnvPsi *psi = PNV_PSI(obj);
>
> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
> + TYPE_ICS_SIMPLE, &error_abort, NULL);
> }
>
> static const uint8_t irq_to_xivr[] = {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index abf9ebc..6c58dca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>
> static void spapr_rtc_create(sPAPRMachineState *spapr)
> {
> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
> - &error_fatal);
> + object_initialize_child(OBJECT(spapr), "rtc",
> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
> + &error_fatal, NULL);
> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
> &error_fatal);
> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
--
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
On 2/21/19 12:24 PM, Thomas Huth wrote:
> 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.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Looks fine to me.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/ppc/pnv.c | 12 ++++++------
> hw/ppc/pnv_psi.c | 4 ++--
> hw/ppc/spapr.c | 6 +++---
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index da54086..9e03e9c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
> {
> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>
> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
> + TYPE_PNV_PSI, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> OBJECT(qdev_get_machine()), &error_abort);
>
> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
> + TYPE_PNV_LPC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
> OBJECT(&chip8->psi), &error_abort);
>
> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
> + TYPE_PNV_OCC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> OBJECT(&chip8->psi), &error_abort);
> }
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 8ced095..44bc0cb 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
> {
> PnvPsi *psi = PNV_PSI(obj);
>
> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
> + TYPE_ICS_SIMPLE, &error_abort, NULL);
> }
>
> static const uint8_t irq_to_xivr[] = {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index abf9ebc..6c58dca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>
> static void spapr_rtc_create(sPAPRMachineState *spapr)
> {
> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
> - &error_fatal);
> + object_initialize_child(OBJECT(spapr), "rtc",
> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
> + &error_fatal, NULL);
> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
> &error_fatal);
> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
>
On 2/21/19 12:24 PM, Thomas Huth wrote:
> 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.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/ppc/pnv.c | 12 ++++++------
> hw/ppc/pnv_psi.c | 4 ++--
> hw/ppc/spapr.c | 6 +++---
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index da54086..9e03e9c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
> {
> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>
> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
> + TYPE_PNV_PSI, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> OBJECT(qdev_get_machine()), &error_abort);
>
> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
> + TYPE_PNV_LPC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
> OBJECT(&chip8->psi), &error_abort);
>
> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
> + TYPE_PNV_OCC, &error_abort, NULL);
> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> OBJECT(&chip8->psi), &error_abort);
> }
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 8ced095..44bc0cb 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
> {
> PnvPsi *psi = PNV_PSI(obj);
>
> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
> + TYPE_ICS_SIMPLE, &error_abort, NULL);
> }
>
> static const uint8_t irq_to_xivr[] = {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index abf9ebc..6c58dca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>
> static void spapr_rtc_create(sPAPRMachineState *spapr)
> {
> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
> - &error_fatal);
> + object_initialize_child(OBJECT(spapr), "rtc",
> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
> + &error_fatal, NULL);
> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
> &error_fatal);
> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
>
What about intc/spapr_xive.c?
-- >8 --
@@ -244,13 +244,15 @@ static void spapr_xive_instance_init(Obj
{
sPAPRXive *xive = SPAPR_XIVE(obj);
- object_initialize(&xive->source, sizeof(xive->source),
TYPE_XIVE_SOURCE);
- object_property_add_child(obj, "source", OBJECT(&xive->source), NULL);
+ object_initialize_child(obj, "source", &xive->source,
+ sizeof(xive->source), TYPE_XIVE_SOURCE,
+ &error_abort, NULL);
- object_initialize(&xive->end_source, sizeof(xive->end_source),
- TYPE_XIVE_END_SOURCE);
- object_property_add_child(obj, "end_source", OBJECT(&xive->end_source),
- NULL);
+ object_initialize_child(obj, "end_source", &xive->end_source,
+ sizeof(xive->end_source), TYPE_XIVE_END_SOURCE,
+ &error_abort, NULL);
}
---
On 2/21/19 7:14 PM, Philippe Mathieu-Daudé wrote:
> On 2/21/19 12:24 PM, Thomas Huth wrote:
>> 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.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/ppc/pnv.c | 12 ++++++------
>> hw/ppc/pnv_psi.c | 4 ++--
>> hw/ppc/spapr.c | 6 +++---
>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index da54086..9e03e9c 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
>> {
>> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>>
>> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
>> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
>> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
>> + TYPE_PNV_PSI, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
>> OBJECT(qdev_get_machine()), &error_abort);
>>
>> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
>> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
>> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
>> + TYPE_PNV_LPC, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
>> OBJECT(&chip8->psi), &error_abort);
>>
>> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
>> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
>> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
>> + TYPE_PNV_OCC, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
>> OBJECT(&chip8->psi), &error_abort);
>> }
>> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
>> index 8ced095..44bc0cb 100644
>> --- a/hw/ppc/pnv_psi.c
>> +++ b/hw/ppc/pnv_psi.c
>> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
>> {
>> PnvPsi *psi = PNV_PSI(obj);
>>
>> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
>> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
>> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
>> + TYPE_ICS_SIMPLE, &error_abort, NULL);
>> }
>>
>> static const uint8_t irq_to_xivr[] = {
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index abf9ebc..6c58dca 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>>
>> static void spapr_rtc_create(sPAPRMachineState *spapr)
>> {
>> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
>> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
>> - &error_fatal);
>> + object_initialize_child(OBJECT(spapr), "rtc",
>> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
>> + &error_fatal, NULL);
>> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
>> &error_fatal);
>> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
>>
>
> What about intc/spapr_xive.c?
I have used the same pattern in many files :) In ARM also.
C.
>
> -- >8 --
> @@ -244,13 +244,15 @@ static void spapr_xive_instance_init(Obj
> {
> sPAPRXive *xive = SPAPR_XIVE(obj);
>
> - object_initialize(&xive->source, sizeof(xive->source),
> TYPE_XIVE_SOURCE);
> - object_property_add_child(obj, "source", OBJECT(&xive->source), NULL);
> + object_initialize_child(obj, "source", &xive->source,
> + sizeof(xive->source), TYPE_XIVE_SOURCE,
> + &error_abort, NULL);
>
> - object_initialize(&xive->end_source, sizeof(xive->end_source),
> - TYPE_XIVE_END_SOURCE);
> - object_property_add_child(obj, "end_source", OBJECT(&xive->end_source),
> - NULL);
> + object_initialize_child(obj, "end_source", &xive->end_source,
> + sizeof(xive->end_source), TYPE_XIVE_END_SOURCE,
> + &error_abort, NULL);
> }
> ---
>
On 2/21/19 7:17 PM, Cédric Le Goater wrote:
> On 2/21/19 7:14 PM, Philippe Mathieu-Daudé wrote:
>> On 2/21/19 12:24 PM, Thomas Huth wrote:
>>> 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.
>>>
>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> hw/ppc/pnv.c | 12 ++++++------
>>> hw/ppc/pnv_psi.c | 4 ++--
>>> hw/ppc/spapr.c | 6 +++---
>>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>> index da54086..9e03e9c 100644
>>> --- a/hw/ppc/pnv.c
>>> +++ b/hw/ppc/pnv.c
>>> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
>>> {
>>> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>>>
>>> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
>>> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
>>> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
>>> + TYPE_PNV_PSI, &error_abort, NULL);
>>> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
>>> OBJECT(qdev_get_machine()), &error_abort);
>>>
>>> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
>>> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
>>> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
>>> + TYPE_PNV_LPC, &error_abort, NULL);
>>> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
>>> OBJECT(&chip8->psi), &error_abort);
>>>
>>> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
>>> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
>>> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
>>> + TYPE_PNV_OCC, &error_abort, NULL);
>>> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
>>> OBJECT(&chip8->psi), &error_abort);
>>> }
>>> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
>>> index 8ced095..44bc0cb 100644
>>> --- a/hw/ppc/pnv_psi.c
>>> +++ b/hw/ppc/pnv_psi.c
>>> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
>>> {
>>> PnvPsi *psi = PNV_PSI(obj);
>>>
>>> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
>>> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
>>> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
>>> + TYPE_ICS_SIMPLE, &error_abort, NULL);
>>> }
>>>
>>> static const uint8_t irq_to_xivr[] = {
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index abf9ebc..6c58dca 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>>>
>>> static void spapr_rtc_create(sPAPRMachineState *spapr)
>>> {
>>> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
>>> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
>>> - &error_fatal);
>>> + object_initialize_child(OBJECT(spapr), "rtc",
>>> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
>>> + &error_fatal, NULL);
>>> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
>>> &error_fatal);
>>> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
>>>
>>
>> What about intc/spapr_xive.c?
>
> I have used the same pattern in many files :) In ARM also.
OK, tested a cocci script so will send a patch.
Regards,
Phil.
On 21/02/2019 19.14, Philippe Mathieu-Daudé wrote:
> On 2/21/19 12:24 PM, Thomas Huth wrote:
>> 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.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/ppc/pnv.c | 12 ++++++------
>> hw/ppc/pnv_psi.c | 4 ++--
>> hw/ppc/spapr.c | 6 +++---
>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index da54086..9e03e9c 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
>> {
>> Pnv8Chip *chip8 = PNV8_CHIP(obj);
>>
>> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
>> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
>> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
>> + TYPE_PNV_PSI, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
>> OBJECT(qdev_get_machine()), &error_abort);
>>
>> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
>> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
>> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
>> + TYPE_PNV_LPC, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
>> OBJECT(&chip8->psi), &error_abort);
>>
>> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
>> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
>> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
>> + TYPE_PNV_OCC, &error_abort, NULL);
>> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
>> OBJECT(&chip8->psi), &error_abort);
>> }
>> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
>> index 8ced095..44bc0cb 100644
>> --- a/hw/ppc/pnv_psi.c
>> +++ b/hw/ppc/pnv_psi.c
>> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
>> {
>> PnvPsi *psi = PNV_PSI(obj);
>>
>> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
>> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
>> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
>> + TYPE_ICS_SIMPLE, &error_abort, NULL);
>> }
>>
>> static const uint8_t irq_to_xivr[] = {
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index abf9ebc..6c58dca 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
>>
>> static void spapr_rtc_create(sPAPRMachineState *spapr)
>> {
>> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
>> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
>> - &error_fatal);
>> + object_initialize_child(OBJECT(spapr), "rtc",
>> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
>> + &error_fatal, NULL);
>> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
>> &error_fatal);
>> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
>>
>
> What about intc/spapr_xive.c?
Good hint. I missed that one since it's in hw/intc, not in hw/ppc...
David, could you please squash this on top:
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 290a290..ac5f5ef 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -244,13 +244,12 @@ static void spapr_xive_instance_init(Object *obj)
{
sPAPRXive *xive = SPAPR_XIVE(obj);
- object_initialize(&xive->source, sizeof(xive->source), TYPE_XIVE_SOURCE);
- object_property_add_child(obj, "source", OBJECT(&xive->source), NULL);
+ object_initialize_child(obj, "source", &xive->source, sizeof(xive->source),
+ TYPE_XIVE_SOURCE, &error_abort, NULL);
- object_initialize(&xive->end_source, sizeof(xive->end_source),
- TYPE_XIVE_END_SOURCE);
- object_property_add_child(obj, "end_source", OBJECT(&xive->end_source),
- NULL);
+ object_initialize_child(obj, "end_source", &xive->end_source,
+ sizeof(xive->end_source), TYPE_XIVE_END_SOURCE,
+ &error_abort, NULL);
}
static void spapr_xive_realize(DeviceState *dev, Error **errp)
... or shall I rather send a v2 or a separate patch for this?
Thomas
On Fri, Feb 22, 2019 at 07:20:14AM +0100, Thomas Huth wrote:
> On 21/02/2019 19.14, Philippe Mathieu-Daudé wrote:
> > On 2/21/19 12:24 PM, Thomas Huth wrote:
> >> 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.
> >>
> >> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >> hw/ppc/pnv.c | 12 ++++++------
> >> hw/ppc/pnv_psi.c | 4 ++--
> >> hw/ppc/spapr.c | 6 +++---
> >> 3 files changed, 11 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> index da54086..9e03e9c 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -736,18 +736,18 @@ static void pnv_chip_power8_instance_init(Object *obj)
> >> {
> >> Pnv8Chip *chip8 = PNV8_CHIP(obj);
> >>
> >> - object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
> >> - object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
> >> + object_initialize_child(obj, "psi", &chip8->psi, sizeof(chip8->psi),
> >> + TYPE_PNV_PSI, &error_abort, NULL);
> >> object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> >> OBJECT(qdev_get_machine()), &error_abort);
> >>
> >> - object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
> >> - object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
> >> + object_initialize_child(obj, "lpc", &chip8->lpc, sizeof(chip8->lpc),
> >> + TYPE_PNV_LPC, &error_abort, NULL);
> >> object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
> >> OBJECT(&chip8->psi), &error_abort);
> >>
> >> - object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
> >> - object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
> >> + object_initialize_child(obj, "occ", &chip8->occ, sizeof(chip8->occ),
> >> + TYPE_PNV_OCC, &error_abort, NULL);
> >> object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> >> OBJECT(&chip8->psi), &error_abort);
> >> }
> >> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> >> index 8ced095..44bc0cb 100644
> >> --- a/hw/ppc/pnv_psi.c
> >> +++ b/hw/ppc/pnv_psi.c
> >> @@ -444,8 +444,8 @@ static void pnv_psi_init(Object *obj)
> >> {
> >> PnvPsi *psi = PNV_PSI(obj);
> >>
> >> - object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE);
> >> - object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL);
> >> + object_initialize_child(obj, "ics-psi", &psi->ics, sizeof(psi->ics),
> >> + TYPE_ICS_SIMPLE, &error_abort, NULL);
> >> }
> >>
> >> static const uint8_t irq_to_xivr[] = {
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index abf9ebc..6c58dca 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1696,9 +1696,9 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
> >>
> >> static void spapr_rtc_create(sPAPRMachineState *spapr)
> >> {
> >> - object_initialize(&spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC);
> >> - object_property_add_child(OBJECT(spapr), "rtc", OBJECT(&spapr->rtc),
> >> - &error_fatal);
> >> + object_initialize_child(OBJECT(spapr), "rtc",
> >> + &spapr->rtc, sizeof(spapr->rtc), TYPE_SPAPR_RTC,
> >> + &error_fatal, NULL);
> >> object_property_set_bool(OBJECT(&spapr->rtc), true, "realized",
> >> &error_fatal);
> >> object_property_add_alias(OBJECT(spapr), "rtc-time", OBJECT(&spapr->rtc),
> >>
> >
> > What about intc/spapr_xive.c?
>
> Good hint. I missed that one since it's in hw/intc, not in hw/ppc...
>
> David, could you please squash this on top:
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 290a290..ac5f5ef 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -244,13 +244,12 @@ static void spapr_xive_instance_init(Object *obj)
> {
> sPAPRXive *xive = SPAPR_XIVE(obj);
>
> - object_initialize(&xive->source, sizeof(xive->source), TYPE_XIVE_SOURCE);
> - object_property_add_child(obj, "source", OBJECT(&xive->source), NULL);
> + object_initialize_child(obj, "source", &xive->source, sizeof(xive->source),
> + TYPE_XIVE_SOURCE, &error_abort, NULL);
>
> - object_initialize(&xive->end_source, sizeof(xive->end_source),
> - TYPE_XIVE_END_SOURCE);
> - object_property_add_child(obj, "end_source", OBJECT(&xive->end_source),
> - NULL);
> + object_initialize_child(obj, "end_source", &xive->end_source,
> + sizeof(xive->end_source), TYPE_XIVE_END_SOURCE,
> + &error_abort, NULL);
> }
>
> static void spapr_xive_realize(DeviceState *dev, Error **errp)
>
>
> ... or shall I rather send a v2 or a separate patch for this?
Folded as requested, thanks.
>
> Thomas
>
--
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
© 2016 - 2025 Red Hat, Inc.