cloexec: preserve text vs. binary across dup_cloexec

On mingw, dup_cloexec mistakenly converted a text fd into a
binary fd.  Cygwin copied the source mode.  Most other platforms
don't distinguish between modes.

* lib/cloexec.c (dup_cloexec) [W32]: Query and use translation
mode.
* modules/dup2-tests (Depends-on): Add binary-io.
* modules/cloexec-tests (Depends-on): Likewise.
* tests/test-dup2.c (setmode, is_mode): New helpers.
(main): Add tests that translation mode is preserved.
* tests/test-cloexec.c (setmode, is_mode, main): Likewise.
Reported by Bruno Haible.

Signed-off-by: Eric Blake <ebb9@byu.net>

diff --git a/ChangeLog b/ChangeLog
index 00ac8868c7b607242c49ef0f783438414afc8ed0..3cf1bdbfdca17af2308e4f7b761ad2a4e50f2f8e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2009-12-07  Eric Blake  <ebb9@byu.net>
 
+       cloexec: preserve text vs. binary across dup_cloexec
+       * lib/cloexec.c (dup_cloexec) [W32]: Query and use translation
+       mode.
+       * modules/dup2-tests (Depends-on): Add binary-io.
+       * modules/cloexec-tests (Depends-on): Likewise.
+       * tests/test-dup2.c (setmode, is_mode): New helpers.
+       (main): Add tests that translation mode is preserved.
+       * tests/test-cloexec.c (setmode, is_mode, main): Likewise.
+       Reported by Bruno Haible.
+
        mgetgroups: reduce duplicate listings
        * lib/mgetgroups.c (mgetgroups): Reduce duplicates from the
        resulting array.

diff --git a/lib/cloexec.c b/lib/cloexec.c
index 5479477e51e49e31575a1f99c85cd291a083bd6e..57e0be59e277858ee74c27e39dda14c684fcbed0 100644 (file)
--- a/lib/cloexec.c
+++ b/lib/cloexec.c
@@ -64,6 +64,8 @@ set_cloexec_flag (int desc, bool value)
 
 #else
 
