6:19 a.m.
Using automated replacement with sed and editing I have now replaced all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
daemon/libvirtd.c | 46 ++++++---------
proxy/libvirt_proxy.c | 16 ++---
src/libvirt.c | 8 +-
src/lxc/lxc_container.c | 15 +++--
src/lxc/lxc_controller.c | 27 +++------
src/lxc/lxc_driver.c | 24 +++-----
src/node_device/node_device_linux_sysfs.c | 5 -
src/nwfilter/nwfilter_ebiptables_driver.c | 8 +-
src/openvz/openvz_conf.c | 35 ++++-------
src/openvz/openvz_driver.c | 3 -
src/phyp/phyp_driver.c | 13 ++--
src/qemu/qemu_conf.c | 30 ++++------
src/qemu/qemu_driver.c | 89 ++++++++++++------------------
src/qemu/qemu_monitor.c | 6 +-
src/remote/remote_driver.c | 18 ++----
src/secret/secret_driver.c | 12 +---
src/security/security_apparmor.c | 9 +--
src/security/security_selinux.c | 9 +--
src/security/virt-aa-helper.c | 9 +--
src/storage/storage_backend.c | 25 +++-----
src/storage/storage_backend_fs.c | 11 ++-
src/storage/storage_backend_iscsi.c | 5 -
src/storage/storage_backend_logical.c | 10 +--
src/storage/storage_backend_mpath.c | 5 -
src/storage/storage_backend_scsi.c | 8 +-
src/storage/storage_driver.c | 5 -
src/test/test_driver.c | 20 ++----
src/uml/uml_conf.c | 3 -
src/uml/uml_driver.c | 24 ++++----
src/util/bridge.c | 13 ++--
src/util/conf.c | 3 -
src/util/hooks.c | 21 +++----
src/util/interface.c | 12 ++--
src/util/logging.c | 6 +-
src/util/macvtap.c | 8 +-
src/util/pci.c | 6 --
src/util/storage_file.c | 5 +
src/util/util.c | 67 ++++++++++------------
src/util/uuid.c | 9 +--
src/util/virtaudit.c | 3 -
src/xen/proxy_internal.c | 8 +-
src/xen/xen_hypervisor.c | 12 ++--
src/xen/xen_inotify.c | 3 -
src/xen/xend_internal.c | 12 +---
tests/testutils.c | 21 ++++---
tools/console.c | 3 -
46 files changed, 327 insertions(+), 383 deletions(-)
Index: libvirt-acl/src/libvirt.c
===================================================================
--- libvirt-acl.orig/src/libvirt.c
+++ libvirt-acl/src/libvirt.c
@@ -10794,7 +10794,7 @@ virStreamRef(virStreamPtr stream)
* ... report an error ....
* done:
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
*
* Returns the number of bytes written, which may be less
* than requested.
@@ -10884,7 +10884,7 @@ error:
* ... report an error ....
* done:
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
*
*
* Returns the number of bytes read, which may be less
@@ -10964,7 +10964,7 @@ error:
* if (virStreamFinish(st) < 0)
* ...report an error...
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
*
* Returns 0 if all the data was successfully sent. The caller
* should invoke virStreamFinish(st) to flush the stream upon
@@ -11061,7 +11061,7 @@ cleanup:
* if (virStreamFinish(st) < 0)
* ...report an error...
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
*
* Returns 0 if all the data was successfully received. The caller
* should invoke virStreamFinish(st) to flush the stream upon
Index: libvirt-acl/src/lxc/lxc_container.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_container.c
+++ libvirt-acl/src/lxc/lxc_container.c
@@ -52,6 +52,7 @@
#include "util.h"
#include "memory.h"
#include "veth.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -127,7 +128,7 @@ static int lxcContainerExecInit(virDomai
static int lxcContainerSetStdio(int control, int ttyfd)
{
int rc = -1;
- int open_max, i;
+ int open_max, i, tpmfd;
if (setsid() < 0) {
virReportSystemError(errno, "%s",
@@ -145,8 +146,10 @@ static int lxcContainerSetStdio(int cont
* close all FDs before executing the container */
open_max = sysconf (_SC_OPEN_MAX);
for (i = 0; i < open_max; i++)
- if (i != ttyfd && i != control)
- close(i);
+ if (i != ttyfd && i != control) {
+ tpmfd = i;
+ VIR_FORCE_CLOSE(tpmfd);
+ }
if (dup2(ttyfd, 0) < 0) {
virReportSystemError(errno, "%s",
@@ -222,7 +225,7 @@ static int lxcContainerWaitForContinue(i
_("Failed to read the container continue
message"));
return -1;
}
- close(control);
+ VIR_FORCE_CLOSE(control);
DEBUG0("Received container continue message");
@@ -776,10 +779,10 @@ static int lxcContainerChild( void *data
VIR_FREE(ttyPath);
if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) {
- close(ttyfd);
+ VIR_FORCE_CLOSE(ttyfd);
return -1;
}
- close(ttyfd);
+ VIR_FORCE_CLOSE(ttyfd);
if (lxcContainerSetupMounts(vmDef, root) < 0)
return -1;
Index: libvirt-acl/src/lxc/lxc_controller.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_controller.c
+++ libvirt-acl/src/lxc/lxc_controller.c
@@ -48,6 +48,7 @@
#include "veth.h"
#include "memory.h"
#include "util.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -233,8 +234,7 @@ static int lxcMonitorServer(const char *
return fd;
error:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -409,7 +409,7 @@ static int lxcControllerMain(int monitor
goto cleanup;
}
if (client != -1) { /* Already connected, so kick new one out */
- close(fd);
+ VIR_FORCE_CLOSE(fd);
continue;
}
client = fd;
@@ -426,8 +426,7 @@ static int lxcControllerMain(int monitor
_("epoll_ctl(client) failed"));
goto cleanup;
}
- close(client);
- client = -1;
+ VIR_FORCE_CLOSE(client);
} else {
if (epollEvent.events & EPOLLIN) {
curFdOff = epollEvent.data.fd == appPty ? 0 : 1;
@@ -485,9 +484,9 @@ static int lxcControllerMain(int monitor
rc = 0;
cleanup:
- close(appPty);
- close(contPty);
- close(epollFd);
+ VIR_FORCE_CLOSE(appPty);
+ VIR_FORCE_CLOSE(contPty);
+ VIR_FORCE_CLOSE(epollFd);
return rc;
}
@@ -660,8 +659,7 @@ lxcControllerRun(virDomainDefPtr def,
control[1],
containerPtyPath)) < 0)
goto cleanup;
- close(control[1]);
- control[1] = -1;
+ VIR_FORCE_CLOSE(control[1]);
if (lxcControllerMoveInterfaces(nveths, veths, container) < 0)
goto cleanup;
@@ -679,13 +677,10 @@ lxcControllerRun(virDomainDefPtr def,
cleanup:
VIR_FREE(devptmx);
VIR_FREE(devpts);
- if (control[0] != -1)
- close(control[0]);
- if (control[1] != -1)
- close(control[1]);
+ VIR_FORCE_CLOSE(control[0]);
+ VIR_FORCE_CLOSE(control[1]);
VIR_FREE(containerPtyPath);
- if (containerPty != -1)
- close(containerPty);
+ VIR_FORCE_CLOSE(containerPty);
if (container > 1) {
int status;
Index: libvirt-acl/src/lxc/lxc_driver.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_driver.c
+++ libvirt-acl/src/lxc/lxc_driver.c
@@ -51,6 +51,7 @@
#include "uuid.h"
#include "stats_linux.h"
#include "hooks.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -974,7 +975,7 @@ static int lxcVmCleanup(lxc_driver_t *dr
}
virEventRemoveHandle(priv->monitorWatch);
- close(priv->monitor);
+ VIR_FORCE_CLOSE(priv->monitor);
virFileDeletePid(driver->stateDir, vm->def->name);
virDomainDeleteConfig(driver->stateDir, NULL, vm);
@@ -1156,8 +1157,7 @@ static int lxcMonitorClient(lxc_driver_t
error:
VIR_FREE(sockpath);
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -1544,14 +1544,10 @@ 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);
+ VIR_FORCE_CLOSE(logfd);
VIR_FREE(logfile);
return rc;
}
@@ -2011,8 +2007,7 @@ lxcReconnectVM(void *payload, const char
/* Read pid from controller */
if ((virFileReadPid(lxc_driver->stateDir, vm->def->name, &vm->pid))
!= 0) {
- close(priv->monitor);
- priv->monitor = -1;
+ VIR_FORCE_CLOSE(priv->monitor);
goto cleanup;
}
@@ -2042,8 +2037,7 @@ lxcReconnectVM(void *payload, const char
}
} else {
vm->def->id = -1;
- close(priv->monitor);
- priv->monitor = -1;
+ VIR_FORCE_CLOSE(priv->monitor);
}
cleanup:
Index: libvirt-acl/src/node_device/node_device_linux_sysfs.c
===================================================================
--- libvirt-acl.orig/src/node_device/node_device_linux_sysfs.c
+++ libvirt-acl/src/node_device/node_device_linux_sysfs.c
@@ -31,6 +31,7 @@
#include "virterror_internal.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#include <dirent.h>
#define VIR_FROM_THIS VIR_FROM_NODEDEV
@@ -104,9 +105,7 @@ int read_wwn_linux(int host, const char
}
out:
- if (fd != -1) {
- close(fd);
- }
+ VIR_FORCE_CLOSE(fd);
return retval;
}
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -37,6 +37,7 @@
#include "nwfilter_conf.h"
#include "nwfilter_gentech_driver.h"
#include "nwfilter_ebiptables_driver.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -2501,13 +2502,12 @@ ebiptablesWriteToTempFile(const char *st
}
VIR_FREE(header);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return filnam;
err_exit:
VIR_FREE(header);
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
unlink(filename);
return NULL;
}
@@ -3267,7 +3267,7 @@ iptablesCheckBridgeNFCallEnabled(bool is
lastReport = now;
}
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
}
}
}
Index: libvirt-acl/src/openvz/openvz_conf.c
===================================================================
--- libvirt-acl.orig/src/openvz/openvz_conf.c
+++ libvirt-acl/src/openvz/openvz_conf.c
@@ -50,6 +50,7 @@
#include "memory.h"
#include "util.h"
#include "nodeinfo.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_OPENVZ
@@ -109,7 +110,7 @@ openvzExtractVersionInfo(const char *cmd
cleanup2:
VIR_FREE(help);
- if (close(newstdout) < 0)
+ if (VIR_CLOSE(newstdout) < 0)
ret = -1;
rewait:
@@ -569,7 +570,7 @@ openvzWriteConfigParam(const char * conf
goto error;
temp_fd = open(temp_file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
if (temp_fd == -1) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto error;
}
@@ -590,12 +591,10 @@ openvzWriteConfigParam(const char * conf
safewrite(temp_fd, "\"\n", 2) < 0)
goto error;
- if (close(fd) < 0)
+ if (VIR_CLOSE(fd) < 0)
goto error;
- fd = -1;
- if (close(temp_fd) < 0)
+ if (VIR_CLOSE(temp_fd) < 0)
goto error;
- temp_fd = -1;
if (rename(temp_file, conf_file) < 0)
goto error;
@@ -603,10 +602,8 @@ openvzWriteConfigParam(const char * conf
return 0;
error:
- if (fd != -1)
- close(fd);
- if (temp_fd != -1)
- close(temp_fd);
+ VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_CLOSE(temp_fd);
if (temp_file)
unlink(temp_file);
VIR_FREE(temp_file);
@@ -662,7 +659,7 @@ openvzReadConfigParam(const char * conf_
}
}
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (ret == 0 && found)
ret = 1;
@@ -703,7 +700,7 @@ openvz_copyfile(char* from_path, char* t
return -1;
copy_fd = open(to_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
if (copy_fd == -1) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -716,19 +713,16 @@ openvz_copyfile(char* from_path, char* t
goto error;
}
- if (close(fd) < 0)
+ if (VIR_CLOSE(fd) < 0)
goto error;
- fd = -1;
- if (close(copy_fd) < 0)
+ if (VIR_CLOSE(copy_fd) < 0)
goto error;
return 0;
error:
- if (fd != -1)
- close(fd);
- if (copy_fd != -1)
- close(copy_fd);
+ VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_CLOSE(copy_fd);
return -1;
}
@@ -880,8 +874,7 @@ openvzGetVPSUUID(int vpsid, char *uuidst
}
retval = 0;
cleanup:
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(conf_file);
return retval;
Index: libvirt-acl/src/openvz/openvz_driver.c
===================================================================
--- libvirt-acl.orig/src/openvz/openvz_driver.c
+++ libvirt-acl/src/openvz/openvz_driver.c
@@ -57,6 +57,7 @@
#include "nodeinfo.h"
#include "memory.h"
#include "bridge.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_OPENVZ
@@ -1540,7 +1541,7 @@ Version: 2.2
}
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (ret < 0)
return -1;
Index: libvirt-acl/src/phyp/phyp_driver.c
===================================================================
--- libvirt-acl.orig/src/phyp/phyp_driver.c
+++ libvirt-acl/src/phyp/phyp_driver.c
@@ -58,6 +58,7 @@
#include "domain_conf.h"
#include "storage_conf.h"
#include "nodeinfo.h"
+#include "files.h"
#include "phyp_driver.h"
@@ -457,11 +458,11 @@ phypUUIDTable_WriteFile(virConnectPtr co
}
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
err:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -672,11 +673,11 @@ phypUUIDTable_ReadFile(virConnectPtr con
} else
virReportOOMError();
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
err:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -764,7 +765,7 @@ phypUUIDTable_Pull(virConnectPtr conn)
}
break;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto exit;
exit:
@@ -1001,7 +1002,7 @@ openSSHSession(virConnectPtr conn, virCo
if (connect(sock, cur->ai_addr, cur->ai_addrlen) == 0) {
goto connected;
}
- close(sock);
+ VIR_FORCE_CLOSE(sock);
}
cur = cur->ai_next;
}
Index: libvirt-acl/src/qemu/qemu_conf.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_conf.c
+++ libvirt-acl/src/qemu/qemu_conf.c
@@ -55,6 +55,7 @@
#include "macvtap.h"
#include "cpu/cpu.h"
#include "domain_nwfilter.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -530,7 +531,7 @@ qemudProbeMachineTypes(const char *binar
cleanup2:
VIR_FREE(output);
cleanup:
- if (close(newstdout) < 0)
+ if (VIR_CLOSE(newstdout) < 0)
ret = -1;
rewait:
@@ -780,7 +781,7 @@ qemudProbeCPUModels(const char *qemu,
cleanup:
VIR_FREE(output);
- if (close(newstdout) < 0)
+ if (VIR_CLOSE(newstdout) < 0)
ret = -1;
rewait:
@@ -1421,7 +1422,7 @@ static void qemudParsePCIDeviceStrs(cons
cleanup:
VIR_FREE(pciassign);
- close(newstderr);
+ VIR_FORCE_CLOSE(newstderr);
rewait:
if (waitpid(child, &status, 0) != child) {
if (errno == EINTR)
@@ -1481,7 +1482,7 @@ int qemudExtractVersionInfo(const char *
cleanup2:
VIR_FREE(help);
- if (close(newstdout) < 0)
+ if (VIR_CLOSE(newstdout) < 0)
ret = -1;
rewait:
@@ -1596,8 +1597,7 @@ qemudPhysIfaceConnect(virConnectPtr conn
if ((net->filter) && (net->ifname)) {
err = virDomainConfNWFilterInstantiate(conn, net);
if (err) {
- close(rc);
- rc = -1;
+ VIR_FORCE_CLOSE(rc);
delMacvtap(net->ifname, net->mac, net->data.direct.linkdev,
&net->data.direct.virtPortProfile);
VIR_FREE(net->ifname);
@@ -1742,10 +1742,8 @@ qemudNetworkIfaceConnect(virConnectPtr c
if (tapfd >= 0) {
if ((net->filter) && (net->ifname)) {
err = virDomainConfNWFilterInstantiate(conn, net);
- if (err) {
- close(tapfd);
- tapfd = -1;
- }
+ if (err)
+ VIR_FORCE_CLOSE(tapfd);
}
}
@@ -4557,7 +4555,7 @@ int qemudBuildCommandLine(virConnectPtr
if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
virDomainConfNWFilterTeardown(net);
- close(tapfd);
+ VIR_FORCE_CLOSE(tapfd);
goto no_memory;
}
@@ -4576,7 +4574,7 @@ int qemudBuildCommandLine(virConnectPtr
if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
virDomainConfNWFilterTeardown(net);
- close(tapfd);
+ VIR_FORCE_CLOSE(tapfd);
goto no_memory;
}
@@ -4596,7 +4594,7 @@ int qemudBuildCommandLine(virConnectPtr
int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags);
if (vhostfd >= 0) {
if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
- close(vhostfd);
+ VIR_FORCE_CLOSE(vhostfd);
goto no_memory;
}
@@ -5094,14 +5092,14 @@ int qemudBuildCommandLine(virConnectPtr
if (configfd >= 0) {
if (virAsprintf(&configfd_name, "%d", configfd)
< 0) {
- close(configfd);
+ VIR_FORCE_CLOSE(configfd);
virReportOOMError();
goto no_memory;
}
if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
VIR_FREE(configfd_name);
- close(configfd);
+ VIR_FORCE_CLOSE(configfd);
goto no_memory;
}
@@ -5194,7 +5192,7 @@ int qemudBuildCommandLine(virConnectPtr
if (vmfds &&
*vmfds) {
for (i = 0; i < *nvmfds; i++)
- close((*vmfds)[i]);
+ VIR_FORCE_CLOSE((*vmfds)[i]);
VIR_FREE(*vmfds);
*nvmfds = 0;
}
Index: libvirt-acl/src/qemu/qemu_driver.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_driver.c
+++ libvirt-acl/src/qemu/qemu_driver.c
@@ -81,6 +81,7 @@
#include "hooks.h"
#include "storage_file.h"
#include "virtaudit.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -761,7 +762,7 @@ qemudLogFD(struct qemud_driver *driver,
if (virSetCloseExec(fd) < 0) {
virReportSystemError(errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
return fd;
@@ -793,14 +794,14 @@ qemudLogReadFD(const char* logDir, const
if (virSetCloseExec(fd) < 0) {
virReportSystemError(errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) {
- virReportSystemError(pos < 0 ? 0 : errno,
+ virReportSystemError(pos < 0 ? 0 : errno,
_("Unable to seek to %lld in %s"),
(long long) pos, logfile);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
}
return fd;
}
@@ -2392,7 +2393,7 @@ cleanup:
}
closelog:
- if (close(logfd) < 0) {
+ if (VIR_CLOSE(logfd) < 0) {
char ebuf[4096];
VIR_WARN("Unable to close logfile: %s",
virStrerror(errno, ebuf, sizeof ebuf));
@@ -2971,13 +2972,13 @@ static int qemudNextFreeVNCPort(struct q
return -1;
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse))
< 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
break;
}
if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) {
/* Not in use, lets grab it */
- close(fd);
+ VIR_FORCE_CLOSE(fd);
/* Add port to bitmap of reserved ports */
if (virBitmapSetBit(driver->reservedVNCPorts,
i - QEMU_VNC_PORT_MIN) < 0) {
@@ -2986,7 +2987,7 @@ static int qemudNextFreeVNCPort(struct q
}
return i;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (errno == EADDRINUSE) {
/* In use, try next */
@@ -3238,7 +3239,7 @@ qemuPrepareChardevDevice(virDomainDefPtr
return -1;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
}
@@ -3955,7 +3956,7 @@ static int qemudStartVMDaemon(virConnect
if (vmfds) {
for (i = 0 ; i < nvmfds ; i++) {
- close(vmfds[i]);
+ VIR_FORCE_CLOSE(vmfds[i]);
}
VIR_FREE(vmfds);
}
@@ -4008,8 +4009,7 @@ static int qemudStartVMDaemon(virConnect
if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
goto cleanup;
- if (logfile != -1)
- close(logfile);
+ VIR_FORCE_CLOSE(logfile);
return 0;
@@ -4019,8 +4019,7 @@ cleanup:
* pretend we never started it */
qemudShutdownVMDaemon(driver, vm, 0);
- if (logfile != -1)
- close(logfile);
+ VIR_FORCE_CLOSE(logfile);
return -1;
}
@@ -4295,7 +4294,7 @@ static int kvmGetMaxVCPUs(void) {
if (r > 0)
maxvcpus = r;
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return maxvcpus;
}
@@ -5397,10 +5396,10 @@ static int qemudDomainSaveFlag(struct qe
goto endjob;
}
if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto endjob;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno, _("unable to close %s"), path);
goto endjob;
}
@@ -5796,7 +5795,7 @@ static int qemudDomainCoreDump(virDomain
goto endjob;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("unable to save file %s"),
path);
@@ -6424,8 +6423,7 @@ static int qemudOpenAsUID(const char *pa
/* parent */
/* parent doesn't need the write side of the pipe */
- close(pipefd[1]);
- pipefd[1] = -1;
+ VIR_FORCE_CLOSE(pipefd[1]);
if (forkRet < 0) {
virReportSystemError(errno,
@@ -6437,10 +6435,8 @@ static int qemudOpenAsUID(const char *pa
fd = pipefd[0];
pipefd[0] = -1;
parent_cleanup:
- if (pipefd[0] != -1)
- close(pipefd[0]);
- if (pipefd[1] != -1)
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(pipefd[0]);
+ VIR_FORCE_CLOSE(pipefd[1]);
if ((fd < 0) && (*child_pid > 0)) {
/* a child process was started and subsequently an error
occurred in the parent, so we need to wait for it to
@@ -6466,7 +6462,7 @@ parent_cleanup:
struct passwd pwd, *pwd_result;
/* child doesn't need the read side of the pipe */
- close(pipefd[0]);
+ VIR_FORCE_CLOSE(pipefd[0]);
if (forkRet < 0) {
exit_code = errno;
@@ -6531,10 +6527,8 @@ parent_cleanup:
child_cleanup:
VIR_FREE(buf);
- if (fd != -1)
- close(fd);
- if (pipefd[1] != -1)
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_CLOSE(pipefd[1]);
_exit(exit_code);
}
@@ -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"));
+ }
if (read_pid != -1) {
/* reap the process that read the file */
@@ -6699,8 +6695,7 @@ qemudDomainSaveImageStartVM(virConnectPt
/* empty */
}
}
- if (intermediatefd != -1)
- close(intermediatefd);
+ VIR_FORCE_CLOSE(intermediatefd);
wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status);
fd = -1;
@@ -8065,9 +8060,7 @@ static int qemudDomainAttachNetDevice(vi
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- if (tapfd != -1)
- close(tapfd);
- tapfd = -1;
+ VIR_FORCE_CLOSE(tapfd);
if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -8117,8 +8110,7 @@ cleanup:
VIR_FREE(nicstr);
VIR_FREE(netstr);
VIR_FREE(tapfd_name);
- if (tapfd != -1)
- close(tapfd);
+ VIR_FORCE_CLOSE(tapfd);
return ret;
@@ -8247,8 +8239,7 @@ static int qemudDomainAttachHostPciDevic
VIR_FREE(devstr);
VIR_FREE(configfd_name);
- if (configfd >= 0)
- close(configfd);
+ VIR_FORCE_CLOSE(configfd);
return 0;
@@ -8262,8 +8253,7 @@ error:
VIR_FREE(devstr);
VIR_FREE(configfd_name);
- if (configfd >= 0)
- close(configfd);
+ VIR_FORCE_CLOSE(configfd);
return -1;
}
@@ -10357,8 +10347,7 @@ static int qemuDomainGetBlockInfo(virDom
}
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (vm)
virDomainObjUnlock(vm);
return ret;
@@ -10673,8 +10662,7 @@ cleanup:
static void qemuStreamMigFree(struct qemuStreamMigFile *qemust)
{
- if (qemust->fd != -1)
- close(qemust->fd);
+ VIR_FORCE_CLOSE(qemust->fd);
VIR_FREE(qemust);
}
@@ -11510,10 +11498,8 @@ finish:
qemuDomainObjExitRemoteWithDriver(driver, vm);
cleanup:
- if (client_sock != -1)
- close(client_sock);
- if (qemu_sock != -1)
- close(qemu_sock);
+ VIR_FORCE_CLOSE(client_sock);
+ VIR_FORCE_CLOSE(qemu_sock);
if (ddomain)
virUnrefDomain(ddomain);
@@ -12292,8 +12278,7 @@ cleanup:
VIR_FREE(snapFile);
VIR_FREE(snapDir);
VIR_FREE(newxml);
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
Index: libvirt-acl/src/qemu/qemu_monitor.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_monitor.c
+++ libvirt-acl/src/qemu/qemu_monitor.c
@@ -36,6 +36,7 @@
#include "virterror_internal.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -283,7 +284,7 @@ qemuMonitorOpenUnix(const char *monitor)
return monfd;
error:
- close(monfd);
+ VIR_FORCE_CLOSE(monfd);
return -1;
}
@@ -694,8 +695,7 @@ void qemuMonitorClose(qemuMonitorPtr mon
if (!mon->closed) {
if (mon->watch)
virEventRemoveHandle(mon->watch);
- if (mon->fd != -1)
- close(mon->fd);
+ VIR_FORCE_CLOSE(mon->fd);
/* NB: ordinarily one might immediately set mon->watch to -1
* and mon->fd to -1, but there may be a callback active
* that is still relying on these fields being valid. So
Index: libvirt-acl/src/remote/remote_driver.c
===================================================================
--- libvirt-acl.orig/src/remote/remote_driver.c
+++ libvirt-acl/src/remote/remote_driver.c
@@ -82,6 +82,7 @@
#include "util.h"
#include "event.h"
#include "ignore-value.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_REMOTE
@@ -711,7 +712,7 @@ doRemoteOpen (virConnectPtr conn,
if (errno == ECONNREFUSED &&
flags & VIR_DRV_OPEN_REMOTE_AUTOSTART &&
trials < 20) {
- close(priv->sock);
+ VIR_FORCE_CLOSE(priv->sock);
priv->sock = -1;
if (trials > 0 ||
remoteForkDaemon() == 0) {
@@ -955,8 +956,7 @@ doRemoteOpen (virConnectPtr conn,
failed:
/* Close the socket if we failed. */
- if (priv->errfd >= 0)
- close(priv->errfd);
+ VIR_FORCE_CLOSE(priv->errfd);
if (priv->sock >= 0) {
if (priv->uses_tls && priv->session) {
@@ -977,10 +977,8 @@ retry:
#endif
}
- if (wakeupFD[0] >= 0) {
- close(wakeupFD[0]);
- close(wakeupFD[1]);
- }
+ VIR_FORCE_CLOSE(wakeupFD[0]);
+ VIR_FORCE_CLOSE(wakeupFD[1]);
VIR_FREE(priv->hostname);
goto cleanup;
@@ -1456,10 +1454,8 @@ retry:
} while (reap != -1 && reap != priv->pid);
}
#endif
- if (priv->wakeupReadFD >= 0) {
- close(priv->wakeupReadFD);
- close(priv->wakeupSendFD);
- }
+ VIR_FORCE_CLOSE(priv->wakeupReadFD);
+ VIR_FORCE_CLOSE(priv->wakeupSendFD);
/* Free hostname copy */
Index: libvirt-acl/src/secret/secret_driver.c
===================================================================
--- libvirt-acl.orig/src/secret/secret_driver.c
+++ libvirt-acl/src/secret/secret_driver.c
@@ -41,6 +41,7 @@
#include "util.h"
#include "uuid.h"
#include "virterror_internal.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_SECRET
@@ -181,7 +182,7 @@ replaceFile(const char *filename, void *
tmp_path);
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno, _("error closing '%s'"),
tmp_path);
goto cleanup;
}
@@ -196,8 +197,7 @@ replaceFile(const char *filename, void *
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (tmp_path != NULL) {
unlink(tmp_path);
VIR_FREE(tmp_path);
@@ -394,8 +394,7 @@ secretLoadValue(virSecretDriverStatePtr
virReportSystemError(errno, _("cannot read '%s'"), filename);
goto cleanup;
}
- close(fd);
- fd = -1;
+ VIR_FORCE_CLOSE(fd);
if (!base64_decode_alloc(contents, st.st_size, &value, &value_size)) {
virSecretReportError(VIR_ERR_INTERNAL_ERROR,
@@ -422,8 +421,7 @@ cleanup:
memset(contents, 0, st.st_size);
VIR_FREE(contents);
}
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(filename);
return ret;
}
Index: libvirt-acl/src/security/security_apparmor.c
===================================================================
--- libvirt-acl.orig/src/security/security_apparmor.c
+++ libvirt-acl/src/security/security_apparmor.c
@@ -37,6 +37,7 @@
#include "uuid.h"
#include "pci.h"
#include "hostusb.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
#define SECURITY_APPARMOR_VOID_DOI "0"
@@ -215,7 +216,7 @@ load_profile(virSecurityDriverPtr drv,
virReportSystemError(errno, "%s", _("unable to write to
pipe"));
goto clean;
}
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(pipefd[1]);
rc = 0;
rewait:
@@ -233,10 +234,8 @@ load_profile(virSecurityDriverPtr drv,
clean:
VIR_FREE(xml);
- if (pipefd[0] > 0)
- close(pipefd[0]);
- if (pipefd[1] > 0)
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(pipefd[0]);
+ VIR_FORCE_CLOSE(pipefd[1]);
return rc;
}
Index: libvirt-acl/src/security/security_selinux.c
===================================================================
--- libvirt-acl.orig/src/security/security_selinux.c
+++ libvirt-acl/src/security/security_selinux.c
@@ -30,6 +30,7 @@
#include "storage_file.h"
#include "uuid.h"
#include "virtaudit.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -122,10 +123,10 @@ SELinuxInitialize(void)
virReportSystemError(errno,
_("cannot read SELinux virtual domain context file
%s"),
selinux_virtual_domain_context_path());
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
ptr = strchrnul(default_domain_context, '\n');
*ptr = '\0';
@@ -141,10 +142,10 @@ SELinuxInitialize(void)
virReportSystemError(errno,
_("cannot read SELinux virtual image context file
%s"),
selinux_virtual_image_context_path());
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
ptr = strchrnul(default_image_context, '\n');
if (*ptr == '\n') {
Index: libvirt-acl/src/security/virt-aa-helper.c
===================================================================
--- libvirt-acl.orig/src/security/virt-aa-helper.c
+++ libvirt-acl/src/security/virt-aa-helper.c
@@ -37,6 +37,7 @@
#include "uuid.h"
#include "hostusb.h"
#include "pci.h"
+#include "files.h"
static char *progname;
@@ -278,12 +279,12 @@ update_include_file(const char *include_
}
if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */
- close(fd);
+ VIR_FORCE_CLOSE(fd);
vah_error(NULL, 0, "failed to write to profile");
goto clean;
}
- if (close(fd) != 0) {
+ if (VIR_CLOSE(fd) != 0) {
vah_error(NULL, 0, "failed to close or write to profile");
goto clean;
}
@@ -385,12 +386,12 @@ create_profile(const char *profile, cons
}
if (safewrite(fd, pcontent, plen - 1) < 0) { /* don't write the '\0'
*/
- close(fd);
+ VIR_FORCE_CLOSE(fd);
vah_error(NULL, 0, "failed to write to profile");
goto clean_all;
}
- if (close(fd) != 0) {
+ if (VIR_CLOSE(fd) != 0) {
vah_error(NULL, 0, "failed to close or write to profile");
goto clean_all;
}
Index: libvirt-acl/src/storage/storage_backend.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend.c
+++ libvirt-acl/src/storage/storage_backend.c
@@ -51,6 +51,7 @@
#include "storage_file.h"
#include "storage_backend.h"
#include "logging.h"
+#include "files.h"
#if WITH_STORAGE_LVM
# include "storage_backend_logical.h"
@@ -181,7 +182,7 @@ virStorageBackendCopyToFD(virStorageVolD
} while ((amtleft -= 512) > 0);
}
- if (inputfd != -1 && close(inputfd) < 0) {
+ if (VIR_CLOSE(inputfd) < 0) {
ret = -errno;
virReportSystemError(errno,
_("cannot close file '%s'"),
@@ -193,8 +194,7 @@ virStorageBackendCopyToFD(virStorageVolD
*total -= remain;
cleanup:
- if (inputfd != -1)
- close(inputfd);
+ VIR_FORCE_CLOSE(inputfd);
VIR_FREE(buf);
@@ -251,7 +251,7 @@ virStorageBackendCreateBlockFrom(virConn
vol->target.path, vol->target.perms.mode);
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot close file '%s'"),
vol->target.path);
@@ -261,8 +261,7 @@ virStorageBackendCreateBlockFrom(virConn
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -608,7 +607,7 @@ static int virStorageBackendQEMUImgBacki
cleanup:
VIR_FREE(help);
- close(newstdout);
+ VIR_FORCE_CLOSE(newstdout);
rewait:
if (child) {
if (waitpid(child, &status, 0) != child) {
@@ -997,7 +996,7 @@ virStorageBackendVolOpenCheckMode(const
virReportSystemError(errno,
_("cannot stat file '%s'"),
path);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -1009,7 +1008,7 @@ virStorageBackendVolOpenCheckMode(const
mode = VIR_STORAGE_VOL_OPEN_BLOCK;
if (!(mode & flags)) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (mode & VIR_STORAGE_VOL_OPEN_ERROR) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1045,7 +1044,7 @@ virStorageBackendUpdateVolTargetInfo(vir
allocation,
capacity);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -1461,10 +1460,8 @@ virStorageBackendRunProgRegex(virStorage
if (list)
fclose(list);
- else {
- if (fd >= 0)
- close(fd);
- }
+ else
+ VIR_FORCE_CLOSE(fd);
while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR);
Index: libvirt-acl/src/storage/storage_backend_fs.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend_fs.c
+++ libvirt-acl/src/storage/storage_backend_fs.c
@@ -45,6 +45,7 @@
#include "util.h"
#include "memory.h"
#include "xml.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -72,25 +73,25 @@ virStorageBackendProbeTarget(virStorageV
if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
allocation,
capacity)) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
memset(&meta, 0, sizeof(meta));
if ((target->format = virStorageFileProbeFormatFromFD(target->path, fd)) <
0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
if (virStorageFileGetMetadataFromFD(target->path, fd,
target->format,
&meta) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (meta.backingStore) {
*backingStore = meta.backingStore;
@@ -98,7 +99,7 @@ virStorageBackendProbeTarget(virStorageV
if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
if ((*backingStoreFormat
= virStorageFileProbeFormat(*backingStore)) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
} else {
Index: libvirt-acl/src/storage/storage_backend_iscsi.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend_iscsi.c
+++ libvirt-acl/src/storage/storage_backend_iscsi.c
@@ -41,6 +41,7 @@
#include "util.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -237,9 +238,7 @@ out:
if (fp != NULL) {
fclose(fp);
} else {
- if (fd != -1) {
- close(fd);
- }
+ VIR_FORCE_CLOSE(fd);
}
return ret;
Index: libvirt-acl/src/storage/storage_backend_logical.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend_logical.c
+++ libvirt-acl/src/storage/storage_backend_logical.c
@@ -37,6 +37,7 @@
#include "util.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -408,10 +409,10 @@ virStorageBackendLogicalBuildPool(virCon
virReportSystemError(errno,
_("cannot clear device header of
'%s'"),
pool->def->source.devices[i].path);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot close device '%s'"),
pool->def->source.devices[i].path);
@@ -622,7 +623,7 @@ virStorageBackendLogicalCreateVol(virCon
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot close file '%s'"),
vol->target.path);
@@ -641,8 +642,7 @@ virStorageBackendLogicalCreateVol(virCon
return 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
return -1;
}
Index: libvirt-acl/src/storage/storage_backend_mpath.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend_mpath.c
+++ libvirt-acl/src/storage/storage_backend_mpath.c
@@ -35,6 +35,7 @@
#include "storage_backend.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -61,9 +62,7 @@ virStorageBackendMpathUpdateVolTargetInf
ret = 0;
out:
- if (fd != -1) {
- close(fd);
- }
+ VIR_FORCE_CLOSE(fd);
return ret;
}
Index: libvirt-acl/src/storage/storage_backend_scsi.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend_scsi.c
+++ libvirt-acl/src/storage/storage_backend_scsi.c
@@ -32,6 +32,7 @@
#include "storage_backend_scsi.h"
#include "memory.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -154,8 +155,7 @@ virStorageBackendSCSIUpdateVolTargetInfo
ret = 0;
cleanup:
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -572,14 +572,14 @@ virStorageBackendSCSITriggerRescan(uint3
if (safewrite(fd,
LINUX_SYSFS_SCSI_HOST_SCAN_STRING,
sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) {
-
+ VIR_FORCE_CLOSE(fd);
virReportSystemError(errno,
_("Write to '%s' to trigger host scan
failed"),
path);
retval = -1;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
free_path:
VIR_FREE(path);
out:
Index: libvirt-acl/src/storage/storage_driver.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_driver.c
+++ libvirt-acl/src/storage/storage_driver.c
@@ -45,6 +45,7 @@
#include "memory.h"
#include "storage_backend.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -1664,9 +1665,7 @@ storageVolumeWipeInternal(virStorageVolD
out:
VIR_FREE(writebuf);
- if (fd != -1) {
- close(fd);
- }
+ VIR_FORCE_CLOSE(fd);
return ret;
}
Index: libvirt-acl/src/test/test_driver.c
===================================================================
--- libvirt-acl.orig/src/test/test_driver.c
+++ libvirt-acl/src/test/test_driver.c
@@ -50,6 +50,7 @@
#include "xml.h"
#include "threads.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_TEST
@@ -788,8 +789,7 @@ static int testOpenFromFile(virConnectPt
_("Invalid XML in file '%s'"), file);
goto error;
}
- close(fd);
- fd = -1;
+ VIR_FORCE_CLOSE(fd);
root = xmlDocGetRootElement(xml);
if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) {
@@ -1101,8 +1101,7 @@ static int testOpenFromFile(virConnectPt
VIR_FREE(networks);
VIR_FREE(ifaces);
VIR_FREE(pools);
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
virDomainObjListDeinit(&privconn->domains);
virNetworkObjListFree(&privconn->networks);
virInterfaceObjListFree(&privconn->ifaces);
@@ -1752,7 +1751,7 @@ static int testDomainSave(virDomainPtr d
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("saving domain '%s' to '%s': write
failed"),
domain->name, path);
@@ -1779,8 +1778,7 @@ cleanup:
* in either case we're already in a failure scenario
* and have reported a earlier error */
if (ret != 0) {
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
unlink(path);
}
if (privdom)
@@ -1870,8 +1868,7 @@ static int testDomainRestore(virConnectP
cleanup:
virDomainDefFree(def);
VIR_FREE(xml);
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (dom)
virDomainObjUnlock(dom);
if (event)
@@ -1911,7 +1908,7 @@ static int testDomainCoreDump(virDomainP
domain->name, to);
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("domain '%s' coredump: write failed:
%s"),
domain->name, to);
@@ -1932,8 +1929,7 @@ static int testDomainCoreDump(virDomainP
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (privdom)
virDomainObjUnlock(privdom);
if (event)
Index: libvirt-acl/src/uml/uml_conf.c
===================================================================
--- libvirt-acl.orig/src/uml/uml_conf.c
+++ libvirt-acl/src/uml/uml_conf.c
@@ -47,6 +47,7 @@
#include "bridge.h"
#include "logging.h"
#include "domain_nwfilter.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_UML
@@ -367,7 +368,7 @@ umlBuildCommandLineChr(virDomainChrDefPt
}
if (virAsprintf(&ret, "%s%d=null,fd:%d", dev,
def->target.port, fd_out) < 0) {
virReportOOMError();
- close(fd_out);
+ VIR_FORCE_CLOSE(fd_out);
return NULL;
}
FD_SET(fd_out, keepfd);
Index: libvirt-acl/src/uml/uml_driver.c
===================================================================
--- libvirt-acl.orig/src/uml/uml_driver.c
+++ libvirt-acl/src/uml/uml_driver.c
@@ -59,6 +59,7 @@
#include "datatypes.h"
#include "logging.h"
#include "domain_nwfilter.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_UML
@@ -533,7 +534,7 @@ umlShutdown(void) {
umlDriverLock(uml_driver);
if (uml_driver->inotifyWatch != -1)
virEventRemoveHandle(uml_driver->inotifyWatch);
- close(uml_driver->inotifyFD);
+ VIR_FORCE_CLOSE(uml_driver->inotifyFD);
virCapabilitiesFree(uml_driver->caps);
/* shutdown active VMs
@@ -659,8 +660,7 @@ restat:
if (bind(priv->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) {
virReportSystemError(errno,
"%s", _("cannot bind socket"));
- close(priv->monitor);
- priv->monitor = -1;
+ VIR_FORCE_CLOSE(priv->monitor);
return -1;
}
@@ -811,7 +811,7 @@ static int umlStartVMDaemon(virConnectPt
virDomainObjPtr vm) {
const char **argv = NULL, **tmp;
const char **progenv = NULL;
- int i, ret;
+ int i, ret, tmpfd;
pid_t pid;
char *logfile;
int logfd = -1;
@@ -870,13 +870,13 @@ static int umlStartVMDaemon(virConnectPt
if (umlSetCloseExec(logfd) < 0) {
virReportSystemError(errno,
"%s", _("Unable to set VM logfile
close-on-exec flag"));
- close(logfd);
+ VIR_FORCE_CLOSE(logfd);
return -1;
}
if (umlBuildCommandLine(conn, driver, vm, &keepfd,
&argv, &progenv) < 0) {
- close(logfd);
+ VIR_FORCE_CLOSE(logfd);
virDomainConfVMNWFilterTeardown(vm);
umlCleanupTapDevices(conn, vm);
return -1;
@@ -912,15 +912,17 @@ static int umlStartVMDaemon(virConnectPt
-1, &logfd, &logfd,
VIR_EXEC_CLEAR_CAPS,
NULL, NULL, NULL);
- close(logfd);
+ VIR_FORCE_CLOSE(logfd);
/*
* At the moment, the only thing that populates keepfd is
* umlBuildCommandLineChr. We want to close every fd it opens.
*/
for (i = 0; i < FD_SETSIZE; i++)
- if (FD_ISSET(i, &keepfd))
- close(i);
+ if (FD_ISSET(i, &keepfd)) {
+ tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
+ }
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
@@ -957,9 +959,7 @@ static void umlShutdownVMDaemon(virConne
virKillProcess(vm->pid, SIGTERM);
- if (priv->monitor != -1)
- close(priv->monitor);
- priv->monitor = -1;
+ VIR_FORCE_CLOSE(priv->monitor);
if ((ret = waitpid(vm->pid, NULL, 0)) != vm->pid) {
VIR_WARN("Got unexpected pid %d != %d",
Index: libvirt-acl/src/util/bridge.c
===================================================================
--- libvirt-acl.orig/src/util/bridge.c
+++ libvirt-acl/src/util/bridge.c
@@ -24,6 +24,7 @@
#if defined(WITH_BRIDGE)
# include "bridge.h"
+# include "files.h"
# include <stdlib.h>
# include <stdio.h>
@@ -81,12 +82,12 @@ brInit(brControl **ctlp)
if ((flags = fcntl(fd, F_GETFD)) < 0 ||
fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
int err = errno;
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return err;
}
if (VIR_ALLOC(*ctlp) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ENOMEM;
}
@@ -107,7 +108,7 @@ brShutdown(brControl *ctl)
if (!ctl)
return;
- close(ctl->fd);
+ VIR_FORCE_CLOSE(ctl->fd);
ctl->fd = 0;
VIR_FREE(ctl);
@@ -539,11 +540,11 @@ brAddTap(brControl *ctl,
if (tapfd)
*tapfd = fd;
else
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
error:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return errno;
}
@@ -574,7 +575,7 @@ int brDeleteTap(brControl *ctl,
}
error:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return errno;
}
Index: libvirt-acl/src/util/conf.c
===================================================================
--- libvirt-acl.orig/src/util/conf.c
+++ libvirt-acl/src/util/conf.c
@@ -24,6 +24,7 @@
#include "util.h"
#include "c-ctype.h"
#include "memory.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_CONF
@@ -954,7 +955,7 @@ virConfWriteFile(const char *filename, v
content = virBufferContentAndReset(&buf);
ret = safewrite(fd, content, use);
VIR_FREE(content);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (ret != (int)use) {
virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"));
return -1;
Index: libvirt-acl/src/util/interface.c
===================================================================
--- libvirt-acl.orig/src/util/interface.c
+++ libvirt-acl/src/util/interface.c
@@ -39,6 +39,7 @@
#include "util.h"
#include "interface.h"
#include "virterror_internal.h"
+#include "files.h"
#define ifaceError(code, ...) \
virReportErrorHelper(NULL, VIR_FROM_NET, code, __FILE__, \
@@ -82,7 +83,7 @@ ifaceGetFlags(const char *ifname, short
*flags = ifr.ifr_flags;
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return rc;
}
@@ -161,7 +162,7 @@ static int chgIfaceFlags(const char *ifn
}
err_exit:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return rc;
}
@@ -259,8 +260,7 @@ ifaceCheck(bool reportError, const char
}
err_exit:
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return rc;
}
@@ -326,7 +326,7 @@ ifaceGetIndex(bool reportError, const ch
}
err_exit:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return rc;
}
@@ -373,7 +373,7 @@ ifaceGetVlanID(const char *vlanifname, i
*vlanid = vlanargs.u.VID;
err_exit:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return rc;
}
Index: libvirt-acl/src/util/logging.c
===================================================================
--- libvirt-acl.orig/src/util/logging.c
+++ libvirt-acl/src/util/logging.c
@@ -40,6 +40,7 @@
#include "util.h"
#include "buf.h"
#include "threads.h"
+#include "files.h"
/*
* Macro used to format the message as a string in virLogMessage
@@ -603,8 +604,7 @@ static int virLogOutputToFd(const char *
static void virLogCloseFd(void *data) {
int fd = (long) data;
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
}
static int virLogAddOutputToStderr(int priority) {
@@ -622,7 +622,7 @@ static int virLogAddOutputToFile(int pri
return(-1);
if (virLogDefineOutput(virLogOutputToFd, virLogCloseFd, (void *)(long)fd,
priority, VIR_LOG_TO_FILE, file, 0) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return(-1);
}
return(0);
Index: libvirt-acl/src/util/macvtap.c
===================================================================
--- libvirt-acl.orig/src/util/macvtap.c
+++ libvirt-acl/src/util/macvtap.c
@@ -52,6 +52,7 @@
# include "conf/domain_conf.h"
# include "virterror_internal.h"
# include "uuid.h"
+# include "files.h"
# define VIR_FROM_THIS VIR_FROM_NET
@@ -94,7 +95,7 @@ static int nlOpen(void)
static void nlClose(int fd)
{
- close(fd);
+ VIR_FORCE_CLOSE(fd);
}
@@ -689,7 +690,7 @@ create_name:
if (rc >= 0) {
if (configMacvtapTap(rc, vnet_hdr) < 0) {
- close(rc);
+ VIR_FORCE_CLOSE(rc);
rc = -1;
goto disassociate_exit;
}
@@ -778,8 +779,7 @@ getLldpadPid(void) {
_("Error opening file %s"), LLDPAD_PID_FILE);
}
- if (fd >= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return pid;
}
Index: libvirt-acl/src/util/pci.c
===================================================================
--- libvirt-acl.orig/src/util/pci.c
+++ libvirt-acl/src/util/pci.c
@@ -37,6 +37,7 @@
#include "memory.h"
#include "util.h"
#include "virterror_internal.h"
+#include "files.h"
/* avoid compilation breakage on some systems */
#ifndef MODPROBE
@@ -188,10 +189,7 @@ pciCloseConfig(pciDevice *dev)
if (!dev)
return;
- if (dev->fd >= 0) {
- close(dev->fd);
- dev->fd = -1;
- }
+ VIR_FORCE_CLOSE(dev->fd);
}
static int
Index: libvirt-acl/src/util/storage_file.c
===================================================================
--- libvirt-acl.orig/src/util/storage_file.c
+++ libvirt-acl/src/util/storage_file.c
@@ -36,6 +36,7 @@
#include "memory.h"
#include "virterror_internal.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -688,7 +689,7 @@ virStorageFileProbeFormat(const char *pa
ret = virStorageFileProbeFormatFromFD(path, fd);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -782,7 +783,7 @@ virStorageFileGetMetadata(const char *pa
ret = virStorageFileGetMetadataFromFD(path, fd, format, meta);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
Index: libvirt-acl/src/util/util.c
===================================================================
--- libvirt-acl.orig/src/util/util.c
+++ libvirt-acl/src/util/util.c
@@ -71,6 +71,7 @@
#include "memory.h"
#include "threads.h"
#include "verify.h"
+#include "files.h"
#ifndef NSIG
# define NSIG 32
@@ -461,6 +462,7 @@ __virExec(const char *const*argv,
int pipeerr[2] = {-1,-1};
int childout = -1;
int childerr = -1;
+ int tmpfd;
if ((null = open("/dev/null", O_RDONLY)) < 0) {
virReportSystemError(errno,
@@ -534,13 +536,13 @@ __virExec(const char *const*argv,
}
if (pid) { /* parent */
- close(null);
+ VIR_FORCE_CLOSE(null);
if (outfd && *outfd == -1) {
- close(pipeout[1]);
+ VIR_FORCE_CLOSE(pipeout[1]);
*outfd = pipeout[0];
}
if (errfd && *errfd == -1) {
- close(pipeerr[1]);
+ VIR_FORCE_CLOSE(pipeerr[1]);
*errfd = pipeerr[0];
}
@@ -568,8 +570,10 @@ __virExec(const char *const*argv,
i != childout &&
i != childerr &&
(!keepfd ||
- !FD_ISSET(i, keepfd)))
- close(i);
+ !FD_ISSET(i, keepfd))) {
+ tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
+ }
if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) {
virReportSystemError(errno,
@@ -589,14 +593,15 @@ __virExec(const char *const*argv,
goto fork_error;
}
- if (infd > 0)
- close(infd);
- close(null);
- if (childout > 0)
- close(childout);
+ VIR_FORCE_CLOSE(infd);
+ VIR_FORCE_CLOSE(null);
+ tmpfd = childout; /* preserve childout value */
+ VIR_FORCE_CLOSE(tmpfd);
if (childerr > 0 &&
- childerr != childout)
- close(childerr);
+ childerr != childout) {
+ VIR_FORCE_CLOSE(childerr);
+ childout = -1;
+ }
/* Daemonize as late as possible, so the parent process can detect
* the above errors with wait* */
@@ -666,16 +671,11 @@ __virExec(const char *const*argv,
/* NB we don't virUtilError() on any failures here
because the code which jumped hre already raised
an error condition which we must not overwrite */
- if (pipeerr[0] > 0)
- close(pipeerr[0]);
- if (pipeerr[1] > 0)
- close(pipeerr[1]);
- if (pipeout[0] > 0)
- close(pipeout[0]);
- if (pipeout[1] > 0)
- close(pipeout[1]);
- if (null > 0)
- close(null);
+ VIR_FORCE_CLOSE(pipeerr[0]);
+ VIR_FORCE_CLOSE(pipeerr[1]);
+ VIR_FORCE_CLOSE(pipeout[0]);
+ VIR_FORCE_CLOSE(pipeout[1]);
+ VIR_FORCE_CLOSE(null);
return -1;
}
@@ -865,10 +865,8 @@ virRunWithHook(const char *const*argv,
VIR_FREE(outbuf);
VIR_FREE(errbuf);
VIR_FREE(argv_str);
- if (outfd != -1)
- close(outfd);
- if (errfd != -1)
- close(errfd);
+ VIR_FORCE_CLOSE(outfd);
+ VIR_FORCE_CLOSE(errfd);
return ret;
}
@@ -1099,7 +1097,7 @@ int virFileReadAll(const char *path, int
}
int len = virFileReadLimFD(fd, maxlen, buf);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (len < 0) {
virReportSystemError(errno, _("Failed to read file '%s'"),
path);
return -1;
@@ -1305,7 +1303,7 @@ static int virFileOperationNoFork(const
if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
goto error;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
ret = -errno;
virReportSystemError(errno, _("failed to close new file
'%s'"),
path);
@@ -1314,8 +1312,7 @@ static int virFileOperationNoFork(const
}
fd = -1;
error:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -1466,7 +1463,7 @@ parenterror:
if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
goto childerror;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
ret = -errno;
virReportSystemError(errno, _("child failed to close new file
'%s'"),
path);
@@ -1743,10 +1740,8 @@ int virFileOpenTtyAt(const char *ptmx,
rc = 0;
cleanup:
- if (rc != 0 &&
- *ttymaster != -1) {
- close(*ttymaster);
- }
+ if (rc != 0)
+ VIR_FORCE_CLOSE(*ttymaster);
return rc;
@@ -1812,7 +1807,7 @@ int virFileWritePidPath(const char *pidf
if (!(file = fdopen(fd, "w"))) {
rc = errno;
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
Index: libvirt-acl/src/util/uuid.c
===================================================================
--- libvirt-acl.orig/src/util/uuid.c
+++ libvirt-acl/src/util/uuid.c
@@ -39,6 +39,7 @@
#include "virterror_internal.h"
#include "logging.h"
#include "memory.h"
+#include "files.h"
#ifndef ENODATA
# define ENODATA EIO
@@ -61,7 +62,7 @@ virUUIDGenerateRandomBytes(unsigned char
if ((n = read(fd, buf, buflen)) <= 0) {
if (errno == EINTR)
continue;
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return n < 0 ? errno : ENODATA;
}
@@ -69,7 +70,7 @@ virUUIDGenerateRandomBytes(unsigned char
buflen -= n;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
}
@@ -240,10 +241,10 @@ getDMISystemUUID(char *uuid, int len)
int fd = open(paths[i], O_RDONLY);
if (fd > 0) {
if (saferead(fd, uuid, len) == len) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
}
i++;
}
Index: libvirt-acl/src/util/virtaudit.c
===================================================================
--- libvirt-acl.orig/src/util/virtaudit.c
+++ libvirt-acl/src/util/virtaudit.c
@@ -30,6 +30,7 @@
#include "virterror_internal.h"
#include "logging.h"
#include "virtaudit.h"
+#include "files.h"
/* Provide the macros in case the header file is old.
FIXME: should be removed. */
@@ -133,6 +134,6 @@ void virAuditSend(const char *file ATTRI
void virAuditClose(void)
{
#if HAVE_AUDIT
- close(auditfd);
+ VIR_CLOSE(auditfd);
#endif
}
Index: libvirt-acl/src/xen/proxy_internal.c
===================================================================
--- libvirt-acl.orig/src/xen/proxy_internal.c
+++ libvirt-acl/src/xen/proxy_internal.c
@@ -30,6 +30,7 @@
#include "util.h"
#include "xen_driver.h"
#include "memory.h"
+#include "files.h"
#define STANDALONE
@@ -196,7 +197,7 @@ retry:
addr.sun_family = AF_UNIX;
addr.sun_path[0] = '\0';
if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -204,7 +205,7 @@ retry:
* now bind the socket to that address and listen on it
*/
if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
if (trials < 3) {
if (virProxyForkServer() < 0)
return(-1);
@@ -236,12 +237,11 @@ virProxyCloseSocket(xenUnifiedPrivatePtr
if (priv->proxy < 0)
return(-1);
- ret = close(priv->proxy);
+ ret = VIR_CLOSE(priv->proxy);
if (ret != 0)
VIR_WARN("Failed to close socket %d", priv->proxy);
else
VIR_DEBUG("Closed socket %d", priv->proxy);
- priv->proxy = -1;
return(ret);
}
Index: libvirt-acl/src/xen/xen_hypervisor.c
===================================================================
--- libvirt-acl.orig/src/xen/xen_hypervisor.c
+++ libvirt-acl/src/xen/xen_hypervisor.c
@@ -65,6 +65,7 @@
#include "buf.h"
#include "capabilities.h"
#include "memory.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_XEN
@@ -2036,7 +2037,7 @@ xenHypervisorInit(void)
hypervisor_version = -1;
virXenError(VIR_ERR_XEN_CALL, " ioctl %lu",
(unsigned long) IOCTL_PRIVCMD_HYPERCALL);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
in_init = 0;
return(-1);
@@ -2122,13 +2123,13 @@ xenHypervisorInit(void)
hypervisor_version = -1;
virXenError(VIR_ERR_XEN_CALL, " ioctl %lu",
(unsigned long)IOCTL_PRIVCMD_HYPERCALL);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
in_init = 0;
VIR_FREE(ipt);
return(-1);
done:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
in_init = 0;
VIR_FREE(ipt);
return(0);
@@ -2191,7 +2192,7 @@ xenHypervisorClose(virConnectPtr conn)
if (priv->handle < 0)
return -1;
- ret = close(priv->handle);
+ ret = VIR_CLOSE(priv->handle);
if (ret < 0)
return (-1);
@@ -2396,8 +2397,7 @@ get_cpu_flags(virConnectPtr conn, const
ret = 1;
out:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return ret;
}
Index: libvirt-acl/src/xen/xen_inotify.c
===================================================================
--- libvirt-acl.orig/src/xen/xen_inotify.c
+++ libvirt-acl/src/xen/xen_inotify.c
@@ -39,6 +39,7 @@
#include "xend_internal.h"
#include "logging.h"
#include "uuid.h"
+#include "files.h"
#include "xm_internal.h" /* for xenXMDomainConfigParse */
@@ -483,7 +484,7 @@ xenInotifyClose(virConnectPtr conn)
if (priv->inotifyWatch != -1)
virEventRemoveHandle(priv->inotifyWatch);
- close(priv->inotifyFD);
+ VIR_FORCE_CLOSE(priv->inotifyFD);
return 0;
}
Index: libvirt-acl/src/xen/xend_internal.c
===================================================================
--- libvirt-acl.orig/src/xen/xend_internal.c
+++ libvirt-acl/src/xen/xend_internal.c
@@ -45,6 +45,7 @@
#include "xs_internal.h" /* To extract VNC port & Serial console TTY */
#include "memory.h"
#include "count-one-bits.h"
+#include "files.h"
/* required for cpumap_t */
#include <xen/dom0_ops.h>
@@ -118,7 +119,6 @@ static int
do_connect(virConnectPtr xend)
{
int s;
- int serrno;
int no_slow_start = 1;
xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) xend->privateData;
@@ -137,9 +137,7 @@ do_connect(virConnectPtr xend)
if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) {
- serrno = errno;
- close(s);
- errno = serrno;
+ VIR_FORCE_CLOSE(s);
s = -1;
/*
@@ -387,7 +385,7 @@ xend_get(virConnectPtr xend, const char
"Content-Type: application/x-www-form-urlencoded\r\n"
"\r\n");
ret = xend_req(s, content);
- close(s);
+ VIR_FORCE_CLOSE(s);
if (((ret < 0) || (ret >= 300)) &&
((ret != 404) || (!STRPREFIX(path, "/xend/domain/")))) {
@@ -437,7 +435,7 @@ xend_post(virConnectPtr xend, const char
swrites(s, ops);
ret = xend_req(s, &err_buf);
- close(s);
+ VIR_FORCE_CLOSE(s);
if ((ret < 0) || (ret >= 300)) {
virXendError(VIR_ERR_POST_FAILED,
@@ -843,7 +841,7 @@ xenDaemonOpen_tcp(virConnectPtr conn, co
memcpy(&priv->addr,
r->ai_addr,
r->ai_addrlen);
- close(sock);
+ VIR_FORCE_CLOSE(sock);
break;
}
Index: libvirt-acl/daemon/libvirtd.c
===================================================================
--- libvirt-acl.orig/daemon/libvirtd.c
+++ libvirt-acl/daemon/libvirtd.c
@@ -50,6 +50,7 @@
#include "libvirt_internal.h"
#include "virterror_internal.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -425,7 +426,7 @@ static int daemonForkIntoBackground(void
int stdoutfd = -1;
int nextpid;
- close(statuspipe[0]);
+ VIR_FORCE_CLOSE(statuspipe[0]);
if ((stdinfd = open("/dev/null", O_RDONLY)) < 0)
goto cleanup;
@@ -437,12 +438,10 @@ static int daemonForkIntoBackground(void
goto cleanup;
if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO)
goto cleanup;
- if (close(stdinfd) < 0)
+ if (VIR_CLOSE(stdinfd) < 0)
goto cleanup;
- stdinfd = -1;
- if (close(stdoutfd) < 0)
+ if (VIR_CLOSE(stdoutfd) < 0)
goto cleanup;
- stdoutfd = -1;
if (setsid() < 0)
goto cleanup;
@@ -458,10 +457,8 @@ static int daemonForkIntoBackground(void
}
cleanup:
- if (stdoutfd != -1)
- close(stdoutfd);
- if (stdinfd != -1)
- close(stdinfd);
+ VIR_FORCE_CLOSE(stdoutfd);
+ VIR_FORCE_CLOSE(stdinfd);
return -1;
}
@@ -475,7 +472,7 @@ static int daemonForkIntoBackground(void
int ret;
char status;
- close(statuspipe[1]);
+ VIR_FORCE_CLOSE(statuspipe[1]);
/* We wait to make sure the first child forked successfully */
if ((got = waitpid(pid, &exitstatus, 0)) < 0 ||
@@ -518,7 +515,7 @@ static int qemudWritePidFile(const char
if (!(fh = fdopen(fd, "w"))) {
VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"),
pidFile, virStrerror(errno, ebuf, sizeof ebuf));
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -607,8 +604,7 @@ static int qemudListenUnix(struct qemud_
return 0;
cleanup:
- if (sock->fd >= 0)
- close(sock->fd);
+ VIR_FORCE_CLOSE(sock->fd);
VIR_FREE(sock);
return -1;
}
@@ -745,7 +741,7 @@ remoteListenTCP (struct qemud_server *se
cleanup:
for (i = 0; i < nfds; ++i)
- close(fds[i]);
+ VIR_FORCE_CLOSE(fds[i]);
return -1;
}
@@ -1518,10 +1514,7 @@ void qemudDispatchClientFailure(struct q
gnutls_deinit (client->tlssession);
client->tlssession = NULL;
}
- if (client->fd != -1) {
- close(client->fd);
- client->fd = -1;
- }
+ VIR_FORCE_CLOSE(client->fd);
}
@@ -2421,17 +2414,15 @@ static int qemudStartEventLoop(struct qe
static void qemudCleanup(struct qemud_server *server) {
struct qemud_socket *sock;
- if (server->sigread != -1)
- close(server->sigread);
- if (server->sigwrite != -1)
- close(server->sigwrite);
+ VIR_FORCE_CLOSE(server->sigread);
+ VIR_FORCE_CLOSE(server->sigwrite);
sock = server->sockets;
while (sock) {
struct qemud_socket *next = sock->next;
if (sock->watch)
virEventRemoveHandleImpl(sock->watch);
- close(sock->fd);
+ VIR_FORCE_CLOSE(sock->fd);
/* Unlink unix domain sockets which are not in
* the abstract namespace */
@@ -2986,8 +2977,8 @@ daemonSetupSignals(struct qemud_server *
return 0;
error:
- close(sigpipe[0]);
- close(sigpipe[1]);
+ VIR_FORCE_CLOSE(sigpipe[0]);
+ VIR_FORCE_CLOSE(sigpipe[1]);
return -1;
}
@@ -3244,8 +3235,7 @@ int main(int argc, char **argv) {
while (write(statuswrite, &status, 1) == -1 &&
errno == EINTR)
;
- close(statuswrite);
- statuswrite = -1;
+ VIR_FORCE_CLOSE(statuswrite);
}
/* Start the event loop in a background thread, since
@@ -3302,7 +3292,7 @@ error:
errno == EINTR)
;
}
- close(statuswrite);
+ VIR_FORCE_CLOSE(statuswrite);
}
if (server)
qemudCleanup(server);
Index: libvirt-acl/src/util/hooks.c
===================================================================
--- libvirt-acl.orig/src/util/hooks.c
+++ libvirt-acl/src/util/hooks.c
@@ -36,6 +36,7 @@
#include "conf/domain_conf.h"
#include "logging.h"
#include "memory.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_HOOK
@@ -368,7 +369,7 @@ virHookCall(int driver, const char *id,
}
ret = virExec(argv, env, NULL, &pid, pipefd[0], &outfd, &errfd,
VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
- if (close(pipefd[1]) < 0) {
+ if (VIR_CLOSE(pipefd[1]) < 0) {
virReportSystemError(errno, "%s",
_("unable to close pipe for hook input"));
}
@@ -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"));
}
if (argv) {
for (i = 0 ; i < argc ; i++)
Index: libvirt-acl/tests/testutils.c
===================================================================
--- libvirt-acl.orig/tests/testutils.c
+++ libvirt-acl/tests/testutils.c
@@ -47,6 +47,8 @@
((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \
((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0)
+#include "files.h"
+
static unsigned int testDebug = -1;
static unsigned int testVerbose = -1;
@@ -205,7 +207,7 @@ int virtTestLoadFile(const char *file,
static
void virtTestCaptureProgramExecChild(const char *const argv[],
int pipefd) {
- int i;
+ int i, tmpfd;
int open_max;
int stdinfd = -1;
const char *const env[] = {
@@ -222,8 +224,10 @@ void virtTestCaptureProgramExecChild(con
open_max = sysconf (_SC_OPEN_MAX);
for (i = 0; i < open_max; i++) {
if (i != stdinfd &&
- i != pipefd)
- close(i);
+ i != pipefd) {
+ tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
+ }
}
if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO)
@@ -237,8 +241,7 @@ void virtTestCaptureProgramExecChild(con
execve(argv[0], (char *const*)argv, (char *const*)env);
cleanup:
- if (stdinfd != -1)
- close(stdinfd);
+ VIR_FORCE_CLOSE(stdinfd);
}
int virtTestCaptureProgramOutput(const char *const argv[],
@@ -252,10 +255,10 @@ int virtTestCaptureProgramOutput(const c
int pid = fork();
switch (pid) {
case 0:
- close(pipefd[0]);
+ VIR_FORCE_CLOSE(pipefd[0]);
virtTestCaptureProgramExecChild(argv, pipefd[1]);
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(pipefd[1]);
_exit(1);
case -1:
@@ -267,7 +270,7 @@ int virtTestCaptureProgramOutput(const c
int ret = -1;
int want = buflen-1;
- close(pipefd[1]);
+ VIR_FORCE_CLOSE(pipefd[1]);
while (want) {
if ((ret = read(pipefd[0], (*buf)+got, want)) <= 0)
@@ -275,7 +278,7 @@ int virtTestCaptureProgramOutput(const c
got += ret;
want -= ret;
}
- close(pipefd[0]);
+ VIR_FORCE_CLOSE(pipefd[0]);
if (!ret)
(*buf)[got] = '\0';
Index: libvirt-acl/proxy/libvirt_proxy.c
===================================================================
--- libvirt-acl.orig/proxy/libvirt_proxy.c
+++ libvirt-acl/proxy/libvirt_proxy.c
@@ -31,6 +31,7 @@
# include "xend_internal.h"
# include "xs_internal.h"
# include "xen_driver.h"
+# include "files.h"
static int fdServer = -1;
static int debug = 0;
@@ -133,10 +134,9 @@ proxyCloseUnixSocket(void) {
if (fdServer < 0)
return(0);
- ret = close(fdServer);
if (debug > 0)
fprintf(stderr, "closing unix socket %d: %d\n", fdServer, ret);
- fdServer = -1;
+ ret = VIR_CLOSE(fdServer);
pollInfos[0].fd = -1;
return(ret);
}
@@ -172,7 +172,7 @@ proxyListenUnixSocket(const char *path)
addr.sun_path[0] = '\0';
if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) {
fprintf(stderr, "Path %s too long to fit into destination\n", path);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -181,12 +181,12 @@ proxyListenUnixSocket(const char *path)
*/
if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
fprintf(stderr, "Failed to bind to socket %s\n", path);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return (-1);
}
if (listen(fd, 30 /* backlog */ ) < 0) {
fprintf(stderr, "Failed to listen to socket %s\n", path);
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return (-1);
}
@@ -230,7 +230,7 @@ retry:
if (nbClients >= MAX_CLIENT) {
fprintf(stderr, "Too many client registered\n");
- close(client);
+ VIR_FORCE_CLOSE(client);
return(-1);
}
nbClients++;
@@ -260,7 +260,7 @@ static int
proxyCloseClientSocket(int nr) {
int ret;
- ret = close(pollInfos[nr].fd);
+ ret = VIR_CLOSE(pollInfos[nr].fd);
if (ret != 0)
fprintf(stderr, "Failed to close socket %d from client %d\n",
pollInfos[nr].fd, nr);
@@ -285,7 +285,7 @@ proxyCloseClientSockets(void) {
int i, ret;
for (i = 1;i <= nbClients;i++) {
- ret = close(pollInfos[i].fd);
+ ret = VIR_CLOSE(pollInfos[i].fd);
if (ret != 0)
fprintf(stderr, "Failed to close socket %d from client %d\n",
pollInfos[i].fd, i);
Index: libvirt-acl/tools/console.c
===================================================================
--- libvirt-acl.orig/tools/console.c
+++ libvirt-acl/tools/console.c
@@ -39,6 +39,7 @@
# include "internal.h"
# include "logging.h"
# include "util.h"
+# include "files.h"
/* ie Ctrl-] as per telnet */
# define CTRL_CLOSE_BRACKET '\35'
@@ -192,7 +193,7 @@ int vshRunConsole(const char *tty) {
tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr);
closetty:
- close(ttyfd);
+ VIR_FORCE_CLOSE(ttyfd);
return ret;
}
11:33 a.m.
On 10/22/2010 05:19 AM, Stefan Berger wrote:
Using automated replacement with sed and editing I have now replaced
all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
Pre-review comment:
One more thing to do, which is adding something like this to cfg.mk:
sc_avoid_close:
@prohibit='\<close *\(' \
in_vc_files='\.c$$' \
halt='use VIR_(FORCE_)CLOSE instead of close' \
$(_sc_search_regexp)
along with adding .x-sc_avoid_close with contents of src/util/file.c to
exempt the one valid use of close().
Would you mind testing 'make syntax-check' after folding that in, and
addressing any other fallout that it detects, while I proceed with the
rest of my review?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:54 a.m.
libvir-list-bounces(a)redhat.com wrote on 10/22/2010 12:33:48 PM:
On 10/22/2010 05:19 AM, Stefan Berger wrote:
> Using automated replacement with sed and editing I have now replaced
all
> occurrences of close() with VIR_(FORCE_)CLOSE() except for one,
of
> course. Some replacements were straight forward, others I needed to
pay
> attention. I hope I payed attention in all the right places...
Please
> have a look. This should have at least solved one more double-close
> error.
>
> Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
Pre-review comment:
One more thing to do, which is adding something like this to cfg.mk:
sc_avoid_close:
@prohibit='\<close *\(' \
in_vc_files='\.c$$' \
halt='use VIR_(FORCE_)CLOSE instead of close' \
$(_sc_search_regexp)
along with adding .x-sc_avoid_close with contents of src/util/file.c to
exempt the one valid use of close().
Would you mind testing 'make syntax-check' after folding that in, and
addressing any other fallout that it detects, while I proceed with the
rest of my review?
Yes, I can do that... runs fine.
The patch will need to be rebased to tip...
Stefan
12:35 p.m.
On 10/22/2010 05:19 AM, Stefan Berger wrote:
Using automated replacement with sed and editing I have now replaced
all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
Index: libvirt-acl/src/libvirt.c
===================================================================
--- libvirt-acl.orig/src/libvirt.c
+++ libvirt-acl/src/libvirt.c
@@ -10794,7 +10794,7 @@ virStreamRef(virStreamPtr stream)
* ... report an error ....
* done:
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
Ignoring close() failure on read seems reasonable...
*
* Returns the number of bytes written, which may be less
* than requested.
@@ -10884,7 +10884,7 @@ error:
* ... report an error ....
* done:
* virStreamFree(st);
- * close(fd);
+ * VIR_FORCE_CLOSE(fd);
but on write, we should instead change the recommendation to check for
close() failure, since some file systems (yes, I'm looking at you, NFS)
can successfully write() to kernel buffers but still fail to close()
when actual network traffic is finally triggered.
* int fd = open("demo.iso", O_WRONLY, 0600)
*
...
* if (virStreamFinish(st) < 0 || VIR_CLOSE(fd) < 0)
* ... report an error ....
* done:
* virStreamFree(st);
* VIR_FORCE_CLOSE(fd);
I'm wondering how much of the rest of your patch might be impacted by
adopting this idiom.
Also, should we be thinking of a separate patch for VIR_FCLOSE for the
FILE* variant?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:24 p.m.
On 10/22/2010 05:19 AM, Stefan Berger wrote:
Using automated replacement with sed and editing I have now replaced
all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
---
daemon/libvirtd.c | 46 ++++++---------
Continuing on (looks like I'll be replying quite a few times today)...
@@ -127,7 +128,7 @@ static int lxcContainerExecInit(virDomai
static int lxcContainerSetStdio(int control, int ttyfd)
{
int rc = -1;
- int open_max, i;
+ int open_max, i, tpmfd;
if (setsid()< 0) {
virReportSystemError(errno, "%s",
@@ -145,8 +146,10 @@ static int lxcContainerSetStdio(int cont
* close all FDs before executing the container */
open_max = sysconf (_SC_OPEN_MAX);
for (i = 0; i< open_max; i++)
- if (i != ttyfd&& i != control)
- close(i);
+ if (i != ttyfd&& i != control) {
+ tpmfd = i;
+ VIR_FORCE_CLOSE(tpmfd);
Yeah, I guess you do have to introduce a temporary rather than
clobbering your iterator.
s/tpmfd/tmpfd/, and perhaps reduce it's scope to just the for loop or if
statement where it is needed.
Index: libvirt-acl/src/lxc/lxc_driver.c
===================================================================
--- libvirt-acl.orig/src/lxc/lxc_driver.c
+++ libvirt-acl/src/lxc/lxc_driver.c
@@ -1544,14 +1544,10 @@ 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);
+ VIR_FORCE_CLOSE(logfd);
logfd might be one where we want to hoist a normal VIR_CLOSE and check
for errors.
Index: libvirt-acl/src/phyp/phyp_driver.c
Pausing here for my lunch...
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:48 p.m.
libvir-list-bounces(a)redhat.com wrote on 10/22/2010 02:24:38 PM:
On 10/22/2010 05:19 AM, Stefan Berger wrote:
> Using automated replacement with sed and editing I have now replaced
all
> occurrences of close() with VIR_(FORCE_)CLOSE() except for one,
of
> course. Some replacements were straight forward, others I needed to
pay
> attention. I hope I payed attention in all the right places...
Please
> have a look. This should have at least solved one more double-close
> error.
>
> Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
>
> ---
> daemon/libvirtd.c | 46 ++++++---------
Continuing on (looks like I'll be replying quite a few times today)...
> @@ -127,7 +128,7 @@ static int lxcContainerExecInit(virDomai
> static int lxcContainerSetStdio(int control, int ttyfd)
> {
> int rc = -1;
> - int open_max, i;
> + int open_max, i, tpmfd;
>
> if (setsid()< 0) {
> virReportSystemError(errno, "%s",
> @@ -145,8 +146,10 @@ static int lxcContainerSetStdio(int cont
> * close all FDs before executing the container */
> open_max = sysconf (_SC_OPEN_MAX);
> for (i = 0; i< open_max; i++)
> - if (i != ttyfd&& i != control)
> - close(i);
> + if (i != ttyfd&& i != control) {
> + tpmfd = i;
> + VIR_FORCE_CLOSE(tpmfd);
Yeah, I guess you do have to introduce a temporary rather than
clobbering your iterator.
s/tpmfd/tmpfd/, and perhaps reduce it's scope to just the for loop or if
statement where it is needed.
ha, what a typo... making it a local variable.
> - close(logfd);
> + if (rc != 0)
> + VIR_FORCE_CLOSE(priv->monitor);
> + VIR_FORCE_CLOSE(parentTty);
> + VIR_FORCE_CLOSE(logfd);
logfd might be one where we want to hoist a normal VIR_CLOSE and check
for errors.
Ok, so I call the virReportSystemError() on this now.
Stefan
5:27 p.m.
On 10/22/2010 05:19 AM, Stefan Berger wrote:
Using automated replacement with sed and editing I have now replaced
all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
Can you isolate any of those double-close errors into separate patches
which we can apply now, rather than drowning them in the giant patch?
Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
src/phyp/phyp_driver.c | 13 ++--
Resuming...
Most of the changes are looking okay in limited context.
The point was raised on IRC that this patch is big enough that we
probably ought to delay until post 0.8.5 to apply it, so as to maximize
testing exposure over the full course of a release cycle rather than
making this next week be all the testing we give. Exceptions for true
double-close bug fixes which can be applied as independent patches now.
Index: libvirt-acl/src/qemu/qemu_driver.c
@@ -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"));
+ }
Yep, this looks like a good case to convert over to reporting an error.
Index: libvirt-acl/src/qemu/qemu_monitor.c
@@ -694,8 +695,7 @@ void qemuMonitorClose(qemuMonitorPtr mon
if (!mon->closed) {
if (mon->watch)
virEventRemoveHandle(mon->watch);
- if (mon->fd != -1)
- close(mon->fd);
+ VIR_FORCE_CLOSE(mon->fd);
/* NB: ordinarily one might immediately set mon->watch to -1
* and mon->fd to -1, but there may be a callback active
* that is still relying on these fields being valid. So
Ouch - given that comment, could we be frying a callback by setting
mon->fd to -1 in VIR_FORCE_CLOSE? We need to double check this, and
possibly use a temporary variable if the callback indeed needs a
non-negative mon->fd for a bit longer, or tighten the specification and
all existing callbacks to tolerate mon->fd changing to -1.
Probably worth splitting this particular hunk into its own commit,
rather than part of the giant patch.
Index: libvirt-acl/src/remote/remote_driver.c
===================================================================
--- libvirt-acl.orig/src/remote/remote_driver.c
+++ libvirt-acl/src/remote/remote_driver.c
@@ -82,6 +82,7 @@
#include "util.h"
#include "event.h"
#include "ignore-value.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_REMOTE
@@ -711,7 +712,7 @@ doRemoteOpen (virConnectPtr conn,
if (errno == ECONNREFUSED&&
flags& VIR_DRV_OPEN_REMOTE_AUTOSTART&&
trials< 20) {
- close(priv->sock);
+ VIR_FORCE_CLOSE(priv->sock);
priv->sock = -1;
This line is now redundant.
Index: libvirt-acl/src/uml/uml_driver.c
@@ -811,7 +811,7 @@ static int umlStartVMDaemon(virConnectPt
virDomainObjPtr vm) {
const char **argv = NULL, **tmp;
const char **progenv = NULL;
- int i, ret;
+ int i, ret, tmpfd;
pid_t pid;
char *logfile;
int logfd = -1;
for (i = 0; i< FD_SETSIZE; i++)
- if (FD_ISSET(i,&keepfd))
- close(i);
+ if (FD_ISSET(i,&keepfd)) {
+ tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
Another awfully large scope for a needed temporary.
Index: libvirt-acl/src/util/bridge.c
@@ -107,7 +108,7 @@ brShutdown(brControl *ctl)
if (!ctl)
return;
- close(ctl->fd);
+ VIR_FORCE_CLOSE(ctl->fd);
ctl->fd = 0;
Huh - is this an existing logic bug? Can we end up accidentally
double-closing stdin?
Index: libvirt-acl/src/util/logging.c
===================================================================
--- libvirt-acl.orig/src/util/logging.c
+++ libvirt-acl/src/util/logging.c
@@ -40,6 +40,7 @@
#include "util.h"
#include "buf.h"
#include "threads.h"
+#include "files.h"
/*
* Macro used to format the message as a string in virLogMessage
@@ -603,8 +604,7 @@ static int virLogOutputToFd(const char *
static void virLogCloseFd(void *data) {
int fd = (long) data;
- if (fd>= 0)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
Should we fix this function to return an int value, and return
VIR_CLOSE(fd) so that callers can choose to detect log close failures?
Index: libvirt-acl/src/util/macvtap.c
===================================================================
--- libvirt-acl.orig/src/util/macvtap.c
+++ libvirt-acl/src/util/macvtap.c
@@ -52,6 +52,7 @@
# include "conf/domain_conf.h"
# include "virterror_internal.h"
# include "uuid.h"
+# include "files.h"
# define VIR_FROM_THIS VIR_FROM_NET
@@ -94,7 +95,7 @@ static int nlOpen(void)
static void nlClose(int fd)
{
- close(fd);
+ VIR_FORCE_CLOSE(fd);
Likewise?
Index: libvirt-acl/src/util/util.c
===================================================================
--- libvirt-acl.orig/src/util/util.c
+++ libvirt-acl/src/util/util.c
@@ -71,6 +71,7 @@
#include "memory.h"
#include "threads.h"
#include "verify.h"
+#include "files.h"
#ifndef NSIG
# define NSIG 32
@@ -461,6 +462,7 @@ __virExec(const char *const*argv,
int pipeerr[2] = {-1,-1};
int childout = -1;
int childerr = -1;
+ int tmpfd;
@@ -568,8 +570,10 @@ __virExec(const char *const*argv,
i != childout&&
i != childerr&&
(!keepfd ||
- !FD_ISSET(i, keepfd)))
- close(i);
+ !FD_ISSET(i, keepfd))) {
+ tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
I thought this was another large scope for a temporary....
+ }
if (dup2(infd>= 0 ? infd : null, STDIN_FILENO)< 0) {
virReportSystemError(errno,
@@ -589,14 +593,15 @@ __virExec(const char *const*argv,
goto fork_error;
}
- if (infd> 0)
- close(infd);
- close(null);
- if (childout> 0)
- close(childout);
+ VIR_FORCE_CLOSE(infd);
+ VIR_FORCE_CLOSE(null);
+ tmpfd = childout; /* preserve childout value */
+ VIR_FORCE_CLOSE(tmpfd);
...but you needed it here too.
if (childerr> 0&&
- childerr != childout)
- close(childerr);
+ childerr != childout) {
+ VIR_FORCE_CLOSE(childerr);
+ childout = -1;
+ }
Looks okay after all - certainly not one of the trivial conversions
though :)
Index: libvirt-acl/src/util/virtaudit.c
===================================================================
--- libvirt-acl.orig/src/util/virtaudit.c
+++ libvirt-acl/src/util/virtaudit.c
@@ -30,6 +30,7 @@
#include "virterror_internal.h"
#include "logging.h"
#include "virtaudit.h"
+#include "files.h"
/* Provide the macros in case the header file is old.
FIXME: should be removed. */
@@ -133,6 +134,6 @@ void virAuditSend(const char *file ATTRI
void virAuditClose(void)
{
#if HAVE_AUDIT
- close(auditfd);
+ VIR_CLOSE(auditfd);
Must be VIR_FORCE_CLOSE; I'm guessing you didn't have audit turned on in
your compile.
Index: libvirt-acl/src/xen/proxy_internal.c
@@ -236,12 +237,11 @@ virProxyCloseSocket(xenUnifiedPrivatePtr
if (priv->proxy< 0)
return(-1);
- ret = close(priv->proxy);
+ ret = VIR_CLOSE(priv->proxy);
if (ret != 0)
VIR_WARN("Failed to close socket %d", priv->proxy);
else
VIR_DEBUG("Closed socket %d", priv->proxy);
Subtle unintended semantic change; you're now printing -1 instead of the
fd you just closed; you'll need a temporary.
Index: libvirt-acl/src/xen/xend_internal.c
@@ -137,9 +137,7 @@ do_connect(virConnectPtr xend)
if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) {
- serrno = errno;
- close(s);
- errno = serrno;
+ VIR_FORCE_CLOSE(s);
s = -1;
Redundant line. Overall, I'm impressed with how many lines this is
shaving off the code base! I think we settled on a pretty good calling
convention.
And with that, I've completed my review of v1.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:21 p.m.
libvir-list-bounces(a)redhat.com wrote on 10/22/2010 06:27:30 PM:
libvir-list
On 10/22/2010 05:19 AM, Stefan Berger wrote:
> Using automated replacement with sed and editing I have now replaced
all
> occurrences of close() with VIR_(FORCE_)CLOSE() except for one,
of
> course. Some replacements were straight forward, others I needed to
pay
> attention. I hope I payed attention in all the right places...
Please
> have a look. This should have at least solved one more double-close
> error.
Can you isolate any of those double-close errors into separate patches
which we can apply now, rather than drowning them in the giant patch?
>
> Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com>
>
> src/phyp/phyp_driver.c | 13 ++--
Resuming...
[...]
> Index: libvirt-acl/src/qemu/qemu_monitor.c
>
> @@ -694,8 +695,7 @@ void qemuMonitorClose(qemuMonitorPtr mon
> if (!mon->closed) {
> if (mon->watch)
> virEventRemoveHandle(mon->watch);
> - if (mon->fd != -1)
> - close(mon->fd);
> + VIR_FORCE_CLOSE(mon->fd);
> /* NB: ordinarily one might immediately set mon->watch to -1
> * and mon->fd to -1, but there may be a callback active
> * that is still relying on these fields being valid. So
Ouch - given that comment, could we be frying a callback by setting
mon->fd to -1 in VIR_FORCE_CLOSE? We need to double check this, and
possibly use a temporary variable if the callback indeed needs a
non-negative mon->fd for a bit longer, or tighten the specification and
all existing callbacks to tolerate mon->fd changing to -1.
Probably worth splitting this particular hunk into its own commit,
rather than part of the giant patch.
Yes, good idea. Obviously I did not read the comment.
> Index: libvirt-acl/src/remote/remote_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/remote/remote_driver.c
> +++ libvirt-acl/src/remote/remote_driver.c
> @@ -82,6 +82,7 @@
> #include "util.h"
> #include "event.h"
> #include "ignore-value.h"
> +#include "files.h"
>
> #define VIR_FROM_THIS VIR_FROM_REMOTE
>
> @@ -711,7 +712,7 @@ doRemoteOpen (virConnectPtr conn,
> if (errno == ECONNREFUSED&&
> flags& VIR_DRV_OPEN_REMOTE_AUTOSTART&&
> trials< 20) {
> - close(priv->sock);
> + VIR_FORCE_CLOSE(priv->sock);
> priv->sock = -1;
This line is now redundant.
Done.
> Index: libvirt-acl/src/uml/uml_driver.c
> @@ -811,7 +811,7 @@ static int umlStartVMDaemon(virConnectPt
> virDomainObjPtr vm) {
> const char **argv = NULL, **tmp;
> const char **progenv = NULL;
> - int i, ret;
> + int i, ret, tmpfd;
> pid_t pid;
> char *logfile;
> int logfd = -1;
> for (i = 0; i< FD_SETSIZE; i++)
> - if (FD_ISSET(i,&keepfd))
> - close(i);
> + if (FD_ISSET(i,&keepfd)) {
> + tmpfd = i;
> + VIR_FORCE_CLOSE(tmpfd);
Another awfully large scope for a needed temporary.
Ok.
> Index: libvirt-acl/src/util/bridge.c
> @@ -107,7 +108,7 @@ brShutdown(brControl *ctl)
> if (!ctl)
> return;
>
> - close(ctl->fd);
> + VIR_FORCE_CLOSE(ctl->fd);
> ctl->fd = 0;
Huh - is this an existing logic bug? Can we end up accidentally
double-closing stdin?
I'ld leave the patch for now as it is, i.e., do the VIR_FORCE_CLOSE and
remember to investigate. So far, the changed does not have any further
negative impact that already isn't there - but it also doesn't solve a
potential problem.
> Index: libvirt-acl/src/util/logging.c
> ===================================================================
> --- libvirt-acl.orig/src/util/logging.c
> +++ libvirt-acl/src/util/logging.c
> @@ -40,6 +40,7 @@
> #include "util.h"
> #include "buf.h"
> #include "threads.h"
> +#include "files.h"
>
> /*
> * Macro used to format the message as a string in virLogMessage
> @@ -603,8 +604,7 @@ static int virLogOutputToFd(const char *
> static void virLogCloseFd(void *data) {
> int fd = (long) data;
>
> - if (fd>= 0)
> - close(fd);
> + VIR_FORCE_CLOSE(fd);
Should we fix this function to return an int value, and return
VIR_CLOSE(fd) so that callers can choose to detect log close failures?
Also that I would delay until further 'cause this may have consequences
for those calling the function.
> Index: libvirt-acl/src/util/macvtap.c
> ===================================================================
> --- libvirt-acl.orig/src/util/macvtap.c
> +++ libvirt-acl/src/util/macvtap.c
> @@ -52,6 +52,7 @@
> # include "conf/domain_conf.h"
> # include "virterror_internal.h"
> # include "uuid.h"
> +# include "files.h"
>
> # define VIR_FROM_THIS VIR_FROM_NET
>
> @@ -94,7 +95,7 @@ static int nlOpen(void)
>
> static void nlClose(int fd)
> {
> - close(fd);
> + VIR_FORCE_CLOSE(fd);
Likewise?
This here is a close of a netlink socket, which was used to communicate
with the kernel. I would just close it and discard the returned value.
> Index: libvirt-acl/src/util/util.c
> ===================================================================
> --- libvirt-acl.orig/src/util/util.c
> +++ libvirt-acl/src/util/util.c
> @@ -71,6 +71,7 @@
> #include "memory.h"
> #include "threads.h"
> #include "verify.h"
> +#include "files.h"
>
> #ifndef NSIG
> # define NSIG 32
> @@ -461,6 +462,7 @@ __virExec(const char *const*argv,
> int pipeerr[2] = {-1,-1};
> int childout = -1;
> int childerr = -1;
> + int tmpfd;
> @@ -568,8 +570,10 @@ __virExec(const char *const*argv,
> i != childout&&
> i != childerr&&
> (!keepfd ||
> - !FD_ISSET(i, keepfd)))
> - close(i);
> + !FD_ISSET(i, keepfd))) {
> + tmpfd = i;
> + VIR_FORCE_CLOSE(tmpfd);
I thought this was another large scope for a temporary....
> + }
>
> if (dup2(infd>= 0 ? infd : null, STDIN_FILENO)< 0) {
> virReportSystemError(errno,
> @@ -589,14 +593,15 @@ __virExec(const char *const*argv,
> goto fork_error;
> }
>
> - if (infd> 0)
> - close(infd);
> - close(null);
> - if (childout> 0)
> - close(childout);
> + VIR_FORCE_CLOSE(infd);
> + VIR_FORCE_CLOSE(null);
> + tmpfd = childout; /* preserve childout value */
> + VIR_FORCE_CLOSE(tmpfd);
...but you needed it here too.
Yes, here I need it twice. So not changing it.
> if (childerr> 0&&
> - childerr != childout)
> - close(childerr);
> + childerr != childout) {
> + VIR_FORCE_CLOSE(childerr);
> + childout = -1;
> + }
Looks okay after all - certainly not one of the trivial conversions
though :)
> Index: libvirt-acl/src/util/virtaudit.c
> ===================================================================
> --- libvirt-acl.orig/src/util/virtaudit.c
> +++ libvirt-acl/src/util/virtaudit.c
> @@ -30,6 +30,7 @@
> #include "virterror_internal.h"
> #include "logging.h"
> #include "virtaudit.h"
> +#include "files.h"
>
> /* Provide the macros in case the header file is old.
> FIXME: should be removed. */
> @@ -133,6 +134,6 @@ void virAuditSend(const char *file ATTRI
> void virAuditClose(void)
> {
> #if HAVE_AUDIT
> - close(auditfd);
> + VIR_CLOSE(auditfd);
Must be VIR_FORCE_CLOSE; I'm guessing you didn't have audit turned on in
your compile.
Right...
> Index: libvirt-acl/src/xen/proxy_internal.c
> @@ -236,12 +237,11 @@ virProxyCloseSocket(xenUnifiedPrivatePtr
> if (priv->proxy< 0)
> return(-1);
>
> - ret = close(priv->proxy);
> + ret = VIR_CLOSE(priv->proxy);
> if (ret != 0)
> VIR_WARN("Failed to close socket %d", priv->proxy);
> else
> VIR_DEBUG("Closed socket %d", priv->proxy);
Subtle unintended semantic change; you're now printing -1 instead of the
fd you just closed; you'll need a temporary.
There was another one like this that I got right :)
> Index: libvirt-acl/src/xen/xend_internal.c
> @@ -137,9 +137,7 @@ do_connect(virConnectPtr xend)
>
>
> if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) ==
-1) {
> - serrno = errno;
> - close(s);
> - errno = serrno;
> + VIR_FORCE_CLOSE(s);
> s = -1;
Redundant line. Overall, I'm impressed with how many lines this is
shaving off the code base! I think we settled on a pretty good calling
convention.
Fixed. Will post v2 and if useful a diff(v1,v2).
Stefan
And with that, I've completed my review of v1.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:30 p.m.
On 10/22/2010 09:21 PM, Stefan Berger wrote:
libvir-list-bounces(a)redhat.com (Eric Blake) wrote on 10/22/2010
06:27:30 PM:
[...]
>
> > Index: libvirt-acl/src/qemu/qemu_monitor.c
> >
> > @@ -694,8 +695,7 @@ void qemuMonitorClose(qemuMonitorPtr mon
> > if (!mon->closed) {
> > if (mon->watch)
> > virEventRemoveHandle(mon->watch);
> > - if (mon->fd != -1)
> > - close(mon->fd);
> > + VIR_FORCE_CLOSE(mon->fd);
> > /* NB: ordinarily one might immediately set mon->watch to -1
> > * and mon->fd to -1, but there may be a callback active
> > * that is still relying on these fields being valid. So
>
> Ouch - given that comment, could we be frying a callback by setting
> mon->fd to -1 in VIR_FORCE_CLOSE? We need to double check this, and
> possibly use a temporary variable if the callback indeed needs a
> non-negative mon->fd for a bit longer, or tighten the specification and
> all existing callbacks to tolerate mon->fd changing to -1.
The only possible thing I could think a callback might legitimately use
a non-negative fd for would be to answer the question "was this file
previously successfully opened?" I would say that if it's being used for
that, it would be cleaner to have a separate flag set in the qemuMonitor
object that was set when the fd was opened, and never reset.
Any other use of the value of fd after it's closed that I can think of
is a bug, and changing fd to -1 here would hopefully reveal that bug so
it could be fixed.
>
>
> > Index: libvirt-acl/src/util/bridge.c
> > @@ -107,7 +108,7 @@ brShutdown(brControl *ctl)
> > if (!ctl)
> > return;
> >
> > - close(ctl->fd);
> > + VIR_FORCE_CLOSE(ctl->fd);
> > ctl->fd = 0;
>
> Huh - is this an existing logic bug? Can we end up accidentally
> double-closing stdin?
Am I missing something? Sure, ctl->fd is set to 0, but then the memory
at ctl is immediately freed (in the next line, which doesn't show up in
the patch diff), and never referenced again. I'm not even sure why they
bothered to set ctl->fd to 0, since it's guaranteed to never be used again.
I'ld leave the patch for now as it is, i.e., do the
VIR_FORCE_CLOSE
and remember to investigate.
I think we can/should remove both the close() and the ctl->fd = 0, and
replace them with VIR_FORCE_CLOSE().
So far, the changed does not have any further negative impact that
already isn't there - but it also doesn't solve a potential problem.
>
> > Index: libvirt-acl/src/util/logging.c
> > ===================================================================
> > --- libvirt-acl.orig/src/util/logging.c
> > +++ libvirt-acl/src/util/logging.c
> > @@ -40,6 +40,7 @@
> > #include "util.h"
> > #include "buf.h"
> > #include "threads.h"
> > +#include "files.h"
> >
> > /*
> > * Macro used to format the message as a string in virLogMessage
> > @@ -603,8 +604,7 @@ static int virLogOutputToFd(const char *
> > static void virLogCloseFd(void *data) {
> > int fd = (long) data;
> >
> > - if (fd>= 0)
> > - close(fd);
> > + VIR_FORCE_CLOSE(fd);
>
> Should we fix this function to return an int value, and return
> VIR_CLOSE(fd) so that callers can choose to detect log close failures?
Also that I would delay until further 'cause this may have
consequences for those calling the function.
Agreed. That's a good idea, but should be a separate patch.
>
> > Index: libvirt-acl/src/util/macvtap.c
> > ===================================================================
> > --- libvirt-acl.orig/src/util/macvtap.c
> > +++ libvirt-acl/src/util/macvtap.c
> > @@ -52,6 +52,7 @@
> > # include "conf/domain_conf.h"
> > # include "virterror_internal.h"
> > # include "uuid.h"
> > +# include "files.h"
> >
> > # define VIR_FROM_THIS VIR_FROM_NET
> >
> > @@ -94,7 +95,7 @@ static int nlOpen(void)
> >
> > static void nlClose(int fd)
> > {
> > - close(fd);
> > + VIR_FORCE_CLOSE(fd);
>
> Likewise?
This here is a close of a netlink socket, which was used to
communicate with the kernel. I would just close it and discard the
returned value.
I kind of agree with that, but since a close failure should never
happen, when it does happen it may very well indicate something "Very
Bad" (eg, corrupted memory, leading to eventual exhaustion of nl sockets
(which turns out doesn't take very long)), so we might want to log an
error just so they'll know. However, the error could just as well be
logged right here, as to return the status to the caller. (And again, I
think it can be a separate patch).
5144
days inactive
5147
days old
8 comments
4 participants
participants (4)
-
Eric Blake -
Laine Stump -
Stefan Berger -
Stefan Berger