changeset 12398:8700bea78e67

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>
author Eric Blake <ebb9@byu.net>
date Mon, 07 Dec 2009 06:53:59 -0700
parents 799c920db2a5
children 7b2940180b02
files ChangeLog lib/cloexec.c modules/cloexec-tests modules/dup2-tests tests/test-cloexec.c tests/test-dup2.c
diffstat 6 files changed, 76 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- 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.
--- a/lib/cloexec.c
+++ b/lib/cloexec.c
@@ -64,6 +64,8 @@
 
 #else
 
+  /* Use dup2 to reject invalid file descriptors; the cloexec flag
+     will be unaffected.  */
   if (desc < 0)
     {
       errno = EBADF;
@@ -90,8 +92,10 @@
   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 @@
       errno = EBADF;
       return -1;
     }
+  setmode (fd, mode);
 
   if (!DuplicateHandle (curr_process,               /* SourceProcessHandle */
                         old_handle,                 /* SourceHandle */
@@ -113,7 +118,7 @@
       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);
--- a/modules/cloexec-tests
+++ b/modules/cloexec-tests
@@ -2,6 +2,7 @@
 tests/test-cloexec.c
 
 Depends-on:
+binary-io
 
 configure.ac:
 
--- a/modules/dup2-tests
+++ b/modules/dup2-tests
@@ -2,6 +2,7 @@
 tests/test-dup2.c
 
 Depends-on:
+binary-io
 cloexec
 open
 
--- 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 @@
 #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 @@
   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);
--- 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 @@
 #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 @@
   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);