[libvirt] [PATCH] Fix a memory leak in cmdSchedInfoUpdateOne
From: hwbi <hwbi2008@gmail.com> The param needs to be virTypedParamsFree()'d in cmdSchedInfoUpdateOne(). --- tools/virsh-domain.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..d704053 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4085,7 +4085,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl, int *nparams, int *maxparams, const char *field, const char *value) { - virTypedParameterPtr param; + virTypedParameterPtr param = NULL; int ret = -1; size_t i; @@ -4109,6 +4109,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl, vshError(ctl, _("invalid scheduler option: %s"), field); cleanup: + virTypedParamsFree(param, *nparams); return ret; } -- 1.7.1
On 08/22/2013 09:40 PM, hwbi2008@gmail.com wrote:
From: hwbi <hwbi2008@gmail.com>
The param needs to be virTypedParamsFree()'d in cmdSchedInfoUpdateOne(). --- tools/virsh-domain.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
[You used git send-email incorrectly - when it asks what to use as in-reply-to, you should hit 'enter' with blank text to create a new thread, rather than attempting to answer 'y' which threads it to any other thread that also used the incorrect 'y' as a message id. Newer git prevents the use of invalid message ids - so another solution is to upgrade your git.]
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..d704053 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4085,7 +4085,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl, int *nparams, int *maxparams, const char *field, const char *value) { - virTypedParameterPtr param; + virTypedParameterPtr param = NULL; int ret = -1; size_t i;
@@ -4109,6 +4109,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl, vshError(ctl, _("invalid scheduler option: %s"), field);
cleanup: + virTypedParamsFree(param, *nparams); return ret; }
Unfortunately, your patch is incorrect. When applying it, and re-running 'make check', I get a failure: FAIL: virsh-schedinfo ===================== ./virsh-schedinfo: line 46: 29400 Aborted (core dumped) virsh -c $test_url schedinfo 1 --set j=k > out 2> err --- exp-out 2013-08-26 09:52:15.318517471 -0600 +++ out 2013-08-26 09:52:15.341517394 -0600 @@ -1,2 +1 @@ Scheduler : fair - Please fix and resubmit. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/26/2013 09:55 AM, Eric Blake wrote:
+++ b/tools/virsh-domain.c @@ -4085,7 +4085,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl, int *nparams, int *maxparams, const char *field, const char *value) { - virTypedParameterPtr param; + virTypedParameterPtr param = NULL; int ret = -1; size_t i;
Looking more at the code, param is only ever set to &(src_params[i]), which means that it is NOT allocated locally, and therefore must NOT be freed locally.
Unfortunately, your patch is incorrect. When applying it, and re-running 'make check', I get a failure:
FAIL: virsh-schedinfo =====================
./virsh-schedinfo: line 46: 29400 Aborted (core dumped)
The test failure is because you are freeing an incorrect pointer.
Please fix and resubmit.
Even better, first prove that there is a memory leak (do you have a valgrind trace to back up your claim? What command line can be used to demonstrate the leak?). I didn't audit whether there is a leak somewhere else, but just on inspection, if there IS a leak, it is NOT in cmdSchedInfoUpdateOne(), but elsewhere. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
hwbi2008＠gmail.com