Re: [PATCH] ide/sii3112: Avoid leaking irqs array

[Philippe Mathieu-Daudé]

On 3/23/20 3:32 PM, BALATON Zoltan wrote:
> Coverity CID 1421984 reports a leak in allocated irqs, this patch
> attempts to plug that.
>
> Signed-off-by: BALATON Zoltan <address@hidden>
> ---
>   hw/ide/sii3112.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2..c886916873 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -31,6 +31,7 @@ typedef struct SiI3112Regs {
>   typedef struct SiI3112PCIState {
>       PCIIDEState i;
>       MemoryRegion mmio;
> +    qemu_irq *irqs;
>       SiI3112Regs regs[2];
>   } SiI3112PCIState;
>   
> @@ -252,7 +253,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
>       SiI3112PCIState *d = SII3112_PCI(dev);
>       PCIIDEState *s = PCI_IDE(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);
> +    d->irqs = qemu_allocate_irqs(sii3112_set_irq, d, 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], d->irqs[i]);
>   
>           bmdma_init(&s->bus[i], &s->bmdma[i], s);
>           s->bmdma[i].bus = &s->bus[i];
> @@ -291,6 +291,13 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
> **errp)
>       }
>   }
>   
> +static void sii3112_unrealize(DeviceState *dev, Error **errp)
> +{
> +    SiI3112PCIState *d = SII3112_PCI(dev);
> +
> +    qemu_free_irqs(d->irqs, 2);

You can't do that without calling unrealize() on all the devices in each 
IDEBus. Apparently there is no code available to do that. Maybe easier 
to not add any sii3112_unrealize(). Keeping a reference in the state 
should be enough to silent Coverity.

> +}
> +
>   static void sii3112_pci_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -301,6 +308,7 @@ static void sii3112_pci_class_init(ObjectClass *klass, void 
> *data)
>       pd->class_id = PCI_CLASS_STORAGE_RAID;
>       pd->revision = 1;
>       pd->realize = sii3112_pci_realize;
> +    dc->unrealize = sii3112_unrealize;
>       dc->reset = sii3112_reset;
>       dc->desc = "SiI3112A SATA controller";
>       set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);