[libvirt] [PATCH 0/2] Couple of almost trivial memleak fixes

*** NOT PUSHED. PLEASE REVIEW *** Michal Prívozník (2): virnetdevip: Free data.devices in virNetDevIPCheckIPv6Forwarding() too networkStartNetworkVirtual: Don't leak macmap object src/network/bridge_driver.c | 2 ++ src/util/virnetdevip.c | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) -- 2.16.4

We are freeing the individual strings (which were filled by virNetDevIPCheckIPv6ForwardingCallback()) but not the array itself. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevip.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index c6d6175627..866ddcc213 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -651,8 +651,7 @@ virNetDevIPCheckIPv6Forwarding(void) cleanup: nlmsg_free(nlmsg); - for (i = 0; i < data.ndevices; i++) - VIR_FREE(data.devices[i]); + virStringListFreeCount(data.devices, data.ndevices); return valid; } -- 2.16.4

On Mon, Aug 13, 2018 at 11:21:44AM +0200, Michal Privoznik wrote:
We are freeing the individual strings (which were filled by virNetDevIPCheckIPv6ForwardingCallback()) but not the array itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevip.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index c6d6175627..866ddcc213 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -651,8 +651,7 @@ virNetDevIPCheckIPv6Forwarding(void)
cleanup: nlmsg_free(nlmsg); - for (i = 0; i < data.ndevices; i++) - VIR_FREE(data.devices[i]); + virStringListFreeCount(data.devices, data.ndevices); return valid;
Reviewed-by: Erik Skultety <eskultet@redhat.com>

When starting network a macmap object is created (which stores MAC -> domain name mappings). However, if something goes wrong (e.g. virNetDevIPCheckIPv6Forwarding() fails) then the object is leaked. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/bridge_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f92cc61e47..588b0d147d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2453,6 +2453,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, goto err1; virNetworkObjSetMacMap(obj, macmap); + macmap = NULL; /* Set bridge options */ @@ -2590,6 +2591,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, ignore_value(virNetDevTapDelete(macTapIfName, NULL)); VIR_FREE(macTapIfName); } + virNetworkObjUnrefMacMap(obj); VIR_FREE(macMapFile); err0: -- 2.16.4

On Mon, Aug 13, 2018 at 11:21:45AM +0200, Michal Privoznik wrote:
When starting network a macmap object is created (which stores MAC -> domain name mappings). However, if something goes wrong (e.g. virNetDevIPCheckIPv6Forwarding() fails) then the object is leaked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/bridge_driver.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f92cc61e47..588b0d147d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2453,6 +2453,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, goto err1;
virNetworkObjSetMacMap(obj, macmap); + macmap = NULL;
/* Set bridge options */
@@ -2590,6 +2591,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, ignore_value(virNetDevTapDelete(macTapIfName, NULL)); VIR_FREE(macTapIfName); } + virNetworkObjUnrefMacMap(obj);
Hopefully there's no occurrence of plain virObjectUnref for obj->macmap. Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
Michal Privoznik