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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list