[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] storage_backend_fs.c: do not ignore probe failure



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]