Thanks for pointing out the problem; I guess I was subconsciously hoping
that platforms lacking O_CLOEXEC were no longer a practical issue. But
that was silly.
On 08/14/2017 10:18 AM, Bruno Haible wrote:
Alternatively, define O_CLOEXEC to a non-zero value on those platforms
that don't support it, and extend gnulib's open() and openat() wrappers
to support O_CLOEXEC. But this is more hairy because the platforms could,
at any time, introduce other O_* flags that happen to collide wit the value
we happen to pick for O_CLOEXEC.
Despite that disadvantage, it would be a win for users or Gnulib, who
could stop worrying about the possibility that O_CLOEXEC == 0. I
installed the attached patch to try to implement this. I don't use
MS-Windows, though, and may well have missed something.
>From 9830b8cbec084211d3169bbb074317053cb1088e Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Mon, 14 Aug 2017 13:04:46 -0700
Subject: [PATCH] open: support O_CLOEXEC
* NEWS, doc/posix-functions/open.texi:
* doc/posix-functions/openat.texi: Document this.
* lib/fcntl.in.h (O_CLOEXEC): Default to a nonzero value.
(GNULIB_defined_O_CLOEXEC): New symbol.
* lib/open.c: Include cloexec.h.
(open): Support O_CLOEXEC.
* lib/openat.c: Include cloexec.h.
(rpl_openat): Support O_CLOEXEC.
* lib/popen-safer.c: Do not include cloexec.h.
(open_noinherit): Remove.
(popen_safer): Use O_CLOEXEC instead of set_cloexec_flag.
* lib/save-cwd.c: Do not include cloexec.h.
(save_cwd): Use O_CLOEXEC instead of set_cloexec_flag.
* m4/open-cloexec.m4: New file.
* m4/open.m4 (gl_FUNC_OPEN): Require gl_PREPROC_O_CLOEXEC.
Replace 'open' if O_CLOEXEC is not present.
* m4/openat.m4 (gl_FUNC_OPENAT): Require gl_PREPROC_O_CLOEXEC.
Replace 'openat' if O_CLOEXEC is not present.
* modules/freopen (Depends-on): Depend on 'open' if replacing freopen.
* modules/open (Files): Add m4/open-cloexec.m4.
(Depends-on): Depend on cloexec if replacing 'open'.
* modules/openat (Files): Add m4/open-cloexec.m4.
(Depends-on): Depend on cloexec if replacing openat.
* modules/popen-safer (Depends-on): Remove cloexec.
* modules/save-cwd (Depends-on): Remove cloexec, and add
fd-safer-flag and 'open'.
---
ChangeLog | 31 +++++++++++++++++++++++++++++++
NEWS | 4 ++++
doc/posix-functions/open.texi | 6 ++++++
doc/posix-functions/openat.texi | 6 ++++++
lib/fcntl.in.h | 5 ++++-
lib/open.c | 29 ++++++++++++++++++++++++++++-
lib/openat.c | 32 ++++++++++++++++++++++++++++++--
lib/popen-safer.c | 35 +----------------------------------
lib/save-cwd.c | 6 ++----
m4/open-cloexec.m4 | 21 +++++++++++++++++++++
m4/open.m4 | 6 +++++-
m4/openat.m4 | 8 +++++---
modules/freopen | 1 +
modules/open | 2 ++
modules/openat | 2 ++
modules/popen-safer | 1 -
modules/save-cwd | 5 +++--
17 files changed, 151 insertions(+), 49 deletions(-)
create mode 100644 m4/open-cloexec.m4
diff --git a/ChangeLog b/ChangeLog
index a07e17251..7f1a5c2e9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,34 @@
+2017-08-14 Paul Eggert <[email protected]>
+
+
+ open: support O_CLOEXEC
+ * NEWS, doc/posix-functions/open.texi:
+ * doc/posix-functions/openat.texi: Document this.
+ * lib/fcntl.in.h (O_CLOEXEC): Default to a nonzero value.
+ (GNULIB_defined_O_CLOEXEC): New symbol.
+ * lib/open.c: Include cloexec.h.
+ (open): Support O_CLOEXEC.
+ * lib/openat.c: Include cloexec.h.
+ (rpl_openat): Support O_CLOEXEC.
+ * lib/popen-safer.c: Do not include cloexec.h.
+ (open_noinherit): Remove.
+ (popen_safer): Use O_CLOEXEC instead of set_cloexec_flag.
+ * lib/save-cwd.c: Do not include cloexec.h.
+ (save_cwd): Use O_CLOEXEC instead of set_cloexec_flag.
+ * m4/open-cloexec.m4: New file.
+ * m4/open.m4 (gl_FUNC_OPEN): Require gl_PREPROC_O_CLOEXEC.
+ Replace 'open' if O_CLOEXEC is not present.
+ * m4/openat.m4 (gl_FUNC_OPENAT): Require gl_PREPROC_O_CLOEXEC.
+ Replace 'openat' if O_CLOEXEC is not present.
+ * modules/freopen (Depends-on): Depend on 'open' if replacing freopen.
+ * modules/open (Files): Add m4/open-cloexec.m4.
+ (Depends-on): Depend on cloexec if replacing 'open'.
+ * modules/openat (Files): Add m4/open-cloexec.m4.
+ (Depends-on): Depend on cloexec if replacing openat.
+ * modules/popen-safer (Depends-on): Remove cloexec.
+ * modules/save-cwd (Depends-on): Remove cloexec, and add
+ fd-safer-flag and 'open'.
+
2017-08-13 Paul Eggert <[email protected]>
reallocarray: minor fixes
diff --git a/NEWS b/NEWS
index 8fb2c54dd..d3a300a30 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,10 @@ User visible incompatible changes
Date Modules Changes
+2017-08-14 fcntl-h This module now defaults O_CLOEXEC to a nonzero
+ value instead of to 0, as the 'open' and
+ 'openat' modules now emulate O_CLOEXEC.
+
2017-07-23 strftime This module is renamed to 'nstrftime'.
2017-05-19 closeout close_stdout longer closes stderr when addresses
diff --git a/doc/posix-functions/open.texi b/doc/posix-functions/open.texi
index b8cda062f..f1565c31a 100644
--- a/doc/posix-functions/open.texi
+++ b/doc/posix-functions/open.texi
@@ -9,6 +9,9 @@ Gnulib module: open, fchdir
Portability problems fixed by the Gnulib module open:
@itemize
@item
+Some platforms do not support @code{O_CLOEXEC}:
+Solaris 10, probably many others.
+@item
On platforms where @code{off_t} is a 32-bit type, @code{open} may not work
correctly with files larger than 2 GB. (Cf. @code{AC_SYS_LARGEFILE}.)
@item
@@ -35,6 +38,9 @@ read-only descriptor for directories.
Portability problems not fixed by Gnulib:
@itemize
@item
+The Gnulib replacement for @code{O_CLOEXEC} is not atomic, and so is
+not safe in the presence of multiple threads or signal handlers.
+@item
@code{open ("symlink", O_NOFOLLOW ...)} fails with @code{errno} set to
@code{EMLINK} instead of the POSIX-required @code{ELOOP} on some
platforms:
diff --git a/doc/posix-functions/openat.texi b/doc/posix-functions/openat.texi
index f93c98c19..9f7632b1b 100644
--- a/doc/posix-functions/openat.texi
+++ b/doc/posix-functions/openat.texi
@@ -14,6 +14,9 @@ glibc 2.3.6, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8,
AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Cygwin 1.5.x, mingw, MSVC 14, Interix 3.5, BeOS.
But the replacement function is not safe to be used in libraries and is not multithread-safe.
@item
+Some platforms do not support @code{O_CLOEXEC}:
+Solaris 10.
+@item
On platforms where @code{off_t} is a 32-bit type, @code{open} may not work
correctly with files larger than 2 GB. (Cf. @code{AC_SYS_LARGEFILE}.)
@item
@@ -26,6 +29,9 @@ Solaris 9.
Portability problems not fixed by Gnulib:
@itemize
@item
+The Gnulib replacement for @code{O_CLOEXEC} is not atomic, and so is
+not safe in the presence of multiple threads or signal handlers.
+@item
@code{openat (fd, "symlink", O_NOFOLLOW ...)} fails with @code{errno}
set to @code{EMLINK} instead of the POSIX-required @code{ELOOP} on
some platforms:
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index 4a1d40af6..076d1ac34 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -213,7 +213,10 @@ _GL_WARN_ON_USE (openat, "openat is not portable - "
#endif
#ifndef O_CLOEXEC
-# define O_CLOEXEC 0
+# define O_CLOEXEC 0x40000000 /* Try to not collide with system O_* flags. */
+# define GNULIB_defined_O_CLOEXEC 1
+#else
+# define GNULIB_defined_O_CLOEXEC 0
#endif
#ifndef O_DIRECT
diff --git a/lib/open.c b/lib/open.c
index 193dc45f6..c62f02b14 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -38,6 +38,8 @@ orig_open (const char *filename, int flags, mode_t mode)
this include because of the preliminary #include <fcntl.h> above. */
#include "fcntl.h"
+#include "cloexec.h"
+
#include <errno.h>
#include <stdarg.h>
#include <string.h>
@@ -52,6 +54,13 @@ orig_open (const char *filename, int flags, mode_t mode)
int
open (const char *filename, int flags, ...)
{
+ /* 0 = unknown, 1 = yes, -1 = no. */
+#if GNULIB_defined_O_CLOEXEC
+ int have_cloexec = -1;
+#else
+ static int have_cloexec;
+#endif
+
mode_t mode;
int fd;
@@ -115,7 +124,25 @@ open (const char *filename, int flags, ...)
}
#endif
- fd = orig_open (filename, flags, mode);
+ fd = orig_open (filename,
+ flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode);
+
+ if (flags & O_CLOEXEC)
+ {
+ if (! have_cloexec)
+ {
+ if (0 <= fd)
+ have_cloexec = 1;
+ else if (errno == EINVAL)
+ {
+ fd = orig_open (filename, flags & ~O_CLOEXEC, mode);
+ have_cloexec = -1;
+ }
+ }
+ if (have_cloexec < 0 && 0 <= fd)
+ set_cloexec_flag (fd, true);
+ }
+
#if REPLACE_FCHDIR
/* Implementing fchdir and fdopendir requires the ability to open a
diff --git a/lib/openat.c b/lib/openat.c
index e739b7143..f39301014 100644
--- a/lib/openat.c
+++ b/lib/openat.c
@@ -41,6 +41,8 @@ orig_openat (int fd, char const *filename, int flags, mode_t mode)
#include "openat.h"
+#include "cloexec.h"
+
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
@@ -50,10 +52,18 @@ orig_openat (int fd, char const *filename, int flags, mode_t mode)
#if HAVE_OPENAT
-/* Like openat, but work around Solaris 9 bugs with trailing slash. */
+/* Like openat, but support O_CLOEXEC and work around Solaris 9 bugs
+ with trailing slash. */
int
rpl_openat (int dfd, char const *filename, int flags, ...)
{
+ /* 0 = unknown, 1 = yes, -1 = no. */
+#if GNULIB_defined_O_CLOEXEC
+ int have_cloexec = -1;
+#else
+ static int have_cloexec;
+#endif
+
mode_t mode;
int fd;
@@ -103,7 +113,25 @@ rpl_openat (int dfd, char const *filename, int flags, ...)
}
# endif
- fd = orig_openat (dfd, filename, flags, mode);
+ fd = orig_openat (dfd, filename,
+ flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode);
+
+ if (flags & O_CLOEXEC)
+ {
+ if (! have_cloexec)
+ {
+ if (0 <= fd)
+ have_cloexec = 1;
+ else if (errno == EINVAL)
+ {
+ fd = orig_openat (dfd, filename, flags & ~O_CLOEXEC, mode);
+ have_cloexec = -1;
+ }
+ }
+ if (have_cloexec < 0 && 0 <= fd)
+ set_cloexec_flag (fd, true);
+ }
+
# if OPEN_TRAILING_SLASH_BUG
/* If the filename ends in a slash and fd does not refer to a directory,
diff --git a/lib/popen-safer.c b/lib/popen-safer.c
index 503780f35..5614f263b 100644
--- a/lib/popen-safer.c
+++ b/lib/popen-safer.c
@@ -25,39 +25,6 @@
#include <fcntl.h>
#include <unistd.h>
-#include "cloexec.h"
-
-/* Like open (name, flags | O_CLOEXEC), although not necessarily
- atomic. FLAGS must not include O_CREAT. */
-
-static int
-open_noinherit (char const *name, int flags)
-{
- int fd;
-#if O_CLOEXEC
- /* 0 = unknown, 1 = yes, -1 = no. */
- static int have_cloexec;
- if (have_cloexec >= 0)
- {
- fd = open (name, flags | O_CLOEXEC);
- if (have_cloexec == 0 && (0 <= fd || errno == EINVAL))
- have_cloexec = (0 <= fd ? 1 : -1);
- if (have_cloexec == 1)
- return fd;
- }
-#endif
-
- fd = open (name, flags);
- if (0 <= fd && set_cloexec_flag (fd, true) != 0)
- {
- int saved_errno = errno;
- close (fd);
- fd = -1;
- errno = saved_errno;
- }
- return fd;
-}
-
/* Like popen, but do not return stdin, stdout, or stderr. */
FILE *
@@ -72,7 +39,7 @@ popen_safer (char const *cmd, char const *mode)
call (even though this puts more pressure on open fds), so that
the original fd created by popen is safe. */
FILE *fp;
- int fd = open_noinherit ("/dev/null", O_RDONLY);
+ int fd = open ("/dev/null", O_RDONLY | O_CLOEXEC);
if (0 <= fd && fd <= STDERR_FILENO)
{
/* Maximum recursion depth is 3. */
diff --git a/lib/save-cwd.c b/lib/save-cwd.c
index 11416cee2..b82a00a01 100644
--- a/lib/save-cwd.c
+++ b/lib/save-cwd.c
@@ -30,7 +30,6 @@
#include "chdir-long.h"
#include "unistd--.h"
-#include "cloexec.h"
#if GNULIB_FCNTL_SAFER
# include "fcntl--.h"
@@ -64,16 +63,15 @@ save_cwd (struct saved_cwd *cwd)
{
cwd->name = NULL;
- cwd->desc = open (".", O_SEARCH);
+ cwd->desc = open (".", O_SEARCH | O_CLOEXEC);
if (!GNULIB_FCNTL_SAFER)
- cwd->desc = fd_safer (cwd->desc);
+ cwd->desc = fd_safer_flag (cwd->desc, O_CLOEXEC);
if (cwd->desc < 0)
{
cwd->name = getcwd (NULL, 0);
return cwd->name ? 0 : -1;
}
- set_cloexec_flag (cwd->desc, true);
return 0;
}
diff --git a/m4/open-cloexec.m4 b/m4/open-cloexec.m4
new file mode 100644
index 000000000..897af6691
--- /dev/null
+++ b/m4/open-cloexec.m4
@@ -0,0 +1,21 @@
+# Test whether O_CLOEXEC is defined.
+
+dnl Copyright 2017 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_PREPROC_O_CLOEXEC],
+[
+ AC_CACHE_CHECK([for O_CLOEXEC],
+ [gl_cv_macro_O_CLOEXEC],
+ [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[#include <fcntl.h>
+ #ifndef O_CLOEXEC
+ choke me;
+ #endif
+ ]],
+ [[return O_CLOEXEC;]])],
+ [gl_cv_macro_O_CLOEXEC=yes],
+ [gl_cv_macro_O_CLOEXEC=no])])
+])
diff --git a/m4/open.m4 b/m4/open.m4
index 2a869dc6b..68253e15f 100644
--- a/m4/open.m4
+++ b/m4/open.m4
@@ -1,4 +1,4 @@
-# open.m4 serial 14
+# open.m4 serial 15
dnl Copyright (C) 2007-2017 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -7,6 +7,7 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_FUNC_OPEN],
[
AC_REQUIRE([AC_CANONICAL_HOST])
+ AC_REQUIRE([gl_PREPROC_O_CLOEXEC])
case "$host_os" in
mingw* | pw*)
REPLACE_OPEN=1
@@ -15,6 +16,9 @@ AC_DEFUN([gl_FUNC_OPEN],
dnl open("foo/") should not create a file when the file name has a
dnl trailing slash. FreeBSD only has the problem on symlinks.
AC_CHECK_FUNCS_ONCE([lstat])
+ if test "$gl_cv_macro_O_CLOEXEC" != yes; then
+ REPLACE_OPEN=1
+ fi
AC_CACHE_CHECK([whether open recognizes a trailing slash],
[gl_cv_func_open_slash],
[# Assume that if we have lstat, we can also check symlinks.
diff --git a/m4/openat.m4 b/m4/openat.m4
index 55b946f96..d0d244a0f 100644
--- a/m4/openat.m4
+++ b/m4/openat.m4
@@ -1,4 +1,4 @@
-# serial 45
+# serial 46
# See if we need to use our replacement for Solaris' openat et al functions.
dnl Copyright (C) 2004-2017 Free Software Foundation, Inc.
@@ -14,10 +14,12 @@ AC_DEFUN([gl_FUNC_OPENAT],
AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
AC_CHECK_FUNCS_ONCE([openat])
AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK])
- case $ac_cv_func_openat+$gl_cv_func_lstat_dereferences_slashed_symlink in
- yes+*yes)
+ AC_REQUIRE([gl_PREPROC_O_CLOEXEC])
+ case $ac_cv_func_openat+$gl_cv_func_lstat_dereferences_slashed_symlink+$gl_cv_macro_O_CLOEXEC in
+ yes+*yes+yes)
;;
yes+*)
+ # Solaris 10 lacks O_CLOEXEC.
# Solaris 9 has *at functions, but uniformly mishandles trailing
# slash in all of them.
REPLACE_OPENAT=1
diff --git a/modules/freopen b/modules/freopen
index adb4bbcbc..41236bd53 100644
--- a/modules/freopen
+++ b/modules/freopen
@@ -7,6 +7,7 @@ m4/freopen.m4
Depends-on:
fcntl-h [test $REPLACE_FREOPEN = 1]
+open [test $REPLACE_FREOPEN = 1]
stdio
largefile
diff --git a/modules/open b/modules/open
index 515170fc3..62c1a0595 100644
--- a/modules/open
+++ b/modules/open
@@ -4,11 +4,13 @@ open() function: open a descriptor to a file.
Files:
lib/open.c
m4/open.m4
+m4/open-cloexec.m4
m4/mode_t.m4
Depends-on:
fcntl-h
largefile
+cloexec [test $REPLACE_OPEN = 1]
fstat [test $REPLACE_OPEN = 1]
stat [test $REPLACE_OPEN = 1]
diff --git a/modules/openat b/modules/openat
index 0db086a76..66fa3b4f4 100644
--- a/modules/openat
+++ b/modules/openat
@@ -4,6 +4,7 @@ openat() function: Open a file at a directory.
Files:
lib/openat.c
m4/openat.m4
+m4/open-cloexec.m4
m4/lstat.m4
m4/mode_t.m4
@@ -14,6 +15,7 @@ largefile
openat-h [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1]
stdbool [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1]
sys_stat [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1]
+cloexec [test $REPLACE_OPENAT = 1]
fstat [test $REPLACE_OPENAT = 1]
at-internal [test $HAVE_OPENAT = 0]
dosname [test $HAVE_OPENAT = 0]
diff --git a/modules/popen-safer b/modules/popen-safer
index 379397632..4c4303704 100644
--- a/modules/popen-safer
+++ b/modules/popen-safer
@@ -7,7 +7,6 @@ lib/stdio-safer.h
lib/popen-safer.c
Depends-on:
-cloexec
open
popen
unistd-safer
diff --git a/modules/save-cwd b/modules/save-cwd
index 876826b0b..52bf3311d 100644
--- a/modules/save-cwd
+++ b/modules/save-cwd
@@ -8,9 +8,10 @@ m4/save-cwd.m4
Depends-on:
chdir-long
-cloexec
-getcwd-lgpl
fchdir
+fd-safer-flag
+getcwd-lgpl
+open
stdbool
unistd-safer
--
2.13.5
