[v1] Fix check-qtest-ppc64 sanitizer errors

[PATCH 02/14] hw/ide: Free macio-ide IRQs

Posted by Akihiko Odaki 5 months ago

This suppresses LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/ide/macio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index aca90d04f0e8..d8fbc1a17ba6 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -464,6 +464,14 @@ static void macio_ide_initfn(Object *obj)
                              qdev_prop_allow_set_link_before_realize, 0);
 }
 
+static void macio_ide_finalize(Object *obj)
+{
+    MACIOIDEState *s = MACIO_IDE(obj);
+
+    qemu_free_irq(s->dma_irq);
+    qemu_free_irq(s->ide_irq);
+}
+
 static Property macio_ide_properties[] = {
     DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
     DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
@@ -486,6 +494,7 @@ static const TypeInfo macio_ide_type_info = {
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(MACIOIDEState),
     .instance_init = macio_ide_initfn,
+    .instance_finalize = macio_ide_finalize,
     .class_init = macio_ide_class_init,
 };
 

-- 
2.45.2

Re: [PATCH 02/14] hw/ide: Free macio-ide IRQs

Posted by Peter Maydell 5 months ago

On Wed, 26 Jun 2024 at 12:09, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> This suppresses LeakSanitizer warnings.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/ide/macio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index aca90d04f0e8..d8fbc1a17ba6 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -464,6 +464,14 @@ static void macio_ide_initfn(Object *obj)
>                               qdev_prop_allow_set_link_before_realize, 0);
>  }
>
> +static void macio_ide_finalize(Object *obj)
> +{
> +    MACIOIDEState *s = MACIO_IDE(obj);
> +
> +    qemu_free_irq(s->dma_irq);
> +    qemu_free_irq(s->ide_irq);
> +}
> +
>  static Property macio_ide_properties[] = {
>      DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
>      DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
> @@ -486,6 +494,7 @@ static const TypeInfo macio_ide_type_info = {
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(MACIOIDEState),
>      .instance_init = macio_ide_initfn,
> +    .instance_finalize = macio_ide_finalize,
>      .class_init = macio_ide_class_init,
>  };

Rather than this, I suspect macio_ide_initfn() should not
be using qemu_allocate_irq() in the first place. Looks like
maybe a QOM conversion that left a loose end un-tidied-up.

thanks
-- PMM

Re: [PATCH 02/14] hw/ide: Free macio-ide IRQs

Posted by Mark Cave-Ayland 4 months, 4 weeks ago

On 26/06/2024 13:59, Peter Maydell wrote:

> On Wed, 26 Jun 2024 at 12:09, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> This suppresses LeakSanitizer warnings.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   hw/ide/macio.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>> index aca90d04f0e8..d8fbc1a17ba6 100644
>> --- a/hw/ide/macio.c
>> +++ b/hw/ide/macio.c
>> @@ -464,6 +464,14 @@ static void macio_ide_initfn(Object *obj)
>>                                qdev_prop_allow_set_link_before_realize, 0);
>>   }
>>
>> +static void macio_ide_finalize(Object *obj)
>> +{
>> +    MACIOIDEState *s = MACIO_IDE(obj);
>> +
>> +    qemu_free_irq(s->dma_irq);
>> +    qemu_free_irq(s->ide_irq);
>> +}
>> +
>>   static Property macio_ide_properties[] = {
>>       DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
>>       DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
>> @@ -486,6 +494,7 @@ static const TypeInfo macio_ide_type_info = {
>>       .parent = TYPE_SYS_BUS_DEVICE,
>>       .instance_size = sizeof(MACIOIDEState),
>>       .instance_init = macio_ide_initfn,
>> +    .instance_finalize = macio_ide_finalize,
>>       .class_init = macio_ide_class_init,
>>   };
> 
> Rather than this, I suspect macio_ide_initfn() should not
> be using qemu_allocate_irq() in the first place. Looks like
> maybe a QOM conversion that left a loose end un-tidied-up.

This is definitely old code: there used to be problems interfacing the IDE code with 
qdev due to the hard-coded bus IRQs but I think this may is now possible with the 
advent of ide_bus_init_output_irq().

I'll have a quick look and see what has changed in this area.


ATB,

Mark.