[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/29] hw/ide/ahci.c: Fix memory leak spotted by
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 01/29] hw/ide/ahci.c: Fix memory leak spotted by valgrind |
Date: |
Thu, 28 May 2015 13:39:00 +0100 |
On 28 May 2015 at 13:24, Shannon Zhao <address@hidden> wrote:
>
>
> On 2015/5/28 20:16, Peter Maydell wrote:
>> On 28 May 2015 at 13:08, Shannon Zhao <address@hidden> wrote:
>>> > From: Shannon Zhao <address@hidden>
>>> > --- a/hw/ide/ahci.c
>>> > +++ b/hw/ide/ahci.c
>>> > @@ -1367,6 +1367,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev,
>>> > AddressSpace *as, int ports)
>>> > ad->port.dma->ops = &ahci_dma_ops;
>>> > ide_register_restart_cb(&ad->port);
>>> > }
>>> > + g_free(irqs);
>>> > }
>> Freeing the return value of qemu_allocate_irqs()
>> directly rather than using qemu_free_irqs() looks
>> dubious to me.
>
> qemu_free_irqs() will free the return value self of qemu_allocate_irqs()
> and also will free all the contents of this array while some of them are
> still in use.
Yes, but you're still looking into the internal implementation
details of this API, which is what I'm suggesting is dubious.
For instance, in this case, why are we calling
qemu_allocate_irqs() in the first place, rather than
just calling qemu_allocate_irq() inside the for() loop in
ahci_init()?
If code wants to deal with the array of irqs as an array,
it should probably be saving the pointer somewhere in its
state struct and freeing the whole thing on deinit. If it
doesn't actually want an array of irqs (as here) it should
probably not be calling qemu_allocate_irqs() in the first
place.
thanks
-- PMM
- [Qemu-devel] [PATCH 00/29] Fix memory leak relevant to calling qemu_allocate_irqs, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 09/29] hw/sparc/leon3.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 05/29] hw/isa/lpc_ich9.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 10/29] hw/sparc/sun4m.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 08/29] hw/intc/exynos4210_gic.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 18/29] hw/m68k/mcf5206.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 17/29] hw/lm32/milkymist.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 16/29] hw/lm32/lm32_boards.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 12/29] hw/ppc/ppc440_bamboo.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 15/29] hw/mips/mips_jazz.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28
- [Qemu-devel] [PATCH 07/29] hw/timer/arm_timer.c: Fix memory leak spotted by valgrind, Shannon Zhao, 2015/05/28