From 73da5fb7e129f1fa540e040582cda710b8c2cce4 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 7 Dec 2009 06:53:59 -0700 Subject: [PATCH] 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 --- ChangeLog | 10 ++++++++++ lib/cloexec.c | 9 +++++++-- modules/cloexec-tests | 1 + modules/dup2-tests | 1 + tests/test-cloexec.c | 31 +++++++++++++++++++++++++++++++ tests/test-dup2.c | 26 ++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 00ac8868c..3cf1bdbfd 100644 --- 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 index 5479477e5..57e0be59e 100644 --- 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 38c304cf5..22792dbac 100644 --- 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 0fb913a58..b02e2a23d 100644 --- 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 8df733a70..a29d1be73 100644 --- 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 @@ 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 005160fc6..e6d3c6292 100644 --- 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 @@ 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); -- 2.11.0