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 00ac886..3cf1bdb 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 5479477..57e0be5 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 38c304c..22792db 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 0fb913a..b02e2a2 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 8df733a..a29d1be 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 005160f..e6d3c62 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);