Re: [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

[Philippe Mathieu-Daudé]

On 7/2/19 6:18 PM, Alistair Francis wrote:
> On Mon, Jul 1, 2019 at 5:32 AM Philippe Mathieu-Daudé <address@hidden> 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é <address@hidden>
> 
> Reviewed-by: Alistair Francis <address@hidden>
>
> Alistair
> 
>> ---
>>  hw/dma/xilinx_axidma.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
>> index 921be178d9..91f5ec587f 100644
>> --- a/hw/dma/xilinx_axidma.c
>> +++ b/hw/dma/xilinx_axidma.c
>> @@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
>>      XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
>>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>
>> -    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
>> -                      TYPE_XILINX_AXI_DMA_DATA_STREAM);
>> -    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
>> -                      TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
>> -    object_property_add_child(OBJECT(s), "axistream-connected-target",
>> -                              (Object *)&s->rx_data_dev, &error_abort);
>> -    object_property_add_child(OBJECT(s), 
>> "axistream-control-connected-target",
>> -                              (Object *)&s->rx_control_dev, &error_abort);
>> +    object_initialize_child(OBJECT(s), "axistream-connected-target",
>> +                            &s->rx_data_dev, sizeof(s->rx_data_dev),
>> +                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
>> +                            NULL);
>> +    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
>> +                            &s->rx_control_dev, sizeof(s->rx_control_dev),
>> +                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, 
>> &error_abort,
>> +                            NULL);
>>
>>      sysbus_init_irq(sbd, &s->streams[0].irq);
>>      sysbus_init_irq(sbd, &s->streams[1].irq);
>> --
>> 2.20.1

As GCC suggested, this patch requires this snippet squashed:

-- >8 --
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
@@ -565,11 +565,11 @@ static void xilinx_axidma_init(Object *obj)

     object_initialize_child(OBJECT(s), "axistream-connected-target",
                             &s->rx_data_dev, sizeof(s->rx_data_dev),
-                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
+                            TYPE_XILINX_AXI_DMA_DATA_STREAM, &error_abort,
                             NULL);
     object_initialize_child(OBJECT(s),
"axistream-control-connected-target",
                             &s->rx_control_dev, sizeof(s->rx_control_dev),
-                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
&error_abort,
+                            TYPE_XILINX_AXI_DMA_CONTROL_STREAM,
&error_abort,
                             NULL);

     sysbus_init_irq(sbd, &s->streams[0].irq);
---

Since it was commented in reply to the cover, I assume your R-b stands
with the snippet applied.

Thanks for reviewing,

Phil.