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

[Cornelia Huck]

On Thu, 28 May 2015 20:08:52 +0800
Shannon Zhao <address@hidden> wrote:

> From: Shannon Zhao <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Shannon Zhao <address@hidden>
> ---
>  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.

>      return 0;
>  }
>