[libvirt] [PATCH v2] bye to close(), welcome to VIR_(FORCE_)CLOSE()

[Daniel P. Berrange]

On Mon, Oct 25, 2010 at 08:13:19AM -0400, Stefan Berger wrote:
> Index: libvirt-acl/src/libvirt.c
> @@ -1544,14 +1544,11 @@ cleanup:
>              vethDelete(veths[i]);
>          VIR_FREE(veths[i]);
>      }
> -    if (rc != 0 && priv->monitor != -1) {
> -        close(priv->monitor);
> -        priv->monitor = -1;
> -    }
> -    if (parentTty != -1)
> -        close(parentTty);
> -    if (logfd != -1)
> -        close(logfd);
> +    if (rc != 0)
> +        VIR_FORCE_CLOSE(priv->monitor);
> +    VIR_FORCE_CLOSE(parentTty);
> +    if (VIR_CLOSE(logfd) < 0)
> +        virReportSystemError(errno, "%s", _("could not close logfile"));

This is reporting an error without returning an error code, so the
caller will still see success. 

> @@ -2011,8 +2008,7 @@ lxcReconnectVM(void *payload, const char
>  
> @@ -457,11 +458,15 @@ phypUUIDTable_WriteFile(virConnectPtr co
>          }
>      }
>  
> -    close(fd);
> +    if (VIR_CLOSE(fd) < 0)
> +        virReportSystemError(errno, _("Could not close %s\n"),
> +                             local_file);
>      return 0;

Again, reporting an error while returning success.

>  
>    err:
> -    close(fd);
> +    if (VIR_CLOSE(fd) < 0)
> +        virReportSystemError(errno, _("Could not close %s\n"),
> +                             local_file);
>      return -1;
>  }

This is likely blowing away a previously reported error.

> @@ -764,7 +769,9 @@ phypUUIDTable_Pull(virConnectPtr conn)
>          }
>          break;
>      }
> -    close(fd);
> +    if (VIR_CLOSE(fd) < 0)
> +        virReportSystemError(errno, _("Could not close %s\n"),
> +                             local_file);
>      goto exit;

Reporting error while returning success

> @@ -6542,8 +6536,10 @@ static int qemudDomainSaveImageClose(int
>  {
>      int ret = 0;
>  
> -    if (fd != -1)
> -        close(fd);
> +    if (VIR_CLOSE(fd) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("cannot close file"));
> +    }

Reporting error while returning success


> Index: libvirt-acl/src/storage/storage_backend_fs.c

> @@ -94,7 +95,9 @@ static int nlOpen(void)
>  
>  static void nlClose(int fd)
>  {
> -    close(fd);
> +    if (VIR_CLOSE(fd) < 0)
> +        virReportSystemError(errno,
> +                             "%s",_("cannot close netlink socket"));
>  }

No return status at all - this function likely shouldn't even
exist. Should be replaced with direct calls to VIR_FORCE_CLOSE
and VIR_CLOSE as appropriate, returning correct error codes
if it wants to handle close failures.

> @@ -418,17 +419,13 @@ virHookCall(int driver, const char *id, 
>      }
>  
>  cleanup:
> -    if (pipefd[0] >= 0) {
> -        if (close(pipefd[0]) < 0) {
> -            virReportSystemError(errno, "%s",
> -                             _("unable to close pipe for hook input"));
> -        }
> -    }
> -    if (pipefd[1] >= 0) {
> -        if (close(pipefd[1]) < 0) {
> -            virReportSystemError(errno, "%s",
> -                             _("unable to close pipe for hook input"));
> -        }
> +    if (VIR_CLOSE(pipefd[0]) < 0) {
> +        virReportSystemError(errno, "%s",
> +                         _("unable to close pipe for hook input"));
> +    }
> +    if (VIR_CLOSE(pipefd[1]) < 0) {
> +        virReportSystemError(errno, "%s",
> +                         _("unable to close pipe for hook input"));
>      }

Reporting errors while returning success.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|