+  /* Use dup2 to reject invalid file descriptors; the cloexec flag
+     will be unaffected.  */
   if (desc < 0)
     {
       errno = EBADF;
@@ -90,8 +92,10 @@ dup_cloexec (int fd)
   HANDLE curr_process = GetCurrentProcess ();
   HANDLE old_handle = (HANDLE) _get_osfhandle (fd);
   HANDLE new_handle;
+  int mode;
 
-  if (old_handle == INVALID_HANDLE_VALUE)
+  if (old_handle == INVALID_HANDLE_VALUE
+      || (mode = setmode (fd, O_BINARY)) == -1)
     {
       /* fd is closed, or is open to no handle at all.
          We cannot duplicate fd in this case, because _open_osfhandle
@@ -99,6 +103,7 @@ dup_cloexec (int fd)
       errno = EBADF;
       return -1;
     }
+  setmode (fd, mode);
 
   if (!DuplicateHandle (curr_process,               /* SourceProcessHandle */
                         old_handle,                 /* SourceHandle */
@@ -113,7 +118,7 @@ dup_cloexec (int fd)
       return -1;
     }
 
-  nfd = _open_osfhandle ((long) new_handle, O_BINARY | O_NOINHERIT);
+  nfd = _open_osfhandle ((long) new_handle, mode | O_NOINHERIT);
   if (nfd < 0)
     {
       CloseHandle (new_handle);

diff --git a/modules/cloexec-tests b/modules/cloexec-tests
index 38c304cf5b767c12aa59364b969723455de79707..22792dbacce351aa461e96e7f27936ff9b9706c6 100644 (file)
--- a/modules/cloexec-tests
+++ b/modules/cloexec-tests
@@ -2,6 +2,7 @@ Files:
 tests/test-cloexec.c
 
 Depends-on:
+binary-io
 
 configure.ac:
 

diff --git a/modules/dup2-tests b/modules/dup2-tests
index 0fb913a58ff64f51e25ff9cb94e0ab5f3d570df3..b02e2a23d776434ccc3932532f7a19bf7f6f3ac2 100644 (file)
--- a/modules/dup2-tests
+++ b/modules/dup2-tests
@@ -2,6 +2,7 @@ Files:
 tests/test-dup2.c
 
 Depends-on:
+binary-io
 cloexec
 open
 

diff --git a/tests/test-cloexec.c b/tests/test-cloexec.c
index 8df733a70bfeeb85d56de8e87e3a8dc0e6d31459..a29d1be739fed35d2789e94c8808cec0fd605c23 100644 (file)
--- a/tests/test-cloexec.c
+++ b/tests/test-cloexec.c
@@ -32,6 +32,8 @@
 # include <windows.h>
 #endif
 
+#include "binary-io.h"
+
 #define ASSERT(expr) \
   do                                                                         \
     {                                                                        \
@@ -66,6 +68,20 @@ is_inheritable (int fd)
 #endif
 }
 
+#if !O_BINARY
+# define setmode(f,m) 0
+#endif
+
+/* Return non-zero if FD is open in the given MODE, which is either
+   O_TEXT or O_BINARY.  */
+static int
+is_mode (int fd, int mode)
+{
+  int value = setmode (fd, O_BINARY);
+  setmode (fd, value);
+  return mode == value;
+}
+
 int
 main (void)
 {
@@ -94,6 +110,21 @@ main (void)
   ASSERT (!is_inheritable (fd));
   ASSERT (close (fd2) == 0);
 
+  /* On systems that distinguish between text and binary mode,
+     dup_cloexec reuses the mode of the source.  */
+  setmode (fd, O_BINARY);
+  ASSERT (is_mode (fd, O_BINARY));
+  fd2 = dup_cloexec (fd);
+  ASSERT (fd < fd2);
+  ASSERT (is_mode (fd2, O_BINARY));
+  ASSERT (close (fd2) == 0);
+  setmode (fd, O_TEXT);
+  ASSERT (is_mode (fd, O_TEXT));
+  fd2 = dup_cloexec (fd);
+  ASSERT (fd < fd2);
+  ASSERT (is_mode (fd2, O_TEXT));
+  ASSERT (close (fd2) == 0);
+
   /* Test error handling.  */
   errno = 0;
   ASSERT (set_cloexec_flag (-1, false) == -1);

diff --git a/tests/test-dup2.c b/tests/test-dup2.c
index 005160fc6c80c9070ff8ea035b00417113efa49b..e6d3c62926358d5d739360278da26921f8459626 100644 (file)
--- a/tests/test-dup2.c
+++ b/tests/test-dup2.c
@@ -25,6 +25,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "binary-io.h"
 #include "cloexec.h"
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
@@ -84,6 +85,20 @@ is_inheritable (int fd)
 #endif
 }
 
+#if !O_BINARY
+# define setmode(f,m) 0
+#endif
+
+/* Return non-zero if FD is open in the given MODE, which is either
+   O_TEXT or O_BINARY.  */
+static int
+is_mode (int fd, int mode)
+{
+  int value = setmode (fd, O_BINARY);
+  setmode (fd, value);
+  return mode == value;
+}
+
 int
 main (void)
 {
@@ -158,6 +173,17 @@ main (void)
   ASSERT (dup2 (fd + 1, fd + 2) == fd + 2);
   ASSERT (is_inheritable (fd + 2));
 
+  /* On systems that distinguish between text and binary mode, dup2
+     reuses the mode of the source.  */
+  setmode (fd, O_BINARY);
+  ASSERT (is_mode (fd, O_BINARY));
+  ASSERT (dup2 (fd, fd + 1) == fd + 1);
+  ASSERT (is_mode (fd + 1, O_BINARY));
+  setmode (fd, O_TEXT);
+  ASSERT (is_mode (fd, O_TEXT));
+  ASSERT (dup2 (fd, fd + 1) == fd + 1);
+  ASSERT (is_mode (fd + 1, O_TEXT));
+
   /* Clean up.  */
   ASSERT (close (fd + 2) == 0);
   ASSERT (close (fd + 1) == 0);