[libvirt] [PATCH] cgroup.c: avoid unconditional leaks

Here's output from "git format-patch -U19 --stdout -1", so you see the context:
From 47ea9beecf93a796e606b4d906424c05e7f6c9bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 4 Feb 2010 11:14:03 +0100 Subject: [PATCH] cgroup.c: avoid unconditional leaks
* src/util/cgroup.c (virCgroupCpuSetInherit) [HAVE_MNTENT_H]: Don't leak CPU-set inheritance value strings. --- src/util/cgroup.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c80cf50..e6f0270 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -416,38 +416,39 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path); for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { char *value; rc = virCgroupGetValueStr(parent, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], &value); if (rc != 0) { VIR_ERROR("Failed to get %s %d", inherit_values[i], rc); break; } VIR_DEBUG("Inherit %s = %s", inherit_values[i], value); rc = virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], value); + VIR_FREE(value); if (rc != 0) { VIR_ERROR("Failed to set %s %d", inherit_values[i], rc); break; } } return rc; } static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int create) { int i; int rc = 0; VIR_DEBUG("Make group %s", group->path); for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { char *path = NULL; -- 1.7.0.rc1.193.ge8618

According to Jim Meyering on 2/4/2010 3:16 AM:
Here's output from "git format-patch -U19 --stdout -1", so you see the context: for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { char *value;
rc = virCgroupGetValueStr(parent, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], &value); if (rc != 0) { VIR_ERROR("Failed to get %s %d", inherit_values[i], rc); break; }
Is value allocated prior to that break?
VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);
rc = virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], value); + VIR_FREE(value);
Or is it only allocated by virCgroupSetValueStr. -- Don't work too hard, make some time for fun as well! Eric Blake ebb9@byu.net

On Thu, Feb 04, 2010 at 06:14:44AM -0700, Eric Blake wrote:
According to Jim Meyering on 2/4/2010 3:16 AM:
Here's output from "git format-patch -U19 --stdout -1", so you see the context: for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { char *value;
rc = virCgroupGetValueStr(parent, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], &value); if (rc != 0) { VIR_ERROR("Failed to get %s %d", inherit_values[i], rc); break; }
Is value allocated prior to that break?
VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);
rc = virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], value); + VIR_FREE(value);
Or is it only allocated by virCgroupSetValueStr.
The virCgroupGetValueStr() should be allocating it upon successful return only, and in the error return case it is guarenteed NULL. The virCgroupSetValueStr() method is declared 'const char *' IIRC and merely writes the passed value out to a file Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Feb 04, 2010 at 11:16:53AM +0100, Jim Meyering wrote:
Here's output from "git format-patch -U19 --stdout -1", so you see the context:
From 47ea9beecf93a796e606b4d906424c05e7f6c9bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 4 Feb 2010 11:14:03 +0100 Subject: [PATCH] cgroup.c: avoid unconditional leaks
* src/util/cgroup.c (virCgroupCpuSetInherit) [HAVE_MNTENT_H]: Don't leak CPU-set inheritance value strings. --- src/util/cgroup.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c80cf50..e6f0270 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -416,38 +416,39 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path); for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { char *value;
rc = virCgroupGetValueStr(parent, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], &value); if (rc != 0) { VIR_ERROR("Failed to get %s %d", inherit_values[i], rc); break; }
VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);
rc = virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], value); + VIR_FREE(value);
if (rc != 0) { VIR_ERROR("Failed to set %s %d", inherit_values[i], rc); break; } }
return rc; }
static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int create) { int i; int rc = 0;
VIR_DEBUG("Make group %s", group->path); for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { char *path = NULL;
--
ACK, that's an unusually large context that diff generated ! Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel P. Berrange wrote:
On Thu, Feb 04, 2010 at 11:16:53AM +0100, Jim Meyering wrote:
Here's output from "git format-patch -U19 --stdout -1", so you see the context: ...
ACK, that's an unusually large context that diff generated !
The idea is to make review a little easier when the change is local and self-contained.

On Thu, Feb 04, 2010 at 02:40:29PM +0100, Jim Meyering wrote:
Daniel P. Berrange wrote:
On Thu, Feb 04, 2010 at 11:16:53AM +0100, Jim Meyering wrote:
Here's output from "git format-patch -U19 --stdout -1", so you see the context: ...
ACK, that's an unusually large context that diff generated !
The idea is to make review a little easier when the change is local and self-contained.
yup, an useful trick, thanks for the tip :-) 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/
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake
-
Jim Meyering