# HG changeset patch # User Eric Blake # Date 1260194039 25200 # Node ID 8700bea78e67bd2f3782f70c8a2b4b292e2d2dea # Parent 799c920db2a535c0913acf5bee457f7a64ae08cf 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 diff --git a/ChangeLog b/ChangeLog --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2009-12-07 Eric Blake + 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 --- 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); diff --git a/modules/cloexec-tests b/modules/cloexec-tests --- a/modules/cloexec-tests +++ b/modules/cloexec-tests @@ -2,6 +2,7 @@ tests/test-cloexec.c Depends-on: +binary-io configure.ac: diff --git a/modules/dup2-tests b/modules/dup2-tests --- a/modules/dup2-tests +++ b/modules/dup2-tests @@ -2,6 +2,7 @@ tests/test-dup2.c Depends-on: +binary-io cloexec open diff --git a/tests/test-cloexec.c b/tests/test-cloexec.c --- a/tests/test-cloexec.c +++ b/tests/test-cloexec.c @@ -32,6 +32,8 @@ # include #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); diff --git a/tests/test-dup2.c b/tests/test-dup2.c --- a/tests/test-dup2.c +++ b/tests/test-dup2.c @@ -25,6 +25,7 @@ #include #include +#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);