From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44969) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjVjW-00069b-BA for qemu-devel@nongnu.org; Wed, 29 Oct 2014 12:04:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjVjK-0005Qq-UN for qemu-devel@nongnu.org; Wed, 29 Oct 2014 12:04:25 -0400 Received: from mail-lb0-f175.google.com ([209.85.217.175]:52166) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjVjK-0005Qd-Nv for qemu-devel@nongnu.org; Wed, 29 Oct 2014 12:04:14 -0400 Received: by mail-lb0-f175.google.com with SMTP id b6so2800047lbj.34 for ; Wed, 29 Oct 2014 09:04:13 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1414591438-14640-1-git-send-email-zodiac@ispras.ru> References: <1414591438-14640-1-git-send-email-zodiac@ispras.ru> From: Peter Maydell Date: Wed, 29 Oct 2014 16:03:53 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Belov Cc: Vasily Efimov , QEMU Developers , Kirill Batuzov On 29 October 2014 14:03, Nikita Belov 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 > --- > 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