[libvirt] [PATCH] lxc_container.c: avoid a leak on error paths

This started with a dead-store report: File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc' But there was a leak, too, upon any but the first failure. This fixes both.
From 141ba8bab2de00c911017bff6fca240c72a80633 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Fri, 4 Sep 2009 16:12:35 +0200 Subject: [PATCH] lxc_container.c: avoid a leak on error paths
* src/lxc_container.c (lxcContainerMountBasicFS): Don't leak upon failure. Add "cleanup:" label and change each post-allocation failure to use "goto cleanup" rather than returning immediately. --- src/lxc_container.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 2073864..f4381e7 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -1,6 +1,6 @@ /* * Copyright IBM Corp. 2008 - * Copyright Red Hat 2008 + * Copyright Red Hat 2008-2009 * * lxc_container.c: file description * @@ -384,12 +384,12 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) { "none", "/selinux", "selinuxfs" }, #endif }; - int i, rc; + int i, rc = -1; char *devpts; if (virAsprintf(&devpts, "/.oldroot%s/dev/pts", root->src) < 0) { virReportOOMError(NULL); - return -1; + return rc; } for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { @@ -397,31 +397,35 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) virReportSystemError(NULL, errno, _("failed to mkdir %s"), mnts[i].src); - return -1; + goto cleanup; } if (mount(mnts[i].src, mnts[i].dst, mnts[i].type, 0, NULL) < 0) { virReportSystemError(NULL, errno, _("failed to mount %s on %s"), mnts[i].type, mnts[i].type); - return -1; + goto cleanup; } } if ((rc = virFileMakePath("/dev/pts") < 0)) { virReportSystemError(NULL, rc, "%s", _("cannot create /dev/pts")); - return -1; + goto cleanup; } VIR_DEBUG("Trying to move %s to %s", devpts, "/dev/pts"); if ((rc = mount(devpts, "/dev/pts", NULL, MS_MOVE, NULL)) < 0) { virReportSystemError(NULL, errno, "%s", _("failed to mount /dev/pts in container")); - return -1; + goto cleanup; } + + rc = 0; + + cleanup: VIR_FREE(devpts); - return 0; + return rc; } static int lxcContainerPopulateDevices(void) -- 1.6.4.2.409.g85dc3

On Fri, Sep 04, 2009 at 04:15:20PM +0200, Jim Meyering wrote:
This started with a dead-store report:
File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc'
But there was a leak, too, upon any but the first failure. This fixes both.
Ah, right ! Good catch :-) ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Daniel Veillard wrote:
On Fri, Sep 04, 2009 at 04:15:20PM +0200, Jim Meyering wrote:
This started with a dead-store report:
File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc'
But there was a leak, too, upon any but the first failure. This fixes both.
Ah, right ! Good catch :-)
ACK !
Thanks. I've just pushed everything up to that point.
participants (2)
-
Daniel Veillard
-
Jim Meyering