On Mon, Jan 18, 2010 at 10:40:26AM +0100, Jim Meyering wrote: > When virStorageBackendProbeTarget fails, it returns -1 or -2. > The two uses below obviously intended to handle those cases > differently, but due to a wrong comparison, they always treated a > "real" (e.g., open) failure (-1) just like an ignorable "wrong file type" > failure (-2). > > FYI, coverity reported that the "goto cleanup" statement was > unreachable, which was true in each case, but hardly the real problem. > @@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, > &backingStore, > &vol->allocation, > &vol->capacity, > - &vol->target.encryption) < 0)) { > + &vol->target.encryption) != 0)) { > if (ret == -1) > goto cleanup; > else { > @@ -603,7 +603,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, > if ((ret = virStorageBackendProbeTarget(conn, > &vol->backingStore, > NULL, NULL, NULL, > - NULL)) < 0) { > + NULL)) != 0) { > if (ret == -1) > goto cleanup; > else { I don't understand how this is making any difference. The method virStorageBackendProbeTarget can return 0, -1 or -2, so virStorageBackendProbeTarget < 0 catches the -1 and -2 cases virStorageBackendProbeTarget != 0 catches the -1 and -2 cases I only see this change making a difference if one of the error return values is a positive integer greater than 0, but we don't have that here. What am I missing ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list