> > No one really cares if we leak memory while dying, but who knows...
> > freeing that buffer may let us go down more gracefully.
> >
> > FYI, the leak is triggered when virFileReadAll succeeds
> > (it allocates "buffer"), yet xmlNewDoc fails:
> >
> > if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
> > return FALSE;
> >
> > doc = xmlNewDoc(NULL);
> > if (doc == NULL)
> > goto no_memory;
>
> The above is correct, but there's another leak in the same function,
> so I've amended the patch to also free the "list" buffer.
> "list" is allocated in the for-loop.
> If on the 2nd or subsequent iteration of that loop we take
> the "goto no_memory", we'd leak that buffer.
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dd916f3..c8ae9f2 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7139,6 +7139,8 @@ cleanup:
> return ret;
>
> no_memory:
> + VIR_FREE(list);
> + VIR_FREE(buffer);
> vshError(ctl, "%s", _("Out of memory"));
> ret = FALSE;
> return ret;
Actually that's not enough and it also duplicates code as all the cleanup code
is already there:
cleanup:
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(doc);
VIR_FREE(result);
if ((list != NULL) && (count > 0)) {
for (i = 0;i < count;i++)
VIR_FREE(list[i]);
}
VIR_FREE(list);
VIR_FREE(buffer);
return ret;
The best solution is jumping to cleanup in OOM path. I should have spotted
missing goto cleanup at the end of no_memory when I was reviewing the code...
Patch attached...
Jirka
From fa16cdcbe2a9632edce3d9c227235d37518b7afa Mon Sep 17 00:00:00 2001
Message-Id:
<fa16cdcbe2a9632edce3d9c227235d37518b7afa.1266922930.git.jdene...@redhat.com>
From: Jiri Denemark <jdene...@redhat.com>
Date: Tue, 23 Feb 2010 12:01:20 +0100
Subject: [PATCH] virsh.c: avoid all leaks in OOM path in cmdCPUBaseline
Mail-Followup-To: libvir-list@redhat.com
Signed-off-by: Jiri Denemark <jdene...@redhat.com>
---
tools/virsh.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index dc9d44c..5fdbbe5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7140,11 +7140,9 @@ cleanup:
return ret;
no_memory:
- VIR_FREE(list);
- VIR_FREE(buffer);
vshError(ctl, "%s", _("Out of memory"));
ret = FALSE;
- return ret;
+ goto cleanup;
}
/* Common code for the edit / net-edit / pool-edit functions which follow. */
--
1.7.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
