* portability of fopen and 'e' (O_CLOEXEC) flag
@ 2020-05-11 14:49 Tim Rühsen
2020-05-11 16:37 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Tim Rühsen @ 2020-05-11 14:49 UTC (permalink / raw)
To: bug-gnulib
[-- Attachment #1.1: Type: text/plain, Size: 405 bytes --]
Hi,
i would like to ask for your expert knowledge.
How to prevent file descriptor leaks in a multi-threaded application
that fork+exec. Quick answer is surely "use O_CLOEXEC" to close those
file descriptors on exec.
But how does this work with fopen in a portable way ?
GNU libc has the 'e' flag for exactly this.
How about other non-GNU OSes / alternative C libraries ?
Regards, Tim
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-11 14:49 portability of fopen and 'e' (O_CLOEXEC) flag Tim Rühsen
@ 2020-05-11 16:37 ` Bruno Haible
2020-05-11 17:41 ` Eric Blake
2020-05-12 8:28 ` Tim Rühsen
0 siblings, 2 replies; 14+ messages in thread
From: Bruno Haible @ 2020-05-11 16:37 UTC (permalink / raw)
To: bug-gnulib; +Cc: Tim Rühsen
Hi Tim,
> i would like to ask for your expert knowledge.
>
> How to prevent file descriptor leaks in a multi-threaded application
> that fork+exec. Quick answer is surely "use O_CLOEXEC" to close those
> file descriptors on exec.
>
> But how does this work with fopen in a portable way ?
> GNU libc has the 'e' flag for exactly this.
Yes [1].
> How about other non-GNU OSes / alternative C libraries ?
POSIX [2][3], macOS [4], FreeBSD [5], Solaris [6] don't support this 'e' flag.
How about using open() with O_CLOEXEC, and then fdopen()?
Bruno
[1] http://man7.org/linux/man-pages/man3/fopen.3.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
[3] http://man7.org/linux/man-pages/man3/fopen.3p.html
[4] https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/fopen.3.html
[5] https://www.freebsd.org/cgi/man.cgi?query=fopen&sektion=3&apropos=0&manpath=freebsd
[6] https://docs.oracle.com/cd/E36784_01/html/E36874/fopen-3c.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-11 16:37 ` Bruno Haible
@ 2020-05-11 17:41 ` Eric Blake
2020-05-12 8:28 ` Tim Rühsen
1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-05-11 17:41 UTC (permalink / raw)
To: Bruno Haible, bug-gnulib; +Cc: Tim Rühsen
On 5/11/20 11:37 AM, Bruno Haible wrote:
>
> POSIX [2][3], macOS [4], FreeBSD [5], Solaris [6] don't support this 'e' flag.
Although it _is_ slated to be added in a future revision of POSIX:
https://www.austingroupbugs.net/view.php?id=411
>
> How about using open() with O_CLOEXEC, and then fdopen()?
In the meantime, this is indeed more portable.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-11 16:37 ` Bruno Haible
2020-05-11 17:41 ` Eric Blake
@ 2020-05-12 8:28 ` Tim Rühsen
2020-05-24 19:22 ` Bruno Haible
1 sibling, 1 reply; 14+ messages in thread
From: Tim Rühsen @ 2020-05-12 8:28 UTC (permalink / raw)
To: Bruno Haible, bug-gnulib
[-- Attachment #1.1: Type: text/plain, Size: 1474 bytes --]
Hi Bruno,
On 11.05.20 18:37, Bruno Haible wrote:
> Hi Tim,
>
>> i would like to ask for your expert knowledge.
>>
>> How to prevent file descriptor leaks in a multi-threaded application
>> that fork+exec. Quick answer is surely "use O_CLOEXEC" to close those
>> file descriptors on exec.
>>
>> But how does this work with fopen in a portable way ?
>> GNU libc has the 'e' flag for exactly this.
>
> Yes [1].
>
>> How about other non-GNU OSes / alternative C libraries ?
>
> POSIX [2][3], macOS [4], FreeBSD [5], Solaris [6] don't support this 'e' flag.
>
> How about using open() with O_CLOEXEC, and then fdopen()?
Thanks. Sure, this is possible. Doing so at dozens of places (or more)
asks for an implementation in gnulib :)
Several projects (most library code) could benefit. I currently think
about GnuTLS where we have to fix this in one or the other way. Updating
gnulib and adding 'e' to the fopen modes would be straight forward.
>
> Bruno
>
> [1] http://man7.org/linux/man-pages/man3/fopen.3.html
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
> [3] http://man7.org/linux/man-pages/man3/fopen.3p.html
> [4] https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/fopen.3.html
> [5] https://www.freebsd.org/cgi/man.cgi?query=fopen&sektion=3&apropos=0&manpath=freebsd
> [6] https://docs.oracle.com/cd/E36784_01/html/E36874/fopen-3c.html
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-12 8:28 ` Tim Rühsen
@ 2020-05-24 19:22 ` Bruno Haible
2020-05-26 6:01 ` Daiki Ueno
0 siblings, 1 reply; 14+ messages in thread
From: Bruno Haible @ 2020-05-24 19:22 UTC (permalink / raw)
To: Tim Rühsen; +Cc: bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]
Tim Rühsen wrote on 2020-05-12:
> > How about using open() with O_CLOEXEC, and then fdopen()?
>
> Thanks. Sure, this is possible. Doing so at dozens of places (or more)
> asks for an implementation in gnulib :)
Additionally, gnulib is the right place to do this because - as Eric said -
the 'e' flag is already scheduled for being added to the next POSIX version:
https://www.austingroupbugs.net/view.php?id=411
My evaluation of current platform support was incorrect: Current versions of
FreeBSD, NetBSD, OpenBSD, Solaris, Cygwin, and even Minix already support it.
2020-05-24 Bruno Haible <bruno@clisp.org>
fopen-gnu: Add tests.
* tests/test-fopen-gnu.c: New file.
* modules/fopen-gnu-tests: New file.
fopen-gnu: New module.
Suggested by Tim Rühsen <tim.ruehsen@gmx.de> in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00119.html>.
* lib/fopen.c (rpl_fopen): When the fopen-gnu module is enabled and the
mode contains an 'x' or 'e' flag, use open() followed by fdopen().
* m4/fopen.m4 (gl_FUNC_FOPEN_GNU): New macro.
* modules/fopen-gnu: New file.
* doc/posix-functions/fopen.texi: Document the 'fopen-gnu' module.
[-- Attachment #2: 0001-fopen-gnu-New-module.patch --]
[-- Type: text/x-patch, Size: 11027 bytes --]
From 10cb4be2a2114dd6fff347acc9841d7904636adf Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 24 May 2020 20:38:53 +0200
Subject: [PATCH 1/2] fopen-gnu: New module.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Suggested by Tim Rühsen <tim.ruehsen@gmx.de> in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00119.html>.
* lib/fopen.c (rpl_fopen): When the fopen-gnu module is enabled and the
mode contains an 'x' or 'e' flag, use open() followed by fdopen().
* m4/fopen.m4 (gl_FUNC_FOPEN_GNU): New macro.
* modules/fopen-gnu: New file.
* doc/posix-functions/fopen.texi: Document the 'fopen-gnu' module.
---
ChangeLog | 11 +++++
doc/posix-functions/fopen.texi | 19 ++++++++-
lib/fopen.c | 92 ++++++++++++++++++++++++++++++++++++++++--
m4/fopen.m4 | 87 ++++++++++++++++++++++++++++++++++++++-
modules/fopen-gnu | 27 +++++++++++++
5 files changed, 229 insertions(+), 7 deletions(-)
create mode 100644 modules/fopen-gnu
diff --git a/ChangeLog b/ChangeLog
index ae9fae4..fab0d87 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2020-05-24 Bruno Haible <bruno@clisp.org>
+ fopen-gnu: New module.
+ Suggested by Tim Rühsen <tim.ruehsen@gmx.de> in
+ <https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00119.html>.
+ * lib/fopen.c (rpl_fopen): When the fopen-gnu module is enabled and the
+ mode contains an 'x' or 'e' flag, use open() followed by fdopen().
+ * m4/fopen.m4 (gl_FUNC_FOPEN_GNU): New macro.
+ * modules/fopen-gnu: New file.
+ * doc/posix-functions/fopen.texi: Document the 'fopen-gnu' module.
+
+2020-05-24 Bruno Haible <bruno@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.
diff --git a/doc/posix-functions/fopen.texi b/doc/posix-functions/fopen.texi
index 308d676..6c562a6 100644
--- a/doc/posix-functions/fopen.texi
+++ b/doc/posix-functions/fopen.texi
@@ -4,9 +4,9 @@
POSIX specification:@* @url{https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html}
-Gnulib module: fopen
+Gnulib module: fopen or fopen-gnu
-Portability problems fixed by Gnulib:
+Portability problems fixed by either Gnulib module @code{fopen} or @code{fopen-gnu}:
@itemize
@item
This function does not fail when the file name argument ends in a slash
@@ -21,6 +21,21 @@ On Windows platforms (excluding Cygwin), this function does usually not
recognize the @file{/dev/null} filename.
@end itemize
+Portability problems fixed by Gnulib module @code{fopen-gnu}:
+@itemize
+@item
+This function does not support the mode character
+@samp{x} (corresponding to @code{O_EXCL}), introduced in ISO C11,
+on some platforms:
+FreeBSD 8.2, NetBSD 6.1, OpenBSD 5.6, Minix 3.2, AIX 6.1, HP-UX 11.31, IRIX 6.5, Solaris 11.3, Cygwin 1.7.16 (2012), mingw, MSVC 14.
+@item
+This function does not support the mode character
+@samp{e} (corresponding to @code{O_CLOEXEC}),
+introduced into a future POSIX revision through
+@url{https://www.austingroupbugs.net/view.php?id=411}, on some platforms:
+glibc 2.6, Mac OS X 10.13, FreeBSD 9.0, NetBSD 5.1, OpenBSD 5.6, Minix 3.2, AIX 7.2, HP-UX 11.31, IRIX 6.5, Solaris 11.3, Cygwin 1.7.16 (2012), mingw, MSVC 14.
+@end itemize
+
Portability problems not fixed by Gnulib:
@itemize
@item
diff --git a/lib/fopen.c b/lib/fopen.c
index ad6511d..20065e4 100644
--- a/lib/fopen.c
+++ b/lib/fopen.c
@@ -49,6 +49,12 @@ rpl_fopen (const char *filename, const char *mode)
{
int open_direction;
int open_flags_standard;
+#if GNULIB_FOPEN_GNU
+ int open_flags_gnu;
+# define BUF_SIZE 80
+ char fdopen_mode_buf[BUF_SIZE + 1];
+#endif
+ int open_flags;
#if defined _WIN32 && ! defined __CYGWIN__
if (strcmp (filename, "/dev/null") == 0)
@@ -58,35 +64,88 @@ rpl_fopen (const char *filename, const char *mode)
/* Parse the mode. */
open_direction = 0;
open_flags_standard = 0;
+#if GNULIB_FOPEN_GNU
+ open_flags_gnu = 0;
+#endif
{
- const char *m;
+ const char *p = mode;
+#if GNULIB_FOPEN_GNU
+ char *q = fdopen_mode_buf;
+#endif
- for (m = mode; *m != '\0'; m++)
+ for (; *p != '\0'; p++)
{
- switch (*m)
+ switch (*p)
{
case 'r':
open_direction = O_RDONLY;
+#if GNULIB_FOPEN_GNU
+ if (q < fdopen_mode_buf + BUF_SIZE)
+ *q++ = *p;
+#endif
continue;
case 'w':
open_direction = O_WRONLY;
open_flags_standard |= O_CREAT | O_TRUNC;
+#if GNULIB_FOPEN_GNU
+ if (q < fdopen_mode_buf + BUF_SIZE)
+ *q++ = *p;
+#endif
continue;
case 'a':
open_direction = O_WRONLY;
open_flags_standard |= O_CREAT | O_APPEND;
+#if GNULIB_FOPEN_GNU
+ if (q < fdopen_mode_buf + BUF_SIZE)
+ *q++ = *p;
+#endif
continue;
case 'b':
+#if GNULIB_FOPEN_GNU
+ if (q < fdopen_mode_buf + BUF_SIZE)
+ *q++ = *p;
+#endif
continue;
case '+':
open_direction = O_RDWR;
+#if GNULIB_FOPEN_GNU
+ if (q < fdopen_mode_buf + BUF_SIZE)
+ *q++ = *p;
+#endif
continue;
+#if GNULIB_FOPEN_GNU
+ case 'x':
+ open_flags_gnu |= O_EXCL;
+ continue;
+ case 'e':
+ open_flags_gnu |= O_CLOEXEC;
+ continue;
+#endif
default:
break;
}
+#if GNULIB_FOPEN_GNU
+ /* The rest of the mode string can be a platform-dependent extension.
+ Copy it unmodified. */
+ {
+ size_t len = strlen (p);
+ if (len > fdopen_mode_buf + BUF_SIZE - q)
+ len = fdopen_mode_buf + BUF_SIZE - q;
+ memcpy (q, p, len);
+ q += len;
+ }
+#endif
break;
}
+#if GNULIB_FOPEN_GNU
+ *q = '\0';
+#endif
}
+#if GNULIB_FOPEN_GNU
+ open_flags = open_flags_standard | open_flags_gnu;
+#else
+ open_flags = open_flags_standard;
+#endif
#if FOPEN_TRAILING_SLASH_BUG
/* Fail if the mode requires write access and the filename ends in a slash,
@@ -116,7 +175,7 @@ rpl_fopen (const char *filename, const char *mode)
return NULL;
}
- fd = open (filename, open_direction | open_flags_standard);
+ fd = open (filename, open_direction | open_flags);
if (fd < 0)
return NULL;
@@ -127,7 +186,11 @@ rpl_fopen (const char *filename, const char *mode)
return NULL;
}
+# if GNULIB_FOPEN_GNU
+ fp = fdopen (fd, fdopen_mode_buf);
+# else
fp = fdopen (fd, mode);
+# endif
if (fp == NULL)
{
int saved_errno = errno;
@@ -139,5 +202,26 @@ rpl_fopen (const char *filename, const char *mode)
}
#endif
+#if GNULIB_FOPEN_GNU
+ if (open_flags_gnu != 0)
+ {
+ int fd;
+ FILE *fp;
+
+ fd = open (filename, open_direction | open_flags);
+ if (fd < 0)
+ return NULL;
+
+ fp = fdopen (fd, fdopen_mode_buf);
+ if (fp == NULL)
+ {
+ int saved_errno = errno;
+ close (fd);
+ errno = saved_errno;
+ }
+ return fp;
+ }
+#endif
+
return orig_fopen (filename, mode);
}
diff --git a/m4/fopen.m4 b/m4/fopen.m4
index 2a4b00d..8eab4a6 100644
--- a/m4/fopen.m4
+++ b/m4/fopen.m4
@@ -1,4 +1,4 @@
-# fopen.m4 serial 10
+# fopen.m4 serial 11
dnl Copyright (C) 2007-2020 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -58,5 +58,90 @@ changequote([,])dnl
esac
])
+AC_DEFUN([gl_FUNC_FOPEN_GNU],
+[
+ AC_REQUIRE([gl_FUNC_FOPEN])
+ AC_CACHE_CHECK([whether fopen supports the mode character 'x'],
+ [gl_cv_func_fopen_mode_x],
+ [rm -f conftest.x
+ AC_RUN_IFELSE(
+ [AC_LANG_SOURCE([[
+#include <stdio.h>
+#include <errno.h>
+int main ()
+{
+ FILE *fp;
+ fp = fopen ("conftest.x", "w");
+ fclose (fp);
+ fp = fopen ("conftest.x", "wx");
+ if (fp != NULL)
+ /* 'x' ignored */
+ return 1;
+ else if (errno == EEXIST)
+ return 0;
+ else
+ /* 'x' rejected */
+ return 2;
+}]])],
+ [gl_cv_func_fopen_mode_x=yes],
+ [gl_cv_func_fopen_mode_x=no],
+ [case "$host_os" in
+ # Guess yes on glibc and musl systems.
+ linux*-gnu* | gnu* | kfreebsd*-gnu | *-musl*)
+ gl_cv_func_fopen_mode_x="guessing yes" ;;
+ # If we don't know, obey --enable-cross-guesses.
+ *)
+ gl_cv_func_fopen_mode_x="$gl_cross_guess_normal" ;;
+ esac
+ ])
+ rm -f conftest.x
+ ])
+ AC_CACHE_CHECK([whether fopen supports the mode character 'e'],
+ [gl_cv_func_fopen_mode_e],
+ [echo foo > conftest.x
+ AC_RUN_IFELSE(
+ [AC_LANG_SOURCE([[
+#include <stdio.h>
+#include <errno.h>
+#include <fcntl.h>
+int main ()
+{
+ FILE *fp = fopen ("conftest.x", "re");
+ if (fp != NULL)
+ {
+ if (fcntl (fileno (fp), F_GETFD) & FD_CLOEXEC)
+ return 0;
+ else
+ /* 'e' ignored */
+ return 1;
+ }
+ else
+ /* 'e' rejected */
+ return 2;
+}]])],
+ [gl_cv_func_fopen_mode_e=yes],
+ [gl_cv_func_fopen_mode_e=no],
+ [case "$host_os" in
+ # Guess yes on glibc and musl systems.
+ linux*-gnu* | gnu* | kfreebsd*-gnu | *-musl*)
+ gl_cv_func_fopen_mode_e="guessing yes" ;;
+ # Guess no on native Windows.
+ mingw*)
+ gl_cv_func_fopen_mode_e="guessing no" ;;
+ # If we don't know, obey --enable-cross-guesses.
+ *)
+ gl_cv_func_fopen_mode_e="$gl_cross_guess_normal" ;;
+ esac
+ ])
+ rm -f conftest.x
+ ])
+ case "$gl_cv_func_fopen_mode_x" in
+ *no) REPLACE_FOPEN=1 ;;
+ esac
+ case "$gl_cv_func_fopen_mode_e" in
+ *no) REPLACE_FOPEN=1 ;;
+ esac
+])
+
# Prerequisites of lib/fopen.c.
AC_DEFUN([gl_PREREQ_FOPEN], [:])
diff --git a/modules/fopen-gnu b/modules/fopen-gnu
new file mode 100644
index 0000000..f0f2054
--- /dev/null
+++ b/modules/fopen-gnu
@@ -0,0 +1,27 @@
+Description:
+fopen() function: open a stream to a file, with GNU extensions.
+
+Files:
+
+Depends-on:
+fopen
+open [test $REPLACE_FOPEN = 1]
+
+configure.ac:
+gl_FUNC_FOPEN_GNU
+if test $REPLACE_FOPEN = 1; then
+ AC_LIBOBJ([fopen])
+ gl_PREREQ_FOPEN
+fi
+gl_MODULE_INDICATOR([fopen-gnu])
+
+Makefile.am:
+
+Include:
+<stdio.h>
+
+License:
+LGPLv2+
+
+Maintainer:
+all
--
2.7.4
[-- Attachment #3: 0002-fopen-gnu-Add-tests.patch --]
[-- Type: text/x-patch, Size: 3446 bytes --]
From 48888b25625ddcdcc296e4cc1b0cd13e417e54f0 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 24 May 2020 20:40:01 +0200
Subject: [PATCH 2/2] fopen-gnu: Add tests.
* tests/test-fopen-gnu.c: New file.
* modules/fopen-gnu-tests: New file.
---
ChangeLog | 4 +++
modules/fopen-gnu-tests | 12 +++++++++
tests/test-fopen-gnu.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 modules/fopen-gnu-tests
create mode 100644 tests/test-fopen-gnu.c
diff --git a/ChangeLog b/ChangeLog
index fab0d87..d7c1e8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2020-05-24 Bruno Haible <bruno@clisp.org>
+ fopen-gnu: Add tests.
+ * tests/test-fopen-gnu.c: New file.
+ * modules/fopen-gnu-tests: New file.
+
fopen-gnu: New module.
Suggested by Tim Rühsen <tim.ruehsen@gmx.de> in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00119.html>.
diff --git a/modules/fopen-gnu-tests b/modules/fopen-gnu-tests
new file mode 100644
index 0000000..9e4c4ca
--- /dev/null
+++ b/modules/fopen-gnu-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-fopen-gnu.c
+tests/macros.h
+
+Depends-on:
+fcntl
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fopen-gnu
+check_PROGRAMS += test-fopen-gnu
diff --git a/tests/test-fopen-gnu.c b/tests/test-fopen-gnu.c
new file mode 100644
index 0000000..cae4042
--- /dev/null
+++ b/tests/test-fopen-gnu.c
@@ -0,0 +1,71 @@
+/* Test of opening a file stream.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <https://www.gnu.org/licenses/>. */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2020. */
+
+#include <config.h>
+
+/* Specification. */
+#include <stdio.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "macros.h"
+
+#define BASE "test-fopen-gnu.t"
+
+int
+main (void)
+{
+ FILE *f;
+ int fd;
+ int flags;
+
+ /* Remove anything from prior partial run. */
+ unlink (BASE "file");
+
+ /* Create the file. */
+ f = fopen (BASE "file", "w");
+ ASSERT (f);
+ fd = fileno (f);
+ ASSERT (fd >= 0);
+ flags = fcntl (fd, F_GETFD);
+ ASSERT (flags >= 0);
+ ASSERT ((flags & FD_CLOEXEC) == 0);
+ ASSERT (fclose (f) == 0);
+
+ /* Create the file and check the 'e' mode. */
+ f = fopen (BASE "file", "we");
+ ASSERT (f);
+ fd = fileno (f);
+ ASSERT (fd >= 0);
+ flags = fcntl (fd, F_GETFD);
+ ASSERT (flags >= 0);
+ ASSERT ((flags & FD_CLOEXEC) != 0);
+ ASSERT (fclose (f) == 0);
+
+ /* Open the file and check the 'x' mode. */
+ f = fopen (BASE "file", "ax");
+ ASSERT (f == NULL);
+ ASSERT (errno == EEXIST);
+
+ /* Cleanup. */
+ ASSERT (unlink (BASE "file") == 0);
+
+ return 0;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-24 19:22 ` Bruno Haible
@ 2020-05-26 6:01 ` Daiki Ueno
2020-05-26 8:18 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Daiki Ueno @ 2020-05-26 6:01 UTC (permalink / raw)
To: Bruno Haible; +Cc: Tim Rühsen, bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 539 bytes --]
Hello Bruno,
Bruno Haible <bruno@clisp.org> writes:
> Additionally, gnulib is the right place to do this because - as Eric said -
> the 'e' flag is already scheduled for being added to the next POSIX version:
> https://www.austingroupbugs.net/view.php?id=411
>
> My evaluation of current platform support was incorrect: Current versions of
> FreeBSD, NetBSD, OpenBSD, Solaris, Cygwin, and even Minix already support it.
Thank you for this; would it make sense to use it in the modules that do
one-shot fopen/fclose, such as read-file?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-read-file-make-use-of-fopen-gnu.patch --]
[-- Type: text/x-patch, Size: 1861 bytes --]
From 5de739d03a7da71127b771cc213c873cd711ce51 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Tue, 26 May 2020 07:56:13 +0200
Subject: [PATCH] read-file: make use of fopen-gnu
* lib/read-file.c (read_file): Pass an 'e' flag to fopen.
(read_binary_file): Likewise.
* modules/read-file (Depends-on): Add fopen-gnu.
---
ChangeLog | 7 +++++++
lib/read-file.c | 4 ++--
modules/read-file | 1 +
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b1acb99ca..07d4d5124 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2020-05-26 Daiki Ueno <ueno@gnu.org>
+
+ read-file: make use of fopen-gnu
+ * lib/read-file.c (read_file): Pass an 'e' flag to fopen.
+ (read_binary_file): Likewise.
+ * modules/read-file (Depends-on): Add fopen-gnu.
+
2020-05-25 Paul Eggert <eggert@cs.ucla.edu>
getentropy, getrandom: new modules
diff --git a/lib/read-file.c b/lib/read-file.c
index c6f230178..293bc3e8a 100644
--- a/lib/read-file.c
+++ b/lib/read-file.c
@@ -171,7 +171,7 @@ internal_read_file (const char *filename, size_t *length, const char *mode)
char *
read_file (const char *filename, size_t *length)
{
- return internal_read_file (filename, length, "r");
+ return internal_read_file (filename, length, "re");
}
/* Open (on non-POSIX systems, in binary mode) and read the contents
@@ -184,5 +184,5 @@ read_file (const char *filename, size_t *length)
char *
read_binary_file (const char *filename, size_t *length)
{
- return internal_read_file (filename, length, "rb");
+ return internal_read_file (filename, length, "rbe");
}
diff --git a/modules/read-file b/modules/read-file
index 506e88f0a..a6e7faf0a 100644
--- a/modules/read-file
+++ b/modules/read-file
@@ -7,6 +7,7 @@ lib/read-file.c
m4/read-file.m4
Depends-on:
+fopen-gnu
fstat
ftello
malloc-posix
--
2.26.2
[-- Attachment #3: Type: text/plain, Size: 25 bytes --]
Regards,
--
Daiki Ueno
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-26 6:01 ` Daiki Ueno
@ 2020-05-26 8:18 ` Bruno Haible
2020-05-26 16:09 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Bruno Haible @ 2020-05-26 8:18 UTC (permalink / raw)
To: Daiki Ueno; +Cc: Tim Rühsen, bug-gnulib
Hi Daiki,
> Thank you for this; would it make sense to use it in the modules that do
> one-shot fopen/fclose, such as read-file?
This would improve things for multithreaded programs that call exec or
posix_spawn. There are not that many programs of this kind. But on the other
hand, multithreading is meant to be a "built-in" functionality nowadays,
and the burden of fopen-gnu is zero on the majority of the modern platforms
and small on the other platforms.
=> I've applied your patch.
Bruno
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-26 8:18 ` Bruno Haible
@ 2020-05-26 16:09 ` Bruno Haible
2020-05-27 18:39 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Bruno Haible @ 2020-05-26 16:09 UTC (permalink / raw)
To: Daiki Ueno; +Cc: Tim Rühsen, bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 345 bytes --]
Hi Daiki,
> > Thank you for this; would it make sense to use it in the modules that do
> > one-shot fopen/fclose, such as read-file?
Here are proposed patches for other modules. Does this look right?
Note: The patch to sethostname.c is only relevant for Minix. Minix does not
have multithreading now, but a future version likely will.
Bruno
[-- Attachment #2: 0001-bitset-Make-more-robust-in-multithreaded-application.patch --]
[-- Type: text/x-patch, Size: 1902 bytes --]
From acb3be326fbbeaf82af33fbd7bb7b3cb180d2616 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 17:51:03 +0200
Subject: [PATCH 1/8] bitset: Make more robust in multithreaded applications.
* lib/bitset/stats.c (bitset_stats_read, bitset_stats_write): Pass an
'e' flag to fopen.
* modules/bitset (Depends-on): Add fopen-gnu.
---
ChangeLog | 7 +++++++
lib/bitset/stats.c | 4 ++--
modules/bitset | 1 +
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 07d4d51..ca9fbb3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
+ bitset: Make more robust in multithreaded applications.
+ * lib/bitset/stats.c (bitset_stats_read, bitset_stats_write): Pass an
+ 'e' flag to fopen.
+ * modules/bitset (Depends-on): Add fopen-gnu.
+
2020-05-26 Daiki Ueno <ueno@gnu.org>
read-file: make use of fopen-gnu
diff --git a/lib/bitset/stats.c b/lib/bitset/stats.c
index 10aa5d7..5bd44c0 100644
--- a/lib/bitset/stats.c
+++ b/lib/bitset/stats.c
@@ -245,7 +245,7 @@ bitset_stats_read (const char *file_name)
if (!file_name)
file_name = BITSET_STATS_FILE;
- FILE *file = fopen (file_name, "r");
+ FILE *file = fopen (file_name, "re");
if (file)
{
if (fread (&bitset_stats_info_data, sizeof (bitset_stats_info_data),
@@ -273,7 +273,7 @@ bitset_stats_write (const char *file_name)
if (!file_name)
file_name = BITSET_STATS_FILE;
- FILE *file = fopen (file_name, "w");
+ FILE *file = fopen (file_name, "we");
if (file)
{
if (fwrite (&bitset_stats_info_data, sizeof (bitset_stats_info_data),
diff --git a/modules/bitset b/modules/bitset
index ec7f34b..20c6806 100644
--- a/modules/bitset
+++ b/modules/bitset
@@ -19,6 +19,7 @@ lib/bitset/vector.h
Depends-on:
attribute
c99
+fopen-gnu
gettext-h
obstack
xalloc
--
2.7.4
[-- Attachment #3: 0002-exclude-Make-more-robust-in-multithreaded-applicatio.patch --]
[-- Type: text/x-patch, Size: 1658 bytes --]
From 67d0dc62291be365765dac3fb9e8c006d8427097 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 17:52:23 +0200
Subject: [PATCH 2/8] exclude: Make more robust in multithreaded applications.
* lib/exclude.c (add_exclude_file): Pass an 'e' flag to fopen.
* modules/exclude (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/exclude.c | 2 +-
modules/exclude | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index ca9fbb3..e766479 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ exclude: Make more robust in multithreaded applications.
+ * lib/exclude.c (add_exclude_file): Pass an 'e' flag to fopen.
+ * modules/exclude (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
bitset: Make more robust in multithreaded applications.
* lib/bitset/stats.c (bitset_stats_read, bitset_stats_write): Pass an
'e' flag to fopen.
diff --git a/lib/exclude.c b/lib/exclude.c
index c63c004..2b57a9b 100644
--- a/lib/exclude.c
+++ b/lib/exclude.c
@@ -683,7 +683,7 @@ add_exclude_file (void (*add_func) (struct exclude *, char const *, int),
if (use_stdin)
in = stdin;
- else if (! (in = fopen (file_name, "r")))
+ else if (! (in = fopen (file_name, "re")))
return -1;
rc = add_exclude_fp (call_addfn, ex, in, options, line_end, &add_func);
diff --git a/modules/exclude b/modules/exclude
index 5ff0539..8fe2708 100644
--- a/modules/exclude
+++ b/modules/exclude
@@ -8,6 +8,7 @@ lib/exclude.c
Depends-on:
filename
fnmatch
+fopen-gnu
hash
mbscasecmp
mbuiter
--
2.7.4
[-- Attachment #4: 0003-getloadavg-Make-more-robust-in-multithreaded-applica.patch --]
[-- Type: text/x-patch, Size: 1651 bytes --]
From 350c6f8dba8cc2a6266cb8a350d01c49805af9db Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 17:53:47 +0200
Subject: [PATCH 3/8] getloadavg: Make more robust in multithreaded
applications.
* lib/getloadavg.c (getloadavg): Pass an 'e' flag to fopen.
* modules/getloadavg (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/getloadavg.c | 2 +-
modules/getloadavg | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index e766479..7dabc5e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ getloadavg: Make more robust in multithreaded applications.
+ * lib/getloadavg.c (getloadavg): Pass an 'e' flag to fopen.
+ * modules/getloadavg (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
exclude: Make more robust in multithreaded applications.
* lib/exclude.c (add_exclude_file): Pass an 'e' flag to fopen.
* modules/exclude (Depends-on): Add fopen-gnu.
diff --git a/lib/getloadavg.c b/lib/getloadavg.c
index ebb6f5d..7e11c32 100644
--- a/lib/getloadavg.c
+++ b/lib/getloadavg.c
@@ -569,7 +569,7 @@ getloadavg (double loadavg[], int nelem)
int count;
FILE *fp;
- fp = fopen (NETBSD_LDAV_FILE, "r");
+ fp = fopen (NETBSD_LDAV_FILE, "re");
if (fp == NULL)
return -1;
count = fscanf (fp, "%lu %lu %lu %lu\n",
diff --git a/modules/getloadavg b/modules/getloadavg
index 1b0f581..e14ddc8 100644
--- a/modules/getloadavg
+++ b/modules/getloadavg
@@ -7,6 +7,7 @@ m4/getloadavg.m4
Depends-on:
extensions
+fopen-gnu
intprops
stdbool
stdlib
--
2.7.4
[-- Attachment #5: 0004-getpass-Make-more-robust-in-multithreaded-applicatio.patch --]
[-- Type: text/x-patch, Size: 1639 bytes --]
From 54e5d001797c755994968a34259bae745ee515e6 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 17:56:31 +0200
Subject: [PATCH 4/8] getpass: Make more robust in multithreaded applications.
* lib/getpass.c (getpass): Pass an 'e' flag to fopen.
* modules/getpass (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/getpass.c | 2 +-
modules/getpass | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 7dabc5e..93d8b5a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ getpass: Make more robust in multithreaded applications.
+ * lib/getpass.c (getpass): Pass an 'e' flag to fopen.
+ * modules/getpass (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
getloadavg: Make more robust in multithreaded applications.
* lib/getloadavg.c (getloadavg): Pass an 'e' flag to fopen.
* modules/getloadavg (Depends-on): Add fopen-gnu.
diff --git a/lib/getpass.c b/lib/getpass.c
index af8d72e..3b0552e 100644
--- a/lib/getpass.c
+++ b/lib/getpass.c
@@ -96,7 +96,7 @@ getpass (const char *prompt)
/* Try to write to and read from the terminal if we can.
If we can't open the terminal, use stderr and stdin. */
- tty = fopen ("/dev/tty", "w+");
+ tty = fopen ("/dev/tty", "w+e");
if (tty == NULL)
{
in = stdin;
diff --git a/modules/getpass b/modules/getpass
index 7e73451..74d781a 100644
--- a/modules/getpass
+++ b/modules/getpass
@@ -9,6 +9,7 @@ m4/getpass.m4
Depends-on:
unistd
extensions
+fopen-gnu
fseeko
getline
stdbool
--
2.7.4
[-- Attachment #6: 0005-readutmp-Make-more-robust-in-multithreaded-applicati.patch --]
[-- Type: text/x-patch, Size: 1613 bytes --]
From 56bcba9b1013176d52f1b25a3085695a3bd8d9b8 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 17:57:58 +0200
Subject: [PATCH 5/8] readutmp: Make more robust in multithreaded applications.
* lib/readutmp.c (read_utmp): Pass an 'e' flag to fopen.
* modules/readutmp (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/readutmp.c | 2 +-
modules/readutmp | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 93d8b5a..893d080 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ readutmp: Make more robust in multithreaded applications.
+ * lib/readutmp.c (read_utmp): Pass an 'e' flag to fopen.
+ * modules/readutmp (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
getpass: Make more robust in multithreaded applications.
* lib/getpass.c (getpass): Pass an 'e' flag to fopen.
* modules/getpass (Depends-on): Add fopen-gnu.
diff --git a/lib/readutmp.c b/lib/readutmp.c
index 308390d..793d480 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -132,7 +132,7 @@ read_utmp (char const *file, size_t *n_entries, STRUCT_UTMP **utmp_buf,
size_t n_alloc = 0;
STRUCT_UTMP *utmp = NULL;
int saved_errno;
- FILE *f = fopen (file, "r");
+ FILE *f = fopen (file, "re");
if (! f)
return -1;
diff --git a/modules/readutmp b/modules/readutmp
index 51f6290..e88897c 100644
--- a/modules/readutmp
+++ b/modules/readutmp
@@ -11,6 +11,7 @@ extensions
xalloc
stdbool
stdint
+fopen-gnu
configure.ac:
gl_READUTMP
--
2.7.4
[-- Attachment #7: 0006-sethostname-Make-more-robust-in-multithreaded-applic.patch --]
[-- Type: text/x-patch, Size: 1918 bytes --]
From acdcd9f872f5338f901a9019627953dcb5e8d95f Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 18:00:04 +0200
Subject: [PATCH 6/8] sethostname: Make more robust in multithreaded
applications.
* lib/sethostname.c (sethostname): Pass an 'e' flag to fopen.
* modules/sethostname (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/sethostname.c | 2 +-
modules/sethostname | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 893d080..162b861 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ sethostname: Make more robust in multithreaded applications.
+ * lib/sethostname.c (sethostname): Pass an 'e' flag to fopen.
+ * modules/sethostname (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
readutmp: Make more robust in multithreaded applications.
* lib/readutmp.c (read_utmp): Pass an 'e' flag to fopen.
* modules/readutmp (Depends-on): Add fopen-gnu.
diff --git a/lib/sethostname.c b/lib/sethostname.c
index 87b3af9..1be69be 100644
--- a/lib/sethostname.c
+++ b/lib/sethostname.c
@@ -52,7 +52,7 @@ sethostname (const char *name, size_t len)
these are appropriate for us to set, even if they may match the
situation, during failed open/write/close operations, so we
leave errno alone and rely on what the system sets up. */
- hostf = fopen ("/etc/hostname.file", "w");
+ hostf = fopen ("/etc/hostname.file", "we");
if (hostf == NULL)
r = -1;
else
diff --git a/modules/sethostname b/modules/sethostname
index f3c0d13..92b4a2b 100644
--- a/modules/sethostname
+++ b/modules/sethostname
@@ -9,6 +9,7 @@ m4/gethostname.m4
Depends-on:
unistd
errno [test $HAVE_SETHOSTNAME = 0]
+fopen-gnu [test $HAVE_SETHOSTNAME = 0]
configure.ac:
gl_FUNC_SETHOSTNAME
--
2.7.4
[-- Attachment #8: 0007-mountlist-Make-more-robust-in-multithreaded-applicat.patch --]
[-- Type: text/x-patch, Size: 2788 bytes --]
From 7fac4fbbd7203d2fcf3be257872e3c7508b86402 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 18:04:26 +0200
Subject: [PATCH 7/8] mountlist: Make more robust in multithreaded
applications.
* lib/mountlist.c (setmntent, read_file_system_list): Pass an 'e' flag
to fopen.
* modules/mountlist (Depends-on): Add fopen-gnu.
---
ChangeLog | 7 +++++++
lib/mountlist.c | 10 +++++-----
modules/mountlist | 1 +
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 162b861..b30f4b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ mountlist: Make more robust in multithreaded applications.
+ * lib/mountlist.c (setmntent, read_file_system_list): Pass an 'e' flag
+ to fopen.
+ * modules/mountlist (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
sethostname: Make more robust in multithreaded applications.
* lib/sethostname.c (sethostname): Pass an 'e' flag to fopen.
* modules/sethostname (Depends-on): Add fopen-gnu.
diff --git a/lib/mountlist.c b/lib/mountlist.c
index 7abe024..4cb19c8 100644
--- a/lib/mountlist.c
+++ b/lib/mountlist.c
@@ -125,7 +125,7 @@
#ifdef MOUNTED_GETMNTENT1
# if !HAVE_SETMNTENT /* Android <= 4.4 */
-# define setmntent(fp,mode) fopen (fp, mode)
+# define setmntent(fp,mode) fopen (fp, mode "e")
# endif
# if !HAVE_ENDMNTENT /* Android <= 4.4 */
# define endmntent(fp) fclose (fp)
@@ -460,7 +460,7 @@ read_file_system_list (bool need_fs_type)
(and that code is in previous versions of this function), however
libmount depends on libselinux which pulls in many dependencies. */
char const *mountinfo = "/proc/self/mountinfo";
- fp = fopen (mountinfo, "r");
+ fp = fopen (mountinfo, "re");
if (fp != NULL)
{
char *line = NULL;
@@ -794,7 +794,7 @@ read_file_system_list (bool need_fs_type)
char *table = "/etc/mnttab";
FILE *fp;
- fp = fopen (table, "r");
+ fp = fopen (table, "re");
if (fp == NULL)
return NULL;
@@ -852,7 +852,7 @@ read_file_system_list (bool need_fs_type)
by the kernel. */
errno = 0;
- fp = fopen (table, "r");
+ fp = fopen (table, "re");
if (fp == NULL)
ret = errno;
else
@@ -924,7 +924,7 @@ read_file_system_list (bool need_fs_type)
# endif
errno = 0;
- fp = fopen (table, "r");
+ fp = fopen (table, "re");
if (fp == NULL)
ret = errno;
else
diff --git a/modules/mountlist b/modules/mountlist
index 11cf809..5bb45ed 100644
--- a/modules/mountlist
+++ b/modules/mountlist
@@ -8,6 +8,7 @@ m4/fstypename.m4
m4/mountlist.m4
Depends-on:
+fopen-gnu
getline
stdbool
stdint
--
2.7.4
[-- Attachment #9: 0008-javacomp-Make-more-robust-in-multithreaded-applicati.patch --]
[-- Type: text/x-patch, Size: 1676 bytes --]
From 0f0f1ad56a75eb04e091ba84a6707b4431eeb68e Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Tue, 26 May 2020 18:05:34 +0200
Subject: [PATCH 8/8] javacomp: Make more robust in multithreaded applications.
* lib/javacomp.c (write_temp_file): Pass an 'e' flag to fopen_temp.
* modules/javacomp (Depends-on): Add fopen-gnu.
---
ChangeLog | 6 ++++++
lib/javacomp.c | 2 +-
modules/javacomp | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index b30f4b1..b95b945 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2020-05-26 Bruno Haible <bruno@clisp.org>
+ javacomp: Make more robust in multithreaded applications.
+ * lib/javacomp.c (write_temp_file): Pass an 'e' flag to fopen_temp.
+ * modules/javacomp (Depends-on): Add fopen-gnu.
+
+2020-05-26 Bruno Haible <bruno@clisp.org>
+
mountlist: Make more robust in multithreaded applications.
* lib/mountlist.c (setmntent, read_file_system_list): Pass an 'e' flag
to fopen.
diff --git a/lib/javacomp.c b/lib/javacomp.c
index 9239616..ac56196 100644
--- a/lib/javacomp.c
+++ b/lib/javacomp.c
@@ -573,7 +573,7 @@ write_temp_file (struct temp_dir *tmpdir, const char *file_name,
FILE *fp;
register_temp_file (tmpdir, file_name);
- fp = fopen_temp (file_name, "w");
+ fp = fopen_temp (file_name, "we");
if (fp == NULL)
{
error (0, errno, _("failed to create \"%s\""), file_name);
diff --git a/modules/javacomp b/modules/javacomp
index 4f49bd9..fccdaac 100644
--- a/modules/javacomp
+++ b/modules/javacomp
@@ -22,6 +22,7 @@ xmalloca
getline
xconcat-filename
fwriteerror
+fopen-gnu
clean-temp
stat
error
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-26 16:09 ` Bruno Haible
@ 2020-05-27 18:39 ` Bruno Haible
2020-05-28 10:00 ` Daiki Ueno
0 siblings, 1 reply; 14+ messages in thread
From: Bruno Haible @ 2020-05-27 18:39 UTC (permalink / raw)
To: Daiki Ueno; +Cc: Tim Rühsen, bug-gnulib
> Here are proposed patches for other modules. Does this look right?
There were no objections. I pushed the changes.
Bruno
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-27 18:39 ` Bruno Haible
@ 2020-05-28 10:00 ` Daiki Ueno
2020-05-28 10:04 ` Daiki Ueno
0 siblings, 1 reply; 14+ messages in thread
From: Daiki Ueno @ 2020-05-28 10:00 UTC (permalink / raw)
To: Bruno Haible; +Cc: Tim Rühsen, bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 490 bytes --]
Bruno Haible <bruno@clisp.org> writes:
>> Here are proposed patches for other modules. Does this look right?
>
> There were no objections. I pushed the changes.
Thank you for this. I have rebased GnuTLS on top of it, but noticed a
strange test failures on Windows CI, which involve reading binary files
(OCSP response):
https://gitlab.com/gnutls/gnutls/-/jobs/569815031
It seems that the fopen module ignores a 'b' flag. The attached patch
fixes the failures.
Regards,
--
Daiki Ueno
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fopen-gnu-make-b-flag-can-be-used-with-e-on-Windows.patch --]
[-- Type: text/x-patch, Size: 3437 bytes --]
From 17fbb2560a05e3006125f8793c8e814ef5baa847 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Thu, 28 May 2020 11:40:49 +0200
Subject: [PATCH] fopen-gnu: make 'b' flag can be used with 'e' on Windows
* lib/fopen.c (rpl_fopen): Pass O_BINARY to open, if a 'b' flag is
specified on Windows.
* tests/test-fopen-gnu.c (DATA): New define.
(main): Add test for reading binary files with an 'e' flag.
---
ChangeLog | 8 ++++++++
lib/fopen.c | 9 +++++++--
tests/test-fopen-gnu.c | 17 +++++++++++++++++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c17b76b72..ea2716b2f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-28 Daiki Ueno <ueno@gnu.org>
+
+ fopen-gnu: make 'b' flag can be used with 'e' on Windows
+ * lib/fopen.c (rpl_fopen): Pass O_BINARY to open, if a 'b' flag is
+ specified on Windows.
+ * tests/test-fopen-gnu.c (DATA): New define.
+ (main): Add test for reading binary files with an 'e' flag.
+
2020-05-27 Bruno Haible <bruno@clisp.org>
Don't assume that UNICODE is not defined.
diff --git a/lib/fopen.c b/lib/fopen.c
index 20065e4c6..f60c51f95 100644
--- a/lib/fopen.c
+++ b/lib/fopen.c
@@ -51,6 +51,7 @@ rpl_fopen (const char *filename, const char *mode)
int open_flags_standard;
#if GNULIB_FOPEN_GNU
int open_flags_gnu;
+ int open_flags_other;
# define BUF_SIZE 80
char fdopen_mode_buf[BUF_SIZE + 1];
#endif
@@ -66,6 +67,7 @@ rpl_fopen (const char *filename, const char *mode)
open_flags_standard = 0;
#if GNULIB_FOPEN_GNU
open_flags_gnu = 0;
+ open_flags_other = 0;
#endif
{
const char *p = mode;
@@ -101,6 +103,9 @@ rpl_fopen (const char *filename, const char *mode)
#endif
continue;
case 'b':
+#if defined _WIN32 && ! defined __CYGWIN__
+ open_flags_other |= O_BINARY;
+#endif
#if GNULIB_FOPEN_GNU
if (q < fdopen_mode_buf + BUF_SIZE)
*q++ = *p;
@@ -142,9 +147,9 @@ rpl_fopen (const char *filename, const char *mode)
#endif
}
#if GNULIB_FOPEN_GNU
- open_flags = open_flags_standard | open_flags_gnu;
+ open_flags = open_flags_standard | open_flags_other | open_flags_gnu;
#else
- open_flags = open_flags_standard;
+ open_flags = open_flags_standard | open_flags_other;
#endif
#if FOPEN_TRAILING_SLASH_BUG
diff --git a/tests/test-fopen-gnu.c b/tests/test-fopen-gnu.c
index cae40421a..eeb1712c7 100644
--- a/tests/test-fopen-gnu.c
+++ b/tests/test-fopen-gnu.c
@@ -29,15 +29,20 @@
#define BASE "test-fopen-gnu.t"
+/* 0x1a is an EOF on Windows. */
+#define DATA "abc\x1adef"
+
int
main (void)
{
FILE *f;
int fd;
int flags;
+ char buf[16];
/* Remove anything from prior partial run. */
unlink (BASE "file");
+ unlink (BASE "binary");
/* Create the file. */
f = fopen (BASE "file", "w");
@@ -64,8 +69,20 @@ main (void)
ASSERT (f == NULL);
ASSERT (errno == EEXIST);
+ /* Open a binary file and check that the 'e' mode doesn't interfere. */
+ f = fopen (BASE "binary", "wbe");
+ ASSERT (f);
+ ASSERT (fwrite (DATA, 1, sizeof(DATA)-1, f) == sizeof(DATA)-1);
+ ASSERT (fclose (f) == 0);
+
+ f = fopen (BASE "binary", "rbe");
+ ASSERT (f);
+ ASSERT (fread (buf, 1, sizeof(buf), f) == sizeof(DATA)-1);
+ ASSERT (fclose (f) == 0);
+
/* Cleanup. */
ASSERT (unlink (BASE "file") == 0);
+ ASSERT (unlink (BASE "binary") == 0);
return 0;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-28 10:00 ` Daiki Ueno
@ 2020-05-28 10:04 ` Daiki Ueno
2020-05-28 11:54 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Daiki Ueno @ 2020-05-28 10:04 UTC (permalink / raw)
To: Bruno Haible; +Cc: Tim Rühsen, bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 630 bytes --]
Daiki Ueno <ueno@gnu.org> writes:
> Bruno Haible <bruno@clisp.org> writes:
>
>>> Here are proposed patches for other modules. Does this look right?
>>
>> There were no objections. I pushed the changes.
>
> Thank you for this. I have rebased GnuTLS on top of it, but noticed a
> strange test failures on Windows CI, which involve reading binary files
> (OCSP response):
> https://gitlab.com/gnutls/gnutls/-/jobs/569815031
>
> It seems that the fopen module ignores a 'b' flag. The attached patch
> fixes the failures.
Sorry, attached an old patch; this would be simpler (and also supports
other platforms that need O_BINARY).
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fopen-gnu-make-b-flag-can-be-used-with-e-on-Windows.patch --]
[-- Type: text/x-patch, Size: 2740 bytes --]
From 81695244eb467603009a2777c3a8438f1a707954 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Thu, 28 May 2020 11:40:49 +0200
Subject: [PATCH] fopen-gnu: make 'b' flag can be used with 'e' on Windows
* lib/fopen.c (rpl_fopen): Pass O_BINARY to open, if a 'b' flag is
specified on Windows.
* tests/test-fopen-gnu.c (DATA): New define.
(main): Add test for reading binary files with an 'e' flag.
---
ChangeLog | 8 ++++++++
lib/fopen.c | 4 ++++
tests/test-fopen-gnu.c | 17 +++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index c17b76b72..ea2716b2f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-28 Daiki Ueno <ueno@gnu.org>
+
+ fopen-gnu: make 'b' flag can be used with 'e' on Windows
+ * lib/fopen.c (rpl_fopen): Pass O_BINARY to open, if a 'b' flag is
+ specified on Windows.
+ * tests/test-fopen-gnu.c (DATA): New define.
+ (main): Add test for reading binary files with an 'e' flag.
+
2020-05-27 Bruno Haible <bruno@clisp.org>
Don't assume that UNICODE is not defined.
diff --git a/lib/fopen.c b/lib/fopen.c
index 20065e4c6..47d7f194d 100644
--- a/lib/fopen.c
+++ b/lib/fopen.c
@@ -101,6 +101,10 @@ rpl_fopen (const char *filename, const char *mode)
#endif
continue;
case 'b':
+ /* While it is non-standard, O_BINARY is guaranteed by
+ gnulib <fcntl.h>. We can also assume that orig_fopen
+ supports the 'b' flag. */
+ open_flags_standard |= O_BINARY;
#if GNULIB_FOPEN_GNU
if (q < fdopen_mode_buf + BUF_SIZE)
*q++ = *p;
diff --git a/tests/test-fopen-gnu.c b/tests/test-fopen-gnu.c
index cae40421a..eeb1712c7 100644
--- a/tests/test-fopen-gnu.c
+++ b/tests/test-fopen-gnu.c
@@ -29,15 +29,20 @@
#define BASE "test-fopen-gnu.t"
+/* 0x1a is an EOF on Windows. */
+#define DATA "abc\x1adef"
+
int
main (void)
{
FILE *f;
int fd;
int flags;
+ char buf[16];
/* Remove anything from prior partial run. */
unlink (BASE "file");
+ unlink (BASE "binary");
/* Create the file. */
f = fopen (BASE "file", "w");
@@ -64,8 +69,20 @@ main (void)
ASSERT (f == NULL);
ASSERT (errno == EEXIST);
+ /* Open a binary file and check that the 'e' mode doesn't interfere. */
+ f = fopen (BASE "binary", "wbe");
+ ASSERT (f);
+ ASSERT (fwrite (DATA, 1, sizeof(DATA)-1, f) == sizeof(DATA)-1);
+ ASSERT (fclose (f) == 0);
+
+ f = fopen (BASE "binary", "rbe");
+ ASSERT (f);
+ ASSERT (fread (buf, 1, sizeof(buf), f) == sizeof(DATA)-1);
+ ASSERT (fclose (f) == 0);
+
/* Cleanup. */
ASSERT (unlink (BASE "file") == 0);
+ ASSERT (unlink (BASE "binary") == 0);
return 0;
}
--
2.26.2
[-- Attachment #3: Type: text/plain, Size: 28 bytes --]
Regards,
--
Daiki Ueno
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-28 10:04 ` Daiki Ueno
@ 2020-05-28 11:54 ` Bruno Haible
2020-05-29 3:04 ` Daiki Ueno
0 siblings, 1 reply; 14+ messages in thread
From: Bruno Haible @ 2020-05-28 11:54 UTC (permalink / raw)
To: Daiki Ueno; +Cc: Tim Rühsen, bug-gnulib
Hi Daiki,
Thank you for noticing this, and the rapid fix.
> Sorry, attached an old patch; this would be simpler (and also supports
> other platforms that need O_BINARY).
> + ASSERT (fwrite (DATA, 1, sizeof(DATA)-1, f) == sizeof(DATA)-1);
...
> + ASSERT (fread (buf, 1, sizeof(buf), f) == sizeof(DATA)-1);
GNU coding style wants a space between 'sizeof' and the opening parenthesis.
Other than that, your patch is perfect.
Thanks again!
Bruno
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-28 11:54 ` Bruno Haible
@ 2020-05-29 3:04 ` Daiki Ueno
2020-05-29 6:25 ` Bruno Haible
0 siblings, 1 reply; 14+ messages in thread
From: Daiki Ueno @ 2020-05-29 3:04 UTC (permalink / raw)
To: Bruno Haible; +Cc: Tim Rühsen, bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 491 bytes --]
Bruno Haible <bruno@clisp.org> writes:
>> + ASSERT (fwrite (DATA, 1, sizeof(DATA)-1, f) == sizeof(DATA)-1);
> ...
>> + ASSERT (fread (buf, 1, sizeof(buf), f) == sizeof(DATA)-1);
>
> GNU coding style wants a space between 'sizeof' and the opening parenthesis.
> Other than that, your patch is perfect.
Thank you for the review; fixed and pushed. Afterwards, I realized a
compilation error in the test code on FreeBSD, so I've added the
follow-up patch attached.
Regards,
--
Daiki Ueno
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fopen-gnu-tests-fix-x-escape-usage.patch --]
[-- Type: text/x-patch, Size: 1079 bytes --]
From 9326739489050009e3b8834838abc02203c592e5 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Fri, 29 May 2020 04:54:31 +0200
Subject: [PATCH] fopen-gnu-tests: fix "\x" escape usage
* tests/test-fopen-gnu.c (DATA): Use safer escape sequence.
---
ChangeLog | 5 +++++
tests/test-fopen-gnu.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index c39bfd372..77c637414 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-29 Daiki Ueno <ueno@gnu.org>
+
+ fopen-gnu-tests: fix "\x" escape usage
+ * tests/test-fopen-gnu.c (DATA): Use safer escape sequence.
+
2020-05-28 Bruno Haible <bruno@clisp.org>
Avoid dynamic loading of Windows API functions when possible.
diff --git a/tests/test-fopen-gnu.c b/tests/test-fopen-gnu.c
index 4d98dcd85..7de45ab8d 100644
--- a/tests/test-fopen-gnu.c
+++ b/tests/test-fopen-gnu.c
@@ -30,7 +30,7 @@
#define BASE "test-fopen-gnu.t"
/* 0x1a is an EOF on Windows. */
-#define DATA "abc\x1adef"
+#define DATA "abc\x1axyz"
int
main (void)
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: portability of fopen and 'e' (O_CLOEXEC) flag
2020-05-29 3:04 ` Daiki Ueno
@ 2020-05-29 6:25 ` Bruno Haible
0 siblings, 0 replies; 14+ messages in thread
From: Bruno Haible @ 2020-05-29 6:25 UTC (permalink / raw)
To: Daiki Ueno; +Cc: Tim Rühsen, bug-gnulib
Daiki Ueno wrote:
> Afterwards, I realized a
> compilation error in the test code on FreeBSD, so I've added the
> follow-up patch attached.
Indeed, octal and hexadecimal escape sequences are not limited to 3 or 2
digits. ISO C 11 section 6.4.4.4 says:
"The hexadecimal digits that follow the backslash and the letter x in a
hexadecimal escape sequence are taken to be part of the construction ..."
"Each octal or hexadecimal escape sequence is the longest sequence of
characters that can constitute the escape sequence."
I had forgotten about it too.
> "abc\x1adef"
One way to write it would be "abc\x1a" "def". Or "abc\032def".
Bruno
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-05-29 6:27 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 14:49 portability of fopen and 'e' (O_CLOEXEC) flag Tim Rühsen
2020-05-11 16:37 ` Bruno Haible
2020-05-11 17:41 ` Eric Blake
2020-05-12 8:28 ` Tim Rühsen
2020-05-24 19:22 ` Bruno Haible
2020-05-26 6:01 ` Daiki Ueno
2020-05-26 8:18 ` Bruno Haible
2020-05-26 16:09 ` Bruno Haible
2020-05-27 18:39 ` Bruno Haible
2020-05-28 10:00 ` Daiki Ueno
2020-05-28 10:04 ` Daiki Ueno
2020-05-28 11:54 ` Bruno Haible
2020-05-29 3:04 ` Daiki Ueno
2020-05-29 6:25 ` Bruno Haible
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).