diff mbox

[29/29] hw/s390x/sclpcpu.c: Fix memory leak spotted by valgrind

Message ID 1432814932-12608-30-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao May 28, 2015, 12:08 p.m. UTC 
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(-)

Comments

Peter Maydell May 28, 2015, 1:21 p.m. UTC | #1 
On 28 May 2015 at 13:08, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> @@ -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);
>      return 0;

Huh? Surely we can't validly use the irq once you've
called qemu_free_irq() on it?

-- PMM
Shannon Zhao May 30, 2015, 11:07 a.m. UTC | #2 
On 2015/5/30 18:34, Paolo Bonzini wrote:
>
> On 28/05/2015 14:08, Shannon Zhao wrote:
>> >-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);
>> >      return 0;
> This is wrong, you cannot free the IRQ after you have stored it in
> irq_cpu_hotplug.

Yeah, sorry for that. But I don't find who calls 
raise_irq_cpu_hotplug(). I'm not very familiar with these codes.
diff mbox

Patch

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);
     return 0;
 }