On 27/07/2020 16.41, Peter Maydell wrote: > On Mon, 27 Jul 2020 at 14:03, Keqian Zhu <zhukeqi...@huawei.com> wrote: >> >> Avoid covering object refcount of qemu_irq, otherwise it may causes >> memory leak. >> >> Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com> >> --- >> hw/core/irq.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/core/irq.c b/hw/core/irq.c >> index fb3045b912..59af4dfc74 100644 >> --- a/hw/core/irq.c >> +++ b/hw/core/irq.c >> @@ -125,7 +125,9 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, >> qemu_irq_handler handler, int n) >> int i; >> qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n); >> for (i = 0; i < n; i++) { >> - *old_irqs[i] = *gpio_in[i]; >> + old_irqs[i]->handler = gpio_in[i]->handler; >> + old_irqs[i]->opaque = gpio_in[i]->opaque; >> + >> gpio_in[i]->handler = handler; >> gpio_in[i]->opaque = &old_irqs[i]; >> } > > This function is leaky by design, because it doesn't do anything > with the old_irqs array and there's no function for un-intercepting > the IRQs (which would need to free that memory). This is not ideal > but OK because it's only used in the test suite.
I think this could better be done without calling qemu_allocate_irqs(): Simply call qemu_allocate_irq() (without "s" at the end) within the for-loop for each irq instead. What do you think? Thomas