Re: [libvirt] [PATCH] vbox_tmpl.c: avoid leak on OOM error path
2010/2/15 Jim Meyering <jim(a)meyering.net>:
Matthias Bolte wrote:
...
> If you look closely at what the function does with the
> virNetworkDefPtr def, you see that it fills it with information and
> calls virNetworkDefFormat in the end. So def is never returned from
> the function and leaks in the successful-return path. I suggest this
> patch instead:
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 68dffd2..d1a701e 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -6183,6 +6183,7 @@ static char *vboxNetworkDumpXML(virNetworkPtr
> network, int flags ATTRIBUTE_UNUSE
> ret = virNetworkDefFormat(def);
>
> cleanup:
> + virNetworkDefFree(def);
> VIR_FREE(networkNameUtf8);
> return ret;
> }
Good point. Definite improvement.
Here you go:
From 1dc101a80c3af855c72e8dbc64fbeb91614c49b4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 15 Feb 2010 17:54:15 +0100
Subject: [PATCH] vbox_tmpl.c: avoid an unconditional leak
* src/vbox/vbox_tmpl.c (vboxDomainDumpXML): Free def.
Improved by Matthias Bolte.
---
src/vbox/vbox_tmpl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 68dffd2..751fe52 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6183,6 +6183,7 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
ret = virNetworkDefFormat(def);
cleanup:
+ VIR_FREE(def);
No, a simple VIR_FREE isn't enough here. At this point def can
contains allocated parts. It's necessary to call
virNetworkDefFree(def) here. virNetworkDefFree takes care of freeing
all parts of def and def itself.
VIR_FREE(networkNameUtf8);
return ret;
}
--
1.7.0.181.g41533
Matthias