Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-29 Thread Bruno Haible 
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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-28 Thread Daiki Ueno 
Bruno Haible  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
>From 9326739489050009e3b8834838abc02203c592e5 Mon Sep 17 00:00:00 2001
From: Daiki Ueno 
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  
+
+	fopen-gnu-tests: fix "\x" escape usage
+	* tests/test-fopen-gnu.c (DATA): Use safer escape sequence.
+
 2020-05-28  Bruno Haible  
 
 	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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-28 Thread Bruno Haible 
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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-28 Thread Daiki Ueno 
Daiki Ueno  writes:

> Bruno Haible  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).

>From 81695244eb467603009a2777c3a8438f1a707954 Mon Sep 17 00:00:00 2001
From: Daiki Ueno 
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  
+
+	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  
 
 	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 .  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



Regards,
-- 
Daiki Ueno

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-28 Thread Daiki Ueno 
Bruno Haible  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
>From 17fbb2560a05e3006125f8793c8e814ef5baa847 Mon Sep 17 00:00:00 2001
From: Daiki Ueno 
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  
+
+	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  
 
 	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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-27 Thread Bruno Haible 
> Here are proposed patches for other modules. Does this look right?

There were no objections. I pushed the changes.

Bruno

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-26 Thread Bruno Haible 
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
>From acb3be326fbbeaf82af33fbd7bb7b3cb180d2616 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
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  
+
+	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  
 
 	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 (_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 (_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

>From 67d0dc62291be365765dac3fb9e8c006d8427097 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
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  
 
+	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  
+
 	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, _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

>From 350c6f8dba8cc2a6266cb8a350d01c49805af9db Mon Sep 17 00:00:00 2001
From: Bruno Haible 
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  
 
+	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  
+
 	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 =

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-26 Thread Bruno Haible 
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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-26 Thread Daiki Ueno 
Hello Bruno,

Bruno Haible  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?

>From 5de739d03a7da71127b771cc213c873cd711ce51 Mon Sep 17 00:00:00 2001
From: Daiki Ueno 
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  
+
+	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  
 
 	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


Regards,
-- 
Daiki Ueno

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-24 Thread Bruno Haible 
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  

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  in
.
* 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.

From 10cb4be2a2114dd6fff347acc9841d7904636adf Mon Sep 17 00:00:00 2001
From: Bruno Haible 
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  in
.

* 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  
 
+	fopen-gnu: New module.
+	Suggested by Tim Rühsen  in
+	.
+	* 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  
+
 	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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-12 Thread Tim Rühsen 
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=3=0=freebsd
> [6] https://docs.oracle.com/cd/E36784_01/html/E36874/fopen-3c.html
> 
> 



signature.asc
Description: OpenPGP digital signature

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-11 Thread Eric Blake 

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

Re: portability of fopen and 'e' (O_CLOEXEC) flag

2020-05-11 Thread Bruno Haible 
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=3=0=freebsd
[6] https://docs.oracle.com/cd/E36784_01/html/E36874/fopen-3c.html