qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_


From: Nikita Belov
Subject: Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_init()
Date: Fri, 31 Oct 2014 13:42:05 +0300
User-agent: Roundcube Webmail/0.8.5

On 2014-10-29 19:03, Peter Maydell wrote:
On 29 October 2014 14:03, Nikita Belov <address@hidden> wrote:
Variable 'ram_lo' is allocated unconditionally, but used only in some cases. When it is unused pointer will be lost at function exit, resulting in a
memory leak. Free memory in this case.

Valgrind output:
==16879== 240 bytes in 1 blocks are definitely lost in loss record 6,033 of 7,018 ==16879== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16879==    by 0x33D2CE: malloc_and_trace (vl.c:2804)
==16879== by 0x509E610: g_malloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==16879==    by 0x288836: realview_init (realview.c:55)
==16879==    by 0x28988C: realview_pb_a8_init (realview.c:375)
==16879==    by 0x341426: main (vl.c:4413)

Signed-off-by: Nikita Belov <address@hidden>
---
 hw/arm/realview.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index af65aa4..673a540 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -141,6 +141,8 @@ static void realview_init(MachineState *machine,
                                &error_abort);
         vmstate_register_ram_global(ram_lo);
         memory_region_add_subregion(sysmem, 0x20000000, ram_lo);
+    } else {
+        g_free(ram_lo);
     }

memory_region_init_ram(ram_hi, NULL, "realview.highmem", ram_size,

We leak all of the MemoryRegions we allocate here, because we
don't have a persistent state struct to keep them in. This
doesn't really matter much because they're generally needed
for the lifetime of the QEMU process anyway, and we only call
board init functions once. So why worry about ram_lo in
particular (and why this board in particular)?

thanks
-- PMM

Indeed, generally we need memory regions for the lifetime of QEMU, but 'mem_lo' is different. It may not be used at all. We use 'ram_lo' only when a condition is true, in other case we will lose this pointer. Because of that if the condition is
false we have memory leak immediately (not when QEMU exits).

Semantically the touched code looks like this:
  if (/* condition */) {
      /* ... */
memory_region_init_ram(ram_lo, NULL, "realview.lowmem", low_ram_size);
      vmstate_register_ram_global(ram_lo);
      memory_region_add_subregion(sysmem, 0x20000000, ram_lo);
+ } else {
+     g_free(ram_lo);
  }
  /* ram_lo is not used any more */

and why this board in particular?
I just memory leak where I found it.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]