Re: [PATCH] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()

[Mark Cave-Ayland]

On 23/03/2020 15:17, Peter Maydell wrote:

> Coverity points out (CID 1421984) that we are leaking the
> memory returned by qemu_allocate_irqs(). We can avoid this
> leak by switching to using qdev_init_gpio_in(); the base
> class finalize will free the irqs that this allocates under
> the hood.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> This is how the 'use qdev gpio' approach to fixing the leak looks.
> Disclaimer: I have only tested this with "make check", nothing more.
> 
>  hw/ide/sii3112.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2b..2ae6f5d9df6 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -251,8 +251,8 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
> **errp)
>  {
>      SiI3112PCIState *d = SII3112_PCI(dev);
>      PCIIDEState *s = PCI_IDE(dev);
> +    DeviceState *ds = DEVICE(dev);
>      MemoryRegion *mr;
> -    qemu_irq *irq;
>      int i;
>  
>      pci_config_set_interrupt_pin(dev->config, 1);
> @@ -280,10 +280,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
> **errp)
>      memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0, 16);
>      pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
>  
> -    irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
> +    qdev_init_gpio_in(ds, sii3112_set_irq, 2);
>      for (i = 0; i < 2; i++) {
>          ide_bus_new(&s->bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
> -        ide_init2(&s->bus[i], irq[i]);
> +        ide_init2(&s->bus[i], qdev_get_gpio_in(ds, i));
>  
>          bmdma_init(&s->bus[i], &s->bmdma[i], s);
>          s->bmdma[i].bus = &s->bus[i];

Looks like there is similar use of qemu_allocate_irqs() in via-ide and 
cmd646-ide,
and also reviewing my latest via-ide changes I spotted a silly mistake which was
obviously left in from a previous experimental version.

I'm not sure why Coverity doesn't pick up these other occurrences, however I'll 
send
along a patchset for this shortly.


ATB,

Mark.