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;
}
}
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org