Re: [Qemu-trivial] [Qemu-devel] [PATCH 29/29] hw/s390x/sclpcpu.c: Fix memory leak spotted by valgrind

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/5/28 21:11, Cornelia Huck wrote:
> On Thu, 28 May 2015 20:08:52 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > valgrind complains about:
>> > ==1413== 188 (8 direct, 180 indirect) bytes in 1 blocks are definitely lost in loss record 951 of 1,199
>> > ==1413==    at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> > ==1413==    by 0x262D8B: malloc_and_trace (vl.c:2556)
>> > ==1413==    by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3)
>> > ==1413==    by 0x2C7ACF: qemu_extend_irqs (irq.c:55)
>> > ==1413==    by 0x2C7B5B: qemu_allocate_irqs (irq.c:64)
>> > ==1413==    by 0x2168F4: irq_cpu_hotplug_init (sclpcpu.c:84)
>> > ==1413==    by 0x21623F: event_realize (event-facility.c:385)
>> > ==1413==    by 0x2C4610: device_set_realized (qdev.c:1058)
>> > ==1413==    by 0x317DDA: property_set_bool (object.c:1514)
>> > ==1413==    by 0x3166D4: object_property_set (object.c:837)
>> > ==1413==    by 0x3189F6: object_property_set_qobject (qom-qobject.c:24)
>> > ==1413==    by 0x316943: object_property_set_bool (object.c:905)
>> > 
>> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> >  hw/s390x/sclpcpu.c | 9 ++++++---
>> >  1 file changed, 6 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c
>> > index 2fe8b5a..1090e2f 100644
>> > --- a/hw/s390x/sclpcpu.c
>> > +++ b/hw/s390x/sclpcpu.c
>> > @@ -25,13 +25,13 @@ typedef struct ConfigMgtData {
>> >      uint8_t event_qualifier;
>> >  } QEMU_PACKED ConfigMgtData;
>> > 
>> > -static qemu_irq *irq_cpu_hotplug; /* Only used in this file */
>> > +static qemu_irq irq_cpu_hotplug; /* Only used in this file */
>> > 
>> >  #define EVENT_QUAL_CPU_CHANGE  1
>> > 
>> >  void raise_irq_cpu_hotplug(void)
>> >  {
>> > -    qemu_irq_raise(*irq_cpu_hotplug);
>> > +    qemu_irq_raise(irq_cpu_hotplug);
>> >  }
>> > 
>> >  static unsigned int send_mask(void)
>> > @@ -81,7 +81,10 @@ static void trigger_signal(void *opaque, int n, int level)
>> > 
>> >  static int irq_cpu_hotplug_init(SCLPEvent *event)
>> >  {
>> > -    irq_cpu_hotplug = qemu_allocate_irqs(trigger_signal, event, 1);
>> > +    qemu_irq irq = qemu_allocate_irq(trigger_signal, event, 0);
>> > +
>> > +    irq_cpu_hotplug = irq;
>> > +    qemu_free_irq(irq);
> This looks... odd. But then, the whole code structure here with its
> global variable and its roundabout way of triggering the notification
> looks weird. We'd probably be better off looking up the sclp event for
> cpu hotplug and triggering the interrupt directly.
> 

Yeah, it's odd and　I don't find who calls raise_irq_cpu_hotplug().

-- 
Shannon