While working on support for mode 'e' in fopen(), I noticed that on macOS 10.13
open with O_CLOEXEC creates a file descriptor that does *not* have the
FD_CLOEXEC flag set.
This was meant to be implemented since 2017-08-14, but it does not work.
The problem is in this code (lib/open.c):
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);
}
At the first invocation:
- have_cloexec is 0,
- thus the flags passed to orig_open() do NOT include O_CLOEXEC,
- thus orig_open() returns an fd >= 0,
- thus have_cloexec gets set to 1.
The detection of a platform without O_CLOEXEC support is not working.
In fact, when I extend the unit test of open(), I see it fail:
$ ./test-open
../../gltests/test-open.h:100: assertion '(flags & FD_CLOEXEC) != 0' failed
Abort trap: 6
$ ./test-openat
../../gltests/test-open.h:100: assertion '(flags & FD_CLOEXEC) != 0' failed
Abort trap: 6
This patch fixes the issue and adds the corresponding unit test.
2020-05-24 Bruno Haible <br...@clisp.org>
open, openat: Really support O_CLOEXEC.
* lib/open.c (open): When have_cloexec is still undecided, do pass a
O_CLOEXEC flag to orig_open.
* lib/openat.c (rpl_openat): When have_cloexec is still undecided, do
pass a O_CLOEXEC flag to orig_openat.
* tests/test-open.h (test_open): Verify that O_CLOEXEC is honoured.
* modules/open-tests (Depends-on): Add fcntl.
* modules/openat-tests (Depends-on): Likewise.
* modules/fcntl-safer-tests (Depends-on): Likewise.
diff --git a/lib/open.c b/lib/open.c
index bb180fd..751b42d 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -124,7 +124,7 @@ open (const char *filename, int flags, ...)
#endif
fd = orig_open (filename,
- flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode);
+ flags & ~(have_cloexec < 0 ? O_CLOEXEC : 0), mode);
if (flags & O_CLOEXEC)
{
diff --git a/lib/openat.c b/lib/openat.c
index 974f1a8..fbe1d2e 100644
--- a/lib/openat.c
+++ b/lib/openat.c
@@ -114,7 +114,7 @@ rpl_openat (int dfd, char const *filename, int flags, ...)
# endif
fd = orig_openat (dfd, filename,
- flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode);
+ flags & ~(have_cloexec < 0 ? O_CLOEXEC : 0), mode);
if (flags & O_CLOEXEC)
{
diff --git a/tests/test-open.h b/tests/test-open.h
index c57054f..862c8df 100644
--- a/tests/test-open.h
+++ b/tests/test-open.h
@@ -88,6 +88,26 @@ test_open (int (*func) (char const *, int, ...), bool print)
ASSERT (0 <= fd);
ASSERT (close (fd) == 0);
+ /* O_CLOEXEC must be honoured. */
+ if (O_CLOEXEC)
+ {
+ /* Since the O_CLOEXEC handling goes through a special code path at its
+ first invocation, test it twice. */
+ int i;
+
+ for (i = 0; i < 2; i++)
+ {
+ int flags;
+
+ fd = func (BASE "file", O_CLOEXEC | O_RDONLY);
+ ASSERT (0 <= fd);
+ flags = fcntl (fd, F_GETFD);
+ ASSERT (flags >= 0);
+ ASSERT ((flags & FD_CLOEXEC) != 0);
+ ASSERT (close (fd) == 0);
+ }
+ }
+
/* Symlink handling, where supported. */
if (symlink (BASE "file", BASE "link") != 0)
{
diff --git a/modules/fcntl-safer-tests b/modules/fcntl-safer-tests
index cb35aed..b967c8a 100644
--- a/modules/fcntl-safer-tests
+++ b/modules/fcntl-safer-tests
@@ -5,6 +5,7 @@ tests/macros.h
Depends-on:
stdbool
+fcntl
symlink
configure.ac:
diff --git a/modules/open-tests b/modules/open-tests
index 5bc4e0f..b2b6710 100644
--- a/modules/open-tests
+++ b/modules/open-tests
@@ -6,6 +6,7 @@ tests/macros.h
Depends-on:
stdbool
+fcntl
symlink
configure.ac:
diff --git a/modules/openat-tests b/modules/openat-tests
index 0b7370d..c4a72f5 100644
--- a/modules/openat-tests
+++ b/modules/openat-tests
@@ -5,6 +5,7 @@ tests/signature.h
tests/macros.h
Depends-on:
+fcntl
symlink
configure.ac:
