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.
* src/qemu/qemu_command.c (qemuParseCommandLine): Plug leaks.
---
src/qemu/qemu_command.c | 23 ++++++++++++++++-------
1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6a2e2ae..c638420 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5938,6 +5938,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
int video = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
int nvirtiodisk = 0;
qemuDomainCmdlineDefPtr cmd = NULL;
+ virDomainDiskDefPtr disk = NULL;
if (pidfile)
*pidfile = NULL;
@@ -6124,7 +6125,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
STRPREFIX(arg, "-fd") ||
STREQ(arg, "-cdrom")) {
WANT_VALUE();
- virDomainDiskDefPtr disk;
if (VIR_ALLOC(disk)< 0)
goto no_memory;
@@ -6239,6 +6239,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
}
def->disks[def->ndisks++] = disk;
+ disk = NULL;
} else if (STREQ(arg, "-no-acpi")) {
def->features&= ~(1<< VIR_DOMAIN_FEATURE_ACPI);
} else if (STREQ(arg, "-no-reboot")) {
@@ -6307,8 +6308,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
if (!(chr = virDomainChrDefNew()))
goto error;
- if (qemuParseCommandLineChr(&chr->source, val)< 0)
+ if (qemuParseCommandLineChr(&chr->source, val)< 0) {
+ virDomainChrDefFree(chr);
goto error;
+ }
if (VIR_REALLOC_N(def->serials, def->nserials+1)< 0) {
virDomainChrDefFree(chr);
goto no_memory;
@@ -6325,8 +6328,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
if (!(chr = virDomainChrDefNew()))
goto error;
- if (qemuParseCommandLineChr(&chr->source, val)< 0)
+ if (qemuParseCommandLineChr(&chr->source, val)< 0) {
+ virDomainChrDefFree(chr);
goto error;
+ }
if (VIR_REALLOC_N(def->parallels, def->nparallels+1)< 0) {
virDomainChrDefFree(chr);
goto no_memory;
@@ -6353,7 +6358,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
}
def->inputs[def->ninputs++] = input;
} else if (STRPREFIX(val, "disk:")) {
- virDomainDiskDefPtr disk;
if (VIR_ALLOC(disk)< 0)
goto no_memory;
disk->src = strdup(val + strlen("disk:"));
@@ -6376,6 +6380,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
}
def->disks[def->ndisks++] = disk;
+ disk = NULL;
} else {
virDomainHostdevDefPtr hostdev;
if (!(hostdev = qemuParseCommandLineUSB(val)))
@@ -6399,7 +6404,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
def->nets[def->nnets++] = net;
}
} else if (STREQ(arg, "-drive")) {
- virDomainDiskDefPtr disk;
WANT_VALUE();
if (!(disk = qemuParseCommandLineDisk(caps, val, nvirtiodisk)))
goto error;
@@ -6408,6 +6412,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
}
def->disks[def->ndisks++] = disk;
+ disk = NULL;
if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
nvirtiodisk++;
@@ -6516,8 +6521,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
if (VIR_ALLOC(chr)< 0)
goto no_memory;
- if (qemuParseCommandLineChr(chr, val)< 0)
+ if (qemuParseCommandLineChr(chr, val)< 0) {
+ VIR_FREE(chr);
Shouldn't this be virDomainChrDefFree(chr)?
goto error;
+ }
*monConfig = chr;
}
@@ -6545,10 +6552,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
char *hosts, *port, *saveptr = NULL, *token;
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).
}
@@ -6676,6 +6684,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
no_memory:
virReportOOMError();
error:
+ VIR_FREE(disk);
VIR_FREE(cmd);
virDomainDefFree(def);
VIR_FREE(nics);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
