Re: [PATCH] bugfix: irq: Avoid covering object refcount of qemu_irq

[zhukeqian]

Hi Thomas,

On 2020/7/28 16:48, Thomas Huth wrote:
> On 27/07/2020 16.41, Peter Maydell wrote:
>> On Mon, 27 Jul 2020 at 14:03, Keqian Zhu <zhukeqian1@huawei.com> wrote:
>>>
>>> Avoid covering object refcount of qemu_irq, otherwise it may causes
>>> memory leak.
>>>
>>> Signed-off-by: Keqian Zhu <zhukeqian1@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?
Yeah, this can save some memory. But I think it does not solve the refcount 
covering
problem.
> 
Thanks
Keqian
>  Thomas
> 
> 
>