On 10/03/2013 03:38 PM, Cole Robinson wrote:
Spotted by coverity as John mentioned here:
http://www.redhat.com/archives/libvir-list/2013-October/msg00078.html
---
src/test/test_driver.c | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 255cc2b..a09facb 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6547,16 +6547,12 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
{
virDomainObjPtr vm = NULL;
int ret = -1;
- virDomainSnapshotObjPtr snap = NULL;
virCheckFlags(0, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
goto cleanup;
- if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
- goto cleanup;
ACK to this hunk.
@@ -6568,27 +6564,11 @@ cleanup:
static int
-testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
+testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot ATTRIBUTE_UNUSED,
unsigned int flags)
{
- virDomainObjPtr vm = NULL;
- int ret = -1;
- virDomainSnapshotObjPtr snap = NULL;
-
virCheckFlags(0, -1);
-
- if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
-
- if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
- goto cleanup;
But this hunk means that you are now blindly returning '1', even if
snapshot doesn't exist, which isn't quite accurate. Better would be
removing the snap variable, but still keeping the check for existence:
if (!testSnapObjFromSnapshot(vm, snapshot))
goto cleanup;
ret = 1;
cleanup:
...
so that the function can still fail on invalid input.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org