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

[libvirt] [PATCH v2] deprecate fclose() and introduce VIR_{FORCE_}FCLOSE()



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 us ibm com>

---
 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            |   12 ++++++------
 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                    |   32 ++++++++++++++++++++++++++++++++
 src/util/files.h                    |    6 ++++++
 src/util/macvtap.c                  |    4 ++--
 src/util/stats_linux.c              |    3 ++-
 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, 164 insertions(+), 90 deletions(-)

Index: libvirt-acl/src/util/files.c
===================================================================
--- libvirt-acl.orig/src/util/files.c
+++ libvirt-acl/src/util/files.c
@@ -44,3 +44,35 @@ 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;
+    }
+
+    return file;
+}
Index: libvirt-acl/src/util/files.h
===================================================================
--- libvirt-acl.orig/src/util/files.h
+++ libvirt-acl/src/util/files.h
@@ -27,6 +27,7 @@
 # define __VIR_FILES_H_
 
 # include <stdbool.h>
+# include <stdio.h>
 
 # include "internal.h"
 # include "ignore-value.h"
@@ -34,13 +35,18 @@
 
 /* Don't call this 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);
 
 /* 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)
+# 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;
@@ -906,10 +906,10 @@ openvzSetDefinedUUID(int vpsid, unsigned
 
         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;
     }
 
@@ -996,7 +996,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
@@ -436,11 +436,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)) {
+        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) &lt; 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) &lt; 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
 
@@ -103,7 +104,7 @@ linuxDomainInterfaceStats(const char *pa
             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;
 


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