[libvirt] [PATCH 0/3] Resolve Coverity warnings

Resolve the "lower hanging fruit" Coverity issues from recent commits. Still left outstanding is rework of virVBoxSnapshotConfAllChildren() code and callers. John Ferlan (3): vbox_temp: Resolve Coverity warnings vbox_snapshot_conf: Resolve Coverity warnings libxl: Resolve Coverity warnings src/libxl/libxl_migration.c | 6 ++---- src/vbox/vbox_snapshot_conf.c | 27 ++++++++++++++++++--------- src/vbox/vbox_tmpl.c | 10 +++++++++- 3 files changed, 29 insertions(+), 14 deletions(-) -- 1.9.3

Clean up code to resolve Coverity RESOURCE_LEAK's from commit id's '632b9600' and 'b739f807'. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/vbox/vbox_tmpl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index da9b2b7..1ed2729 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6267,6 +6267,11 @@ vboxSnapshotRedefine(virDomainPtr dom, */ parentUuid = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, realReadOnlyDisksPath[it]); + if (parentUuid == NULL) { + VIR_FREE(readWriteDisk); + goto cleanup; + } + if (virVBoxSnapshotConfAddHardDiskToMediaRegistry(readWriteDisk, snapshotMachineDesc->mediaRegistry, parentUuid) < 0) { @@ -8576,14 +8581,17 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get medium uuid, rc=%08x"), (unsigned)rc); + VIR_FREE(disk); goto cleanup; } VBOX_UTF16_TO_UTF8(uuidUtf16, &uuid); disk->uuid = uuid; VBOX_UTF16_FREE(uuidUtf16); - if (VIR_STRDUP(disk->location, newLocationUtf8) < 0) + if (VIR_STRDUP(disk->location, newLocationUtf8) < 0) { + VIR_FREE(disk); goto cleanup; + } rc = newMedium->vtbl->GetFormat(newMedium, &formatUtf16); VBOX_UTF16_TO_UTF8(formatUtf16, &format); -- 1.9.3

On 06/11/2014 03:38 PM, John Ferlan wrote:
Clean up code to resolve Coverity RESOURCE_LEAK's from commit id's '632b9600' and 'b739f807'.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/vbox/vbox_tmpl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
ACK Jan

Clean up some Coverity warnings from commit id '4dc5d8f1' Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/vbox/vbox_snapshot_conf.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 9c78410..676a0e1 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -1257,6 +1257,11 @@ virVBoxSnapshotConfIsCurrentSnapshot(virVBoxSnapshotConfMachinePtr machine, goto cleanup; } snapshot = virVBoxSnapshotConfSnapshotByName(machine->snapshot, snapshotName); + if (snapshot == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find the snapshot %s"), snapshotName); + goto cleanup; + } return STREQ(snapshot->uuid, machine->currentSnapshot); cleanup: @@ -1274,7 +1279,7 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, { int result = -1; size_t i = 0; - char **ret; + char **ret = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr xPathContext = NULL; xmlNodePtr *nodes = NULL; @@ -1296,7 +1301,9 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, goto cleanup; } xPathContext->node = xmlDocGetRootElement(xml); - nodeSize = virXPathNodeSet("/domainsnapshot/disks/disk", xPathContext, &nodes); + if ((nodeSize = virXPathNodeSet("/domainsnapshot/disks/disk", + xPathContext, &nodes)) < 0) + goto cleanup; if (VIR_ALLOC_N(ret, nodeSize) < 0) goto cleanup; @@ -1315,13 +1322,12 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, xmlFreeDoc(xml); xmlXPathFreeContext(xPathContext); if (result < 0) { - for (i = 0; i < nodeSize; i++) - VIR_FREE(ret[i]); - VIR_FREE(ret); + virStringFreeList(ret); nodeSize = -1; } else { *rwDisksPath = ret; } + VIR_FREE(nodes); return nodeSize; } @@ -1357,9 +1363,10 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, goto cleanup; } xPathContext->node = xmlDocGetRootElement(xml); - nodeSize = virXPathNodeSet("/domainsnapshot/domain/devices/disk", - xPathContext, - &nodes); + if ((nodeSize = virXPathNodeSet("/domainsnapshot/domain/devices/disk", + xPathContext, + &nodes)) < 0) + goto cleanup; if (VIR_ALLOC_N(ret, nodeSize) < 0) goto cleanup; @@ -1379,8 +1386,10 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, if (result < 0) { virStringFreeList(ret); nodeSize = -1; + } else { + *roDisksPath = ret; } - *roDisksPath = ret; + VIR_FREE(nodes); return nodeSize; } -- 1.9.3

On 06/11/2014 03:38 PM, John Ferlan wrote:
Clean up some Coverity warnings from commit id '4dc5d8f1'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/vbox/vbox_snapshot_conf.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 9c78410..676a0e1 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -1257,6 +1257,11 @@ virVBoxSnapshotConfIsCurrentSnapshot(virVBoxSnapshotConfMachinePtr machine, goto cleanup; } snapshot = virVBoxSnapshotConfSnapshotByName(machine->snapshot, snapshotName); + if (snapshot == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR,
You can use VIR_ERR_NO_DOMAIN_SNAPSHOT here
+ _("Unable to find the snapshot %s"), snapshotName); + goto cleanup; + } return STREQ(snapshot->uuid, machine->currentSnapshot);
ACK Jan

Resolve two Coverity issues introduced by commit id '9b8d6e1e' Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 9fe904e..a25edf0 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -98,8 +98,7 @@ libxlDoMigrateReceive(virNetSocketPtr sock, size_t i; int ret; - virNetSocketAccept(sock, &client_sock); - if (client_sock == NULL) { + if (virNetSocketAccept(sock, &client_sock) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Fail to accept migration connection")); goto cleanup; @@ -526,8 +525,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, cleanup: if (event) libxlDomainEventQueue(driver, event); - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); virObjectUnref(cfg); return dom; } -- 1.9.3

John Ferlan wrote:
Resolve two Coverity issues introduced by commit id '9b8d6e1e'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 9fe904e..a25edf0 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -98,8 +98,7 @@ libxlDoMigrateReceive(virNetSocketPtr sock, size_t i; int ret;
- virNetSocketAccept(sock, &client_sock); - if (client_sock == NULL) { + if (virNetSocketAccept(sock, &client_sock) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Fail to accept migration connection")); goto cleanup; @@ -526,8 +525,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, cleanup: if (event) libxlDomainEventQueue(driver, event); - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); virObjectUnref(cfg); return dom; }
ACK. Regards, Jim

On 06/11/2014 11:08 AM, Jim Fehlig wrote:
John Ferlan wrote:
Resolve two Coverity issues introduced by commit id '9b8d6e1e'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
ACK.
Regards, Jim
Tks, I have pushed 3/3 John

On 06/11/2014 09:38 AM, John Ferlan wrote:
Resolve the "lower hanging fruit" Coverity issues from recent commits. Still left outstanding is rework of virVBoxSnapshotConfAllChildren() code and callers.
John Ferlan (3): vbox_temp: Resolve Coverity warnings vbox_snapshot_conf: Resolve Coverity warnings libxl: Resolve Coverity warnings
src/libxl/libxl_migration.c | 6 ++---- src/vbox/vbox_snapshot_conf.c | 27 ++++++++++++++++++--------- src/vbox/vbox_tmpl.c | 10 +++++++++- 3 files changed, 29 insertions(+), 14 deletions(-)
The vbox changes were pushed with the suggested mod by Jan. Tks, John
participants (3)
-
Jim Fehlig
-
John Ferlan
-
Ján Tomko