V3:
- fixes from V2 review + one lost hunk
-> diff(tree+V2, tree+v3) at bottom of email
V2:
- following Eric's suggestion, deprecating fdclose(), introducing VIR_FDCLOSE()
- following some other nits that Eric pointed out
- replaced some more fclose () I previously had missed (last 2 files in patch)
Similarly to deprecating close(), I am now deprecating fclose() and
introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with
VIR_FDOPEN().
Most of the files are opened in read-only mode, so usage of
VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write
mode already had the fclose()< 0 check and I converted those to
VIR_FCLOSE()< 0.
I did not find occurrences of possible double-closed files on the way.
Signed-off-by: Stefan Berger<stefanb@xxxxxxxxxx>
---
HACKING | 33 +++++++++++++++++++++++++++------
daemon/libvirtd.c | 6 +++---
docs/hacking.html.in | 36 ++++++++++++++++++++++++++++--------
src/libvirt_private.syms | 2 ++
src/nodeinfo.c | 8 ++++----
src/openvz/openvz_conf.c | 16 +++++++++-------
src/openvz/openvz_driver.c | 4 ++--
src/qemu/qemu_driver.c | 4 ++--
src/storage/storage_backend.c | 18 +++++++-----------
src/storage/storage_backend_fs.c | 4 ++--
src/storage/storage_backend_iscsi.c | 9 +++------
src/storage/storage_backend_scsi.c | 2 +-
src/uml/uml_driver.c | 8 ++++----
src/util/cgroup.c | 10 +++++-----
src/util/dnsmasq.c | 5 +++--
src/util/files.c | 34 ++++++++++++++++++++++++++++++++++
src/util/files.h | 10 +++++++++-
src/util/macvtap.c | 4 ++--
src/util/stats_linux.c | 5 +++--
src/util/util.c | 10 ++++------
src/xen/block_stats.c | 3 ++-
src/xen/xen_driver.c | 7 ++++---
src/xen/xen_hypervisor.c | 8 +++-----
tests/nodeinfotest.c | 5 +++--
tests/testutils.c | 8 ++++----
tests/xencapstest.c | 7 +++----
26 files changed, 173 insertions(+), 93 deletions(-)
Index: libvirt-acl/src/util/files.c
===================================================================
--- libvirt-acl.orig/src/util/files.c
+++ libvirt-acl/src/util/files.c
@@ -44,3 +44,37 @@ int virClose(int *fdptr, bool preserve_e
return rc;
}
+
+
+int virFclose(FILE **file, bool preserve_errno)
+{
+ int saved_errno;
+ int rc = 0;
+
+ if (*file) {
+ if (preserve_errno)
+ saved_errno = errno;
+ rc = fclose(*file);
+ *file = NULL;
+ if (preserve_errno)
+ errno = saved_errno;
+ }
+
+ return rc;
+}
+
+
+FILE *virFdopen(int *fdptr, const char *mode)
+{
+ FILE *file = NULL;
+
+ if (*fdptr>= 0) {
+ file = fdopen(*fdptr, mode);
+ if (file)
+ *fdptr = -1;
+ } else {
+ errno = EBADF;
+ }
+
+ return file;
+}
Index: libvirt-acl/src/util/files.h
===================================================================
--- libvirt-acl.orig/src/util/files.h
+++ libvirt-acl/src/util/files.h
@@ -27,20 +27,28 @@
# define __VIR_FILES_H_
# include<stdbool.h>
+# include<stdio.h>
# include "internal.h"
# include "ignore-value.h"
-/* Don't call this directly - use the macros below */
+/* Don't call these directly - use the macros below */
int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
+int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
+FILE *virFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
/* For use on normal paths; caller must check return value,
and failure sets errno per close(). */
# define VIR_CLOSE(FD) virClose(&(FD), false)
+# define VIR_FCLOSE(FILE) virFclose(&(FILE), false)
+
+/* Wrapper around fdopen that consumes fd on success. */
+# define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE)
/* For use on cleanup paths; errno is unaffected by close,
and no return value to worry about. */
# define VIR_FORCE_CLOSE(FD) ignore_value(virClose(&(FD), true))
+# define VIR_FORCE_FCLOSE(FILE) ignore_value(virFclose(&(FILE), true))
#endif /* __VIR_FILES_H */
Index: libvirt-acl/src/libvirt_private.syms
===================================================================
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -344,6 +344,8 @@ virFDStreamCreateFile;
# files.h
virClose;
+virFclose;
+virFdopen;
# hash.h
Index: libvirt-acl/daemon/libvirtd.c
===================================================================
--- libvirt-acl.orig/daemon/libvirtd.c
+++ libvirt-acl/daemon/libvirtd.c
@@ -512,7 +512,7 @@ static int qemudWritePidFile(const char
return -1;
}
- if (!(fh = fdopen(fd, "w"))) {
+ if (!(fh = VIR_FDOPEN(fd, "w"))) {
VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"),
pidFile, virStrerror(errno, ebuf, sizeof ebuf));
VIR_FORCE_CLOSE(fd);
@@ -522,11 +522,11 @@ static int qemudWritePidFile(const char
if (fprintf(fh, "%lu\n", (unsigned long)getpid())< 0) {
VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"),
argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf));
- fclose(fh);
+ VIR_FORCE_FCLOSE(fh);
return -1;
}
- if (fclose(fh) == EOF) {
+ if (VIR_FCLOSE(fh) == EOF) {
VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"),
argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
Index: libvirt-acl/src/nodeinfo.c
===================================================================
--- libvirt-acl.orig/src/nodeinfo.c
+++ libvirt-acl/src/nodeinfo.c
@@ -46,6 +46,7 @@
#include "virterror_internal.h"
#include "count-one-bits.h"
#include "intprops.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -102,8 +103,7 @@ get_cpu_value(unsigned int cpu, const ch
}
cleanup:
- if (pathfp)
- fclose(pathfp);
+ VIR_FORCE_FCLOSE(pathfp);
VIR_FREE(path);
return value;
@@ -155,7 +155,7 @@ static unsigned long count_thread_siblin
}
cleanup:
- fclose(pathfp);
+ VIR_FORCE_FCLOSE(pathfp);
VIR_FREE(path);
return ret;
@@ -329,7 +329,7 @@ int nodeGetInfo(virConnectPtr conn ATTRI
return -1;
}
ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo);
- fclose(cpuinfo);
+ VIR_FORCE_FCLOSE(cpuinfo);
if (ret< 0)
return -1;
Index: libvirt-acl/src/openvz/openvz_conf.c
===================================================================
--- libvirt-acl.orig/src/openvz/openvz_conf.c
+++ libvirt-acl/src/openvz/openvz_conf.c
@@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driv
dom = NULL;
}
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return 0;
@@ -536,7 +536,7 @@ int openvzLoadDomains(struct openvz_driv
virReportOOMError();
cleanup:
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
if (dom)
virDomainObjUnref(dom);
return -1;
@@ -888,6 +888,7 @@ openvzSetDefinedUUID(int vpsid, unsigned
{
char *conf_file;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ FILE *fp = NULL;
int ret = -1;
if (uuid == NULL)
@@ -900,21 +901,22 @@ openvzSetDefinedUUID(int vpsid, unsigned
goto cleanup;
if (uuidstr[0] == 0) {
- FILE *fp = fopen(conf_file, "a"); /* append */
+ fp = fopen(conf_file, "a"); /* append */
if (fp == NULL)
goto cleanup;
virUUIDFormat(uuid, uuidstr);
- /* Record failure if fprintf or fclose fails,
+ /* Record failure if fprintf or VIR_FCLOSE fails,
and be careful always to close the stream. */
- if ((fprintf(fp, "\n#UUID: %s\n", uuidstr)< 0)
- + (fclose(fp) == EOF))
+ if ((fprintf(fp, "\n#UUID: %s\n", uuidstr)< 0) ||
+ (VIR_FCLOSE(fp) == EOF))
goto cleanup;
}
ret = 0;
cleanup:
+ VIR_FORCE_FCLOSE(fp);
VIR_FREE(conf_file);
return ret;
}
@@ -996,7 +998,7 @@ int openvzGetVEID(const char *name) {
}
ok = fscanf(fp, "%d\n",&veid ) == 1;
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
if (ok&& veid>= 0)
return veid;
Index: libvirt-acl/src/openvz/openvz_driver.c
===================================================================
--- libvirt-acl.orig/src/openvz/openvz_driver.c
+++ libvirt-acl/src/openvz/openvz_driver.c
@@ -154,7 +154,7 @@ openvzDomainDefineCmd(const char *args[]
max_veid = veid;
}
}
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
if (max_veid == 0) {
max_veid = 100;
@@ -189,7 +189,7 @@ no_memory:
return -1;
cleanup:
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return -1;
#undef ADD_ARG
Index: libvirt-acl/src/qemu/qemu_driver.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_driver.c
+++ libvirt-acl/src/qemu/qemu_driver.c
@@ -4588,7 +4588,7 @@ static int qemudGetProcessInfo(unsigned
/* startstack -> processor */
"%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
&usertime,&systime,&cpu) != 3) {
- fclose(pidinfo);
+ VIR_FORCE_FCLOSE(pidinfo);
VIR_WARN0("cannot parse process status data");
errno = -EINVAL;
return -1;
@@ -4608,7 +4608,7 @@ static int qemudGetProcessInfo(unsigned
VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d",
pid, tid, usertime, systime, cpu);
- fclose(pidinfo);
+ VIR_FORCE_FCLOSE(pidinfo);
return 0;
}
Index: libvirt-acl/src/storage/storage_backend.c
===================================================================
--- libvirt-acl.orig/src/storage/storage_backend.c
+++ libvirt-acl/src/storage/storage_backend.c
@@ -1398,7 +1398,7 @@ virStorageBackendRunProgRegex(virStorage
goto cleanup;
}
- if ((list = fdopen(fd, "r")) == NULL) {
+ if ((list = VIR_FDOPEN(fd, "r")) == NULL) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot read fd"));
goto cleanup;
@@ -1458,10 +1458,8 @@ virStorageBackendRunProgRegex(virStorage
VIR_FREE(reg);
- if (list)
- fclose(list);
- else
- VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_FCLOSE(list);
+ VIR_FORCE_CLOSE(fd);
while ((err = waitpid(child,&exitstatus, 0) == -1)&& errno == EINTR);
@@ -1531,9 +1529,9 @@ virStorageBackendRunProgNul(virStoragePo
goto cleanup;
}
- if ((fp = fdopen(fd, "r")) == NULL) {
+ if ((fp = VIR_FDOPEN(fd, "r")) == NULL) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot read fd"));
+ "%s", _("cannot open file using fd"));
goto cleanup;
}
@@ -1573,10 +1571,8 @@ virStorageBackendRunProgNul(virStoragePo
VIR_FREE(v[i]);
VIR_FREE(v);
- if (fp)
- fclose (fp);
- else
- VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_FCLOSE(fp);
+ VIR_FORCE_CLOSE(fd);
while ((w_err = waitpid (child,&exitstatus, 0) == -1)&& errno == EINTR)
/* empty */ ;
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
@@ -284,12 +284,12 @@ virStorageBackendFileSystemIsMounted(vir
while ((getmntent_r(mtab,&ent, buf, sizeof(buf))) != NULL) {
if (STREQ(ent.mnt_dir, pool->def->target.path)) {
- fclose(mtab);
+ VIR_FORCE_FCLOSE(mtab);
return 1;
}
}
- fclose(mtab);
+ VIR_FORCE_FCLOSE(mtab);
return 0;
}
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
@@ -188,7 +188,7 @@ virStorageBackendIQNFound(virStoragePool
goto out;
}
- if ((fp = fdopen(fd, "r")) == NULL) {
+ if ((fp = VIR_FDOPEN(fd, "r")) == NULL) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open stream for file descriptor "
"when reading output from '%s': '%s'"),
@@ -235,11 +235,8 @@ out:
}
VIR_FREE(line);
- if (fp != NULL) {
- fclose(fp);
- } else {
- VIR_FORCE_CLOSE(fd);
- }
+ VIR_FORCE_FCLOSE(fp);
+ 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
@@ -70,7 +70,7 @@ getDeviceType(uint32_t host,
}
gottype = fgets(typestr, 3, typefile);
- fclose(typefile);
+ VIR_FORCE_FCLOSE(typefile);
if (gottype == NULL) {
virReportSystemError(errno,
Index: libvirt-acl/src/uml/uml_driver.c
===================================================================
--- libvirt-acl.orig/src/uml/uml_driver.c
+++ libvirt-acl/src/uml/uml_driver.c
@@ -587,11 +587,11 @@ reopen:
if (fscanf(file, "%d",&vm->pid) != 1) {
errno = EINVAL;
- fclose(file);
+ VIR_FORCE_FCLOSE(file);
goto cleanup;
}
- if (fclose(file)< 0)
+ if (VIR_FCLOSE(file)< 0)
goto cleanup;
rc = 0;
@@ -1096,7 +1096,7 @@ static int umlGetProcessInfo(unsigned lo
if (fscanf(pidinfo, "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu",&usertime,&systime) != 2) {
umlDebug("not enough arg");
- fclose(pidinfo);
+ VIR_FORCE_FCLOSE(pidinfo);
return -1;
}
@@ -1109,7 +1109,7 @@ static int umlGetProcessInfo(unsigned lo
umlDebug("Got %llu %llu %llu", usertime, systime, *cpuTime);
- fclose(pidinfo);
+ VIR_FORCE_FCLOSE(pidinfo);
return 0;
}
Index: libvirt-acl/src/util/cgroup.c
===================================================================
--- libvirt-acl.orig/src/util/cgroup.c
+++ libvirt-acl/src/util/cgroup.c
@@ -31,6 +31,7 @@
#include "memory.h"
#include "cgroup.h"
#include "logging.h"
+#include "files.h"
#define CGROUP_MAX_VAL 512
@@ -127,13 +128,12 @@ static int virCgroupDetectMounts(virCgro
}
}
- fclose(mounts);
+ VIR_FORCE_FCLOSE(mounts);
return 0;
no_memory:
- if (mounts)
- fclose(mounts);
+ VIR_FORCE_FCLOSE(mounts);
return -ENOMEM;
}
@@ -192,12 +192,12 @@ static int virCgroupDetectPlacement(virC
}
}
- fclose(mapping);
+ VIR_FORCE_FCLOSE(mapping);
return 0;
no_memory:
- fclose(mapping);
+ VIR_FORCE_FCLOSE(mapping);
return -ENOMEM;
}
Index: libvirt-acl/src/util/dnsmasq.c
===================================================================
--- libvirt-acl.orig/src/util/dnsmasq.c
+++ libvirt-acl/src/util/dnsmasq.c
@@ -44,6 +44,7 @@
#include "memory.h"
#include "virterror_internal.h"
#include "logging.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_NETWORK
#define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile"
@@ -171,7 +172,7 @@ hostsfileWrite(const char *path,
for (i = 0; i< nhosts; i++) {
if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) {
rc = errno;
- fclose(f);
+ VIR_FORCE_FCLOSE(f);
if (istmp)
unlink(tmp);
@@ -180,7 +181,7 @@ hostsfileWrite(const char *path,
}
}
- if (fclose(f) == EOF) {
+ if (VIR_FCLOSE(f) == EOF) {
rc = errno;
goto cleanup;
}
Index: libvirt-acl/src/util/macvtap.c
===================================================================
--- libvirt-acl.orig/src/util/macvtap.c
+++ libvirt-acl/src/util/macvtap.c
@@ -414,11 +414,11 @@ int openTap(const char *ifname,
virReportSystemError(errno,
"%s",_("cannot determine macvtap's tap device "
"interface index"));
- fclose(file);
+ VIR_FORCE_FCLOSE(file);
return -1;
}
- fclose(file);
+ VIR_FORCE_FCLOSE(file);
if (snprintf(tapname, sizeof(tapname),
"/dev/tap%d", ifindex)>= sizeof(tapname)) {
Index: libvirt-acl/src/util/util.c
===================================================================
--- libvirt-acl.orig/src/util/util.c
+++ libvirt-acl/src/util/util.c
@@ -1816,7 +1816,7 @@ int virFileWritePidPath(const char *pidf
goto cleanup;
}
- if (!(file = fdopen(fd, "w"))) {
+ if (!(file = VIR_FDOPEN(fd, "w"))) {
rc = errno;
VIR_FORCE_CLOSE(fd);
goto cleanup;
@@ -1830,10 +1830,8 @@ int virFileWritePidPath(const char *pidf
rc = 0;
cleanup:
- if (file&&
- fclose(file)< 0) {
+ if (VIR_FCLOSE(file)< 0)
rc = errno;
- }
return rc;
}
@@ -1864,11 +1862,11 @@ int virFileReadPid(const char *dir,
if (fscanf(file, "%d", pid) != 1) {
rc = EINVAL;
- fclose(file);
+ VIR_FORCE_FCLOSE(file);
goto cleanup;
}
- if (fclose(file)< 0) {
+ if (VIR_FCLOSE(file)< 0) {
rc = errno;
goto cleanup;
}
Index: libvirt-acl/src/xen/xen_driver.c
===================================================================
--- libvirt-acl.orig/src/xen/xen_driver.c
+++ libvirt-acl/src/xen/xen_driver.c
@@ -47,6 +47,7 @@
#include "pci.h"
#include "uuid.h"
#include "fdstream.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_XEN
@@ -216,10 +217,10 @@ xenUnifiedProbe (void)
return 1;
#endif
#ifdef __sun
- FILE *fh;
+ int fd;
- if (fh = fopen("/dev/xen/domcaps", "r")) {
- fclose(fh);
+ if ((fd = open("/dev/xen/domcaps", O_RDONLY))>= 0) {
+ VIR_FORCE_CLOSE(fd);
return 1;
}
#endif
Index: libvirt-acl/src/xen/xen_hypervisor.c
===================================================================
--- libvirt-acl.orig/src/xen/xen_hypervisor.c
+++ libvirt-acl/src/xen/xen_hypervisor.c
@@ -2641,7 +2641,7 @@ xenHypervisorMakeCapabilities(virConnect
capabilities = fopen ("/sys/hypervisor/properties/capabilities", "r");
if (capabilities == NULL) {
if (errno != ENOENT) {
- fclose(cpuinfo);
+ VIR_FORCE_FCLOSE(cpuinfo);
virReportSystemError(errno,
_("cannot read file %s"),
"/sys/hypervisor/properties/capabilities");
@@ -2654,10 +2654,8 @@ xenHypervisorMakeCapabilities(virConnect
cpuinfo,
capabilities);
- if (cpuinfo)
- fclose(cpuinfo);
- if (capabilities)
- fclose(capabilities);
+ VIR_FORCE_FCLOSE(cpuinfo);
+ VIR_FORCE_FCLOSE(capabilities);
return caps;
#endif /* __sun */
Index: libvirt-acl/tests/nodeinfotest.c
===================================================================
--- libvirt-acl.orig/tests/nodeinfotest.c
+++ libvirt-acl/tests/nodeinfotest.c
@@ -9,6 +9,7 @@
#include "internal.h"
#include "nodeinfo.h"
#include "util.h"
+#include "files.h"
#ifndef __linux__
@@ -49,10 +50,10 @@ static int linuxTestCompareFiles(const c
fprintf(stderr, "\n%s\n", error->message);
virFreeError(error);
}
- fclose(cpuinfo);
+ VIR_FORCE_FCLOSE(cpuinfo);
return -1;
}
- fclose(cpuinfo);
+ VIR_FORCE_FCLOSE(cpuinfo);
/* 'nodes' is filled using libnuma.so from current machine
* topology, which makes it unsuitable for the test suite
Index: libvirt-acl/tests/testutils.c
===================================================================
--- libvirt-acl.orig/tests/testutils.c
+++ libvirt-acl/tests/testutils.c
@@ -180,26 +180,26 @@ int virtTestLoadFile(const char *file,
if (fstat(fileno(fp),&st)< 0) {
fprintf (stderr, "%s: failed to fstat: %s\n", file, strerror(errno));
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return -1;
}
if (st.st_size> (buflen-1)) {
fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1);
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return -1;
}
if (st.st_size) {
if (fread(*buf, st.st_size, 1, fp) != 1) {
fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno));
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return -1;
}
}
(*buf)[st.st_size] = '\0';
- fclose(fp);
+ VIR_FORCE_FCLOSE(fp);
return st.st_size;
}
Index: libvirt-acl/tests/xencapstest.c
===================================================================
--- libvirt-acl.orig/tests/xencapstest.c
+++ libvirt-acl/tests/xencapstest.c
@@ -9,6 +9,7 @@
#include "xml.h"
#include "testutils.h"
#include "xen/xen_hypervisor.h"
+#include "files.h"
static char *progname;
static char *abs_srcdir;
@@ -63,10 +64,8 @@ static int testCompareFiles(const char *
fail:
free(actualxml);
- if (fp1)
- fclose(fp1);
- if (fp2)
- fclose(fp2);
+ VIR_FORCE_FCLOSE(fp1);
+ VIR_FORCE_FCLOSE(fp2);
virCapabilitiesFree(caps);
return ret;
Index: libvirt-acl/docs/hacking.html.in
===================================================================
--- libvirt-acl.orig/docs/hacking.html.in
+++ libvirt-acl/docs/hacking.html.in
@@ -414,25 +414,45 @@
<h2><a name="file_handling">File handling</a></h2>
<p>
- Use of the close() API is deprecated in libvirt code base to help
- avoiding double-closing of a file descriptor. Instead of this API,
- use the macro from files.h
+ Usage of the<code>fdopen()</code>,<code>close()</code>,<code>fclose()</code>
+ APIs is deprecated in libvirt code base to help avoiding double-closing of files
+ or file descriptors, which is particulary dangerous in a multi-threaded
+ applications. Instead of these APIs, use the macros from files.h
</p>
-<ul>
+<ul>
+<li><p>eg opening a file from a file descriptor</p>
+
+<pre>
+ if ((file = VIR_FDOPEN(fd, "r")) == NULL) {
+ virReportSystemError(errno, "%s",
+ _("failed to open file from file descriptor"));
+ return -1;
+ }
+ /* fd is now invalid; only access the file using file variable */
+</pre></li>
+
<li><p>e.g. close a file descriptor</p>
<pre>
if (VIR_CLOSE(fd)< 0) {
- virReportSystemError(errno, _("failed to close file"));
+ virReportSystemError(errno, "%s", _("failed to close file"));
}
-</pre>
-</li>
+</pre></li>
+
+<li><p>eg close a file</p>
+
+<pre>
+ if (VIR_FCLOSE(file)< 0) {
+ virReportSystemError(errno, "%s", _("failed to close file"));
+ }
+</pre></li>
-<li><p>eg close a file descriptor in an error path, without losing
+<li><p>eg close a file or file descriptor in an error path, without losing
the previous<code>errno</code> value</p>
<pre>
VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_FCLOSE(file);
</pre>
</li>
</ul>
Index: libvirt-acl/HACKING
===================================================================
--- libvirt-acl.orig/HACKING
+++ libvirt-acl/HACKING
@@ -339,22 +339,43 @@ routines, use the macros from memory.h
File handling
=============
-Use of the close() API is deprecated in libvirt code base to help avoiding
-double-closing of a file descriptor. Instead of this API, use the macro from
-files.h
+Usage of the "fdopen()", "close()", "fclose()" APIs is deprecated in libvirt
+code base to help avoiding double-closing of files or file descriptors, which
+is particulary dangerous in a multi-threaded applications. Instead of these
+APIs, use the macros from files.h
+
+- eg opening a file from a file descriptor
+
+ if ((file = VIR_FDOPEN(fd, "r")) == NULL) {
+ virReportSystemError(errno, "%s",
+ _("failed to open file from file descriptor"));
+ return -1;
+ }
+ /* fd is now invalid; only access the file using file variable */
+
+
- e.g. close a file descriptor
if (VIR_CLOSE(fd)< 0) {
- virReportSystemError(errno, _("failed to close file"));
+ virReportSystemError(errno, "%s", _("failed to close file"));
+ }
+
+
+
+- eg close a file
+
+ if (VIR_FCLOSE(file)< 0) {
+ virReportSystemError(errno, "%s", _("failed to close file"));
}
-- eg close a file descriptor in an error path, without losing the previous
-"errno" value
+- eg close a file or file descriptor in an error path, without losing the
+previous "errno" value
VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_FCLOSE(file);
Index: libvirt-acl/src/util/stats_linux.c
===================================================================
--- libvirt-acl.orig/src/util/stats_linux.c
+++ libvirt-acl/src/util/stats_linux.c
@@ -25,6 +25,7 @@
# include "util.h"
# include "stats_linux.h"
# include "memory.h"
+# include "files.h"
# define VIR_FROM_THIS VIR_FROM_STATS_LINUX
@@ -98,12 +99,12 @@ linuxDomainInterfaceStats(const char *pa
stats->tx_packets = tx_packets;
stats->tx_errs = tx_errs;
stats->tx_drop = tx_drop;
- fclose (fp);
+ VIR_FORCE_FCLOSE (fp);
return 0;
}
}
- fclose (fp);
+ VIR_FORCE_FCLOSE(fp);
virStatsError(VIR_ERR_INTERNAL_ERROR,
"/proc/net/dev: Interface not found");
Index: libvirt-acl/src/xen/block_stats.c
===================================================================
--- libvirt-acl.orig/src/xen/block_stats.c
+++ libvirt-acl/src/xen/block_stats.c
@@ -27,6 +27,7 @@
# include "util.h"
# include "block_stats.h"
# include "memory.h"
+# include "files.h"
# define VIR_FROM_THIS VIR_FROM_STATS_LINUX
@@ -100,7 +101,7 @@ read_stat (const char *path)
/* read, but don't bail out before closing */
i = fread (str, 1, sizeof str - 1, fp);
- if (fclose (fp) != 0 /* disk error */
+ if (VIR_FCLOSE(fp) != 0 /* disk error */
|| i< 1) /* ensure we read at least one byte */
return -1;
-------------------------------------------------------------
--- /root/tmp/bak/libvirt-acl/src/openvz/openvz_conf.c 2010-11-15 17:03:15.962422965 -0500
+++ src/openvz/openvz_conf.c 2010-11-16 06:31:49.716482090 -0500
@@ -887,8 +887,9 @@
openvzSetDefinedUUID(int vpsid, unsigned char *uuid)
{
char *conf_file;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ FILE *fp = NULL;
int ret = -1;
if (uuid == NULL)
return -1;
@@ -899,9 +900,9 @@
if (openvzGetVPSUUID(vpsid, uuidstr, sizeof(uuidstr)))
goto cleanup;
if (uuidstr[0] == 0) {
- FILE *fp = fopen(conf_file, "a"); /* append */
+ fp = fopen(conf_file, "a"); /* append */
if (fp == NULL)
goto cleanup;
virUUIDFormat(uuid, uuidstr);
@@ -914,8 +915,9 @@
}
ret = 0;
cleanup:
+ VIR_FORCE_FCLOSE(fp);
VIR_FREE(conf_file);
return ret;
}
--- /root/tmp/bak/libvirt-acl/src/util/files.c 2010-11-15 17:03:15.949423495 -0500
+++ src/util/files.c 2010-11-16 06:31:49.714482170 -0500
@@ -71,8 +71,10 @@
if (*fdptr>= 0) {
file = fdopen(*fdptr, mode);
if (file)
*fdptr = -1;
+ } else {
+ errno = EBADF;
}
return file;
}
--- /root/tmp/bak/libvirt-acl/src/util/files.h 2010-11-15 17:03:15.956423210 -0500
+++ src/util/files.h 2010-11-16 06:31:49.714482170 -0500
@@ -32,17 +32,19 @@
# include "internal.h"
# include "ignore-value.h"
-/* Don't call this directly - use the macros below */
+/* Don't call these directly - use the macros below */
int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
-FILE *virFdopen(int *fdptr, const char *mode);
+FILE *virFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
/* For use on normal paths; caller must check return value,
and failure sets errno per close(). */
# define VIR_CLOSE(FD) virClose(&(FD), false)
# define VIR_FCLOSE(FILE) virFclose(&(FILE), false)
+
+/* Wrapper around fdopen that consumes fd on success. */
# define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE)
/* For use on cleanup paths; errno is unaffected by close,
and no return value to worry about. */
--- /root/tmp/bak/libvirt-acl/src/util/stats_linux.c 2010-11-15 17:03:15.994421659 -0500
+++ src/util/stats_linux.c 2010-11-16 06:31:49.724481766 -0500
@@ -98,9 +98,9 @@
stats->tx_bytes = tx_bytes;
stats->tx_packets = tx_packets;
stats->tx_errs = tx_errs;
stats->tx_drop = tx_drop;
- fclose (fp);
+ VIR_FORCE_FCLOSE (fp);
return 0;
}
}
--- /root/tmp/bak/libvirt-acl/src/xen/xen_driver.c 2010-11-15 17:03:15.972422557 -0500
+++ src/xen/xen_driver.c 2010-11-16 06:31:49.722481846 -0500
@@ -218,9 +218,9 @@
#endif
#ifdef __sun
int fd;
- if (fd = open("/dev/xen/domcaps", O_RDONLY)) {
+ if ((fd = open("/dev/xen/domcaps", O_RDONLY))>= 0) {
VIR_FORCE_CLOSE(fd);
return 1;
}
#endif
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list