Re: [PATCH 03/14] hw/isa/vt82c686: Free irqs

[Peter Maydell]

On Wed, 26 Jun 2024 at 12:08, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> This suppresses LeakSanitizer warnings.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/isa/vt82c686.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 8582ac0322eb..189b487f1d22 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -721,7 +721,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>
>      qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>      qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
> -    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>      isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>                            errp);
>
> @@ -729,7 +728,9 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>          return;
>      }
>
> +    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>      s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
> +    qemu_free_irqs(isa_irq, 1);

This looks pretty weird -- we allocate the irq array, and
then immediately free it. The memory ought not to be
freed until the end of the simulation.

The ideal way to resolve this kind of leak with qemu_allocate_irqs()
is to avoid using the function entirely, and instead use IRQ
arrays allocated via qdev_init_gpio_* or the sysbus IRQ APIs.
qemu_allocate_irqs() is old and a sort of inherently leaky API.

The less ideal way is to keep the pointer to the array in the
device stuct.

If you look through 'git log' for qemu_allocate_irqs() you'll
see various places in the past where we've refactored things
to avoid this kind of uninteresting memory leak.

>      isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
>      i8254_pit_init(isa_bus, 0x40, 0, NULL);
>      i8257_dma_init(OBJECT(d), isa_bus, 0);

thanks
-- PMM