[libvirt] [PATCH] Memory Leak Fix: Check def->info->alias before assigning

Osier Yang jyang at redhat.com
Wed Nov 27 06:19:58 UTC 2013


On 27/11/13 06:31, Nehal J Wani wrote:
> On running the command make -C tests valgrind, there used to be a bunch of
> memory leaks shown by valgrind. Specifically, one can check it by running:
> libtool --mode=execute valgrind --quiet --leak-check=full --suppressions=./.valgrind.supp qemuhotplugtest
> The issue was that def->info->alias was already malloc'ed by xmlStrndup in
> virDomainDeviceInfoParseXML (domain_conf.c:3439). The new alias was being
> assigned again without freeing the old one in qemuAssignDeviceAliases().
> This patch checks if the entity exists, and frees accordingly, hence making
> valgrind cry lesser.
>
> ---
>   src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 763417f..bbec1d4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -979,6 +979,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
>       size_t i;
>   
>       for (i = 0; i < def->ndisks; i++) {
> +        if (def->disks[i]->info.alias)
> +            VIR_FREE(def->disks[i]->info.alias);

Instead of free'ing it, it should be used to avoid calculate/strdup the
alias again.  Since the "alias" from virDomainDeviceInfoParseXML only
works for active domain's XML:

<snip>
             if (alias == NULL &&
                 !(flags & VIR_DOMAIN_XML_INACTIVE) &&
                 xmlStrEqual(cur->name, BAD_CAST "alias")) {
                 alias = cur;
</snip>

And I don't think using the "alias" existing in the XML could cause any
conflicts for an active domain.

Regards,
Osier




More information about the libvir-list mailing list