2010/2/15 Jim Meyering <jim@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@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