Re: [libvirt] [PATCH] qemu: avoid memory leaks

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; } }