Re: [libvirt] [PATCH] Fix a memory leak in cmdSchedInfoUpdateOne
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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)