qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: BALATON Zoltan
Subject: Re: [PATCH] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()
Date: Mon, 23 Mar 2020 18:04:48 +0100 (CET)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

On Mon, 23 Mar 2020, 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));

Maybe we could just use DEVICE(dev) here and above as well just like in ide_bus_new above just to keep it consistent and avoid the confusion caused by having dev, d, s and now also ds. DEVICE(dev) is short and clear enough I think.

Regards,
BALATON Zoltan


        bmdma_init(&s->bus[i], &s->bmdma[i], s);
        s->bmdma[i].bus = &s->bus[i];




reply via email to

[Prev in Thread] Current Thread [Next in Thread]