From: Stefan Berger Date: Wed, 17 Nov 2010 02:13:29 +0000 (-0500) Subject: deprecate fclose() and introduce VIR_{FORCE_}FCLOSE() X-Git-Tag: v0.8.6~88 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b7cb1ecc989db87f3ede16b9aefc0dd3fbd8301;p=thirdparty%2Flibvirt.git 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. --- 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 + + 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); 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

-