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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list