From 7b7cb1ecc989db87f3ede16b9aefc0dd3fbd8301 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Tue, 16 Nov 2010 21:13:29 -0500 Subject: [PATCH] deprecate fclose() and introduce VIR_{FORCE_}FCLOSE() 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. --- 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(-) diff --git a/HACKING b/HACKING index cbd38839f7..2711ea1e08 100644 --- a/HACKING +++ b/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 descriptor in an error path, without losing the previous -"errno" value +- eg close a file + + if (VIR_FCLOSE(file) < 0) { + virReportSystemError(errno, "%s", _("failed to close file")); + } + + + +- 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); diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 94466388a9..6c2d3c372b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -512,7 +512,7 @@ static int qemudWritePidFile(const char *pidFile) { 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 *pidFile) { 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; diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 47849c51ee..c42654e0b7 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -414,25 +414,45 @@

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

-