[libvirt] [PATCH] qemu: avoid memory leaks

[Laine Stump]

On 08/02/2011 05:46 PM, Eric Blake wrote:
> On 08/02/2011 02:48 PM, Laine Stump wrote:
>> On 08/02/2011 04:10 PM, Eric Blake wrote:
>>> Quite a few leaks detected by coverity. For chr, the leaks were
>>> close enough to the allocations to plug in place; for disk, the
>>> leaks were separated from the allocation by enough other lines with
>>> intermediate failure cases that I refactored the cleanup instead.
>>>
>>>
>>> - if (qemuParseCommandLineChr(chr, val)< 0)
>>> + if (qemuParseCommandLineChr(chr, val)< 0) {
>>> + VIR_FREE(chr);
>>
>>
>> Shouldn't this be virDomainChrDefFree(chr)?
>>
>>
>
>>> virDomainDiskDefPtr first_rbd_disk = NULL;
>>> for (i = 0 ; i< def->ndisks ; i++) {
>>> - virDomainDiskDefPtr disk = def->disks[i];
>>> + disk = def->disks[i];
>>> if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK&&
>>> disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
>>> first_rbd_disk = disk;
>>> + disk = NULL;
>>> break;
>>> }
>>
>>
>> If you never hit the if condition, disk will be left pointing to one of
>> the disks in def, and will be freed during cleanup. I think you want to
>> set disk = NULL after this look is finished as well, don't you? (Either
>> that, or just use a different variable).
>
> Both good points.  I'm squashing this in to create v2.
>
> diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
> index c638420..194f3ff 100644
> --- i/src/qemu/qemu_command.c
> +++ w/src/qemu/qemu_command.c
> @@ -6522,7 +6522,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr 
> caps,
>                      goto no_memory;
>
>                  if (qemuParseCommandLineChr(chr, val) < 0) {
> -                    VIR_FREE(chr);
> +                    virDomainChrSourceDefFree(chr);
>                      goto error;
>                  }
>
> @@ -6552,11 +6552,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr 
> caps,
>              char *hosts, *port, *saveptr = NULL, *token;
>              virDomainDiskDefPtr first_rbd_disk = NULL;
>              for (i = 0 ; i < def->ndisks ; i++) {
> -                disk = def->disks[i];
> -                if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
> -                    disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
> -                    first_rbd_disk = disk;
> -                    disk = NULL;
> +                if (def->disks[i]->type == 
> VIR_DOMAIN_DISK_TYPE_NETWORK &&
> +                    def->disks[i]->protocol == 
> VIR_DOMAIN_DISK_PROTOCOL_RBD) {
> +                    first_rbd_disk = def->disks[i];
>                      break;
>                  }
>              }
>
>

ACK with those changes.