-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Bruno Haible on 7/20/2009 6:00 PM:
> Eric Blake wrote:
>> On mingw, dup2 always returns 0 for success, rather than the fd just opened.
>> So I guess I'll start the process of writing a dup2 replacement module;
>
> Yes, that's definitely a portability pitfall that warrants a wrapper module.
And in writing the testcase, I learned that cygwin 1.5 also has a bug, in
that dup2(1,1) returns 0 (but at least cygwin's dup2(1,2) returns 2).
Although you seldom see code using dup2 as a no-op for an fd onto itself,
it provides a great way to non-destructively check whether an fd is open
without having to use fcntl or _get_osfhandle (and so I did just that in
test-pipe.c).
I also noticed an unportable use of open, while using test-open.c as my
template for test-dup2.c.
Here's the patches I'm pushing.
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkplvYkACgkQ84KuGfSFAYAnQQCffBd+hN8NEHYO6T9RY2wEWEvf
Af8An33APaFX/hlkiR2N0UKtEva7A172
=Yv8J
-----END PGP SIGNATURE-----
>From ce186310c3af2ff05e80724f873fe37955a3a29a Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Tue, 21 Jul 2009 06:48:26 -0600
Subject: [PATCH 1/2] dup2: work around mingw and cygwin 1.5 bug
* m4/dup2.m4 (gl_FUNC_DUP2): Detect mingw bug.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witness.
* modules/unistd (Makefile.am): Substitute it.
* lib/unistd.in.h (dup2): Declare the replacement.
* lib/dup2.c (dup2) [REPLACE_DUP2]: Implement it.
* doc/posix-functions/dup2.texi (dup2): Document the bugs.
* lib/fchdir.c (rpl_dup2): Don't collide with mingw replacement.
* modules/execute (Depends-on): Add dup2.
* modules/fseterr (Depends-on): Likewise.
* modules/pipe (Depends-on): Likewise.
* modules/posix_spawn-internal (Depends-on): Likewise.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 15 +++++++++++++++
doc/posix-functions/dup2.texi | 8 ++++++++
lib/dup2.c | 29 +++++++++++++++++++++++------
lib/fchdir.c | 7 ++++++-
lib/unistd.in.h | 11 ++++++++---
m4/dup2.m4 | 24 ++++++++++++++++++++++--
m4/unistd_h.m4 | 3 ++-
modules/execute | 1 +
modules/fseterr | 1 +
modules/pipe | 1 +
modules/posix_spawn-internal | 1 +
modules/unistd | 1 +
12 files changed, 89 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 1b3d36c..18e1159 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2009-07-21 Eric Blake <e...@byu.net>
+
+ dup2: work around mingw and cygwin 1.5 bug
+ * m4/dup2.m4 (gl_FUNC_DUP2): Detect mingw bug.
+ * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witness.
+ * modules/unistd (Makefile.am): Substitute it.
+ * lib/unistd.in.h (dup2): Declare the replacement.
+ * lib/dup2.c (dup2) [REPLACE_DUP2]: Implement it.
+ * doc/posix-functions/dup2.texi (dup2): Document the bugs.
+ * lib/fchdir.c (rpl_dup2): Don't collide with mingw replacement.
+ * modules/execute (Depends-on): Add dup2.
+ * modules/fseterr (Depends-on): Likewise.
+ * modules/pipe (Depends-on): Likewise.
+ * modules/posix_spawn-internal (Depends-on): Likewise.
+
2009-07-20 Bruno Haible <br...@clisp.org>
* tests/test-pipe.c (BACKUP_STDERR_FILENO): New macro.
diff --git a/doc/posix-functions/dup2.texi b/doc/posix-functions/dup2.texi
index 42d22e5..bfaff38 100644
--- a/doc/posix-functions/dup2.texi
+++ b/doc/posix-functions/dup2.texi
@@ -9,6 +9,14 @@ Gnulib module: dup2
Portability problems fixed by Gnulib:
@itemize
@item
+This function always returns 0 for success on some platforms:
+mingw.
+
+...@item
+This function returns 0 for dup2 (1, 1) on some platforms:
+Cygwin 1.5.x.
+
+...@item
This function is missing on some older platforms.
@end itemize
diff --git a/lib/dup2.c b/lib/dup2.c
index 0999082..e5d3de4 100644
--- a/lib/dup2.c
+++ b/lib/dup2.c
@@ -1,6 +1,7 @@
/* Duplicate an open file descriptor to a specified file descriptor.
- Copyright (C) 1999, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+ Copyright (C) 1999, 2004, 2005, 2006, 2007, 2009 Free Software
+ Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -25,7 +26,22 @@
#include <errno.h>
#include <fcntl.h>
-#ifndef F_DUPFD
+#if REPLACE_DUP2
+/* On mingw, dup2 exists, but always returns 0 for success. */
+int
+dup2 (int fd, int desired_fd)
+#undef dup2
+{
+ int result = dup2 (fd, desired_fd);
+ if (result == 0)
+ result = desired_fd;
+ return result;
+}
+
+#else /* !REPLACE_DUP2 */
+/* On older platforms, dup2 did not exist. */
+
+# ifndef F_DUPFD
static int
dupfd (int fd, int desired_fd)
{
@@ -41,7 +57,7 @@ dupfd (int fd, int desired_fd)
return r;
}
}
-#endif
+# endif
int
dup2 (int fd, int desired_fd)
@@ -49,9 +65,10 @@ dup2 (int fd, int desired_fd)
if (fd == desired_fd)
return fd;
close (desired_fd);
-#ifdef F_DUPFD
+# ifdef F_DUPFD
return fcntl (fd, F_DUPFD, desired_fd);
-#else
+# else
return dupfd (fd, desired_fd);
-#endif
+# endif
}
+#endif /* !REPLACE_DUP2 */
diff --git a/lib/fchdir.c b/lib/fchdir.c
index 969e984..cc2fa63 100644
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -1,5 +1,5 @@
/* fchdir replacement.
- Copyright (C) 2006-2008 Free Software Foundation, Inc.
+ Copyright (C) 2006-2009 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -177,6 +177,11 @@ rpl_dup2 (int oldfd, int newfd)
#undef dup2
{
int retval = dup2 (oldfd, newfd);
+#if REPLACE_DUP2
+ /* Inline mingw replacement from dup2.c. */
+ if (retval == 0)
+ retval = newfd;
+#endif
if (retval >= 0 && oldfd >= 0 && newfd >= 0 && newfd != oldfd)
{
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index cd3a60b..47d3096 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -150,10 +150,13 @@ extern int close (int);
#if @GNULIB_DUP2@
-# if !...@have_dup2@
+# if @REPLACE_DUP2@
+# define dup2 rpl_dup2
+# endif
+# if !...@have_dup2@ || @REPLACE_DUP2@
/* Copy the file descriptor OLDFD into file descriptor NEWFD. Do nothing if
NEWFD = OLDFD, otherwise close NEWFD first if it is open.
- Return 0 if successful, otherwise -1 and errno set.
+ Return newfd if successful, otherwise -1 and errno set.
See the POSIX:2001 specification
<http://www.opengroup.org/susv3xsh/dup2.html>. */
extern int dup2 (int oldfd, int newfd);
@@ -214,8 +217,10 @@ extern int fchdir (int /*fd*/);
# define dup rpl_dup
extern int dup (int);
-# define dup2 rpl_dup2
+# if !...@replace_dup2@
+# define dup2 rpl_dup2
extern int dup2 (int, int);
+# endif
# endif
#elif defined GNULIB_POSIXCHECK
diff --git a/m4/dup2.m4 b/m4/dup2.m4
index 0549823..2abc74c 100644
--- a/m4/dup2.m4
+++ b/m4/dup2.m4
@@ -1,5 +1,5 @@
-#serial 5
-dnl Copyright (C) 2002, 2005, 2007 Free Software Foundation, Inc.
+#serial 6
+dnl Copyright (C) 2002, 2005, 2007, 2009 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
@@ -7,9 +7,29 @@ dnl with or without modifications, as long as this notice is
preserved.
AC_DEFUN([gl_FUNC_DUP2],
[
AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+ AC_REQUIRE([AC_CANONICAL_HOST])
AC_CHECK_FUNCS_ONCE([dup2])
if test $ac_cv_func_dup2 = no; then
HAVE_DUP2=0
AC_LIBOBJ([dup2])
+ else
+ AC_CACHE_CHECK([whether dup2 works], [gl_cv_func_dup2_works],
+ [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#include <unistd.h>
+]], [return 1 - dup2 (1, 1);])],
+ [gl_cv_func_dup2_works=yes], [gl_cv_func_dup2_works=no],
+ [case "$host_os" in
+ mingw*) # on this platform, dup2 always returns 0 for success
+ gl_cv_func_dup2_works=no;;
+ cygwin*) # on cygwin 1.5.x, dup2(1,1) returns 0
+ gl_cv_func_dup2_works=no;;
+ *) gl_cv_func_dup2_works=yes;;
+ esac])])
+ if test "$gl_cv_func_dup2_works" = no; then
+ REPLACE_DUP2=1
+ AC_LIBOBJ([dup2])
+ fi
fi
+ AC_DEFINE_UNQUOTED([REPLACE_DUP2], [$REPLACE_DUP2],
+ [Define to 1 if dup2 returns 0 instead of the target fd.])
])
diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
index ff9a4ea..96fddba 100644
--- a/m4/unistd_h.m4
+++ b/m4/unistd_h.m4
@@ -1,4 +1,4 @@
-# unistd_h.m4 serial 17
+# unistd_h.m4 serial 18
dnl Copyright (C) 2006-2009 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -73,6 +73,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS],
HAVE_SYS_PARAM_H=0; AC_SUBST([HAVE_SYS_PARAM_H])
REPLACE_CHOWN=0; AC_SUBST([REPLACE_CHOWN])
REPLACE_CLOSE=0; AC_SUBST([REPLACE_CLOSE])
+ REPLACE_DUP2=0; AC_SUBST([REPLACE_DUP2])
REPLACE_FCHDIR=0; AC_SUBST([REPLACE_FCHDIR])
REPLACE_GETCWD=0; AC_SUBST([REPLACE_GETCWD])
REPLACE_GETPAGESIZE=0; AC_SUBST([REPLACE_GETPAGESIZE])
diff --git a/modules/execute b/modules/execute
index 6dddd38..88c4fa4 100644
--- a/modules/execute
+++ b/modules/execute
@@ -8,6 +8,7 @@ lib/w32spawn.h
m4/execute.m4
Depends-on:
+dup2
error
exit
fatal-signal
diff --git a/modules/fseterr b/modules/fseterr
index 190cbd2..60f9251 100644
--- a/modules/fseterr
+++ b/modules/fseterr
@@ -7,6 +7,7 @@ lib/fseterr.c
lib/stdio-impl.h
Depends-on:
+dup2
configure.ac:
diff --git a/modules/pipe b/modules/pipe
index 08c2f94..570d71d 100644
--- a/modules/pipe
+++ b/modules/pipe
@@ -8,6 +8,7 @@ lib/w32spawn.h
m4/pipe.m4
Depends-on:
+dup2
environ
error
exit
diff --git a/modules/posix_spawn-internal b/modules/posix_spawn-internal
index 211cc22..da3ed28 100644
--- a/modules/posix_spawn-internal
+++ b/modules/posix_spawn-internal
@@ -9,6 +9,7 @@ m4/posix_spawn.m4
Depends-on:
spawn
alloca-opt
+dup2
errno
open
strchrnul
diff --git a/modules/unistd b/modules/unistd
index 1359c52..7f7495d 100644
--- a/modules/unistd
+++ b/modules/unistd
@@ -64,6 +64,7 @@ unistd.h: unistd.in.h
-e 's|@''HAVE_SYS_PARAM_H''@|$(HAVE_SYS_PARAM_H)|g' \
-e 's|@''REPLACE_CHOWN''@|$(REPLACE_CHOWN)|g' \
-e 's|@''REPLACE_CLOSE''@|$(REPLACE_CLOSE)|g' \
+ -e 's|@''REPLACE_DUP2''@|$(REPLACE_DUP2)|g' \
-e 's|@''REPLACE_FCHDIR''@|$(REPLACE_FCHDIR)|g' \
-e 's|@''REPLACE_GETCWD''@|$(REPLACE_GETCWD)|g' \
-e 's|@''REPLACE_GETPAGESIZE''@|$(REPLACE_GETPAGESIZE)|g' \
--
1.6.3.3.334.g916e1
>From 8dd1b999381551879a924569a30631f6ceb75de8 Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Tue, 21 Jul 2009 06:48:56 -0600
Subject: [PATCH 2/2] dup2-tests: test previous patch
* modules/dup2-tests: New file.
* tests/test-dup2.c: Likewise.
* tests/test-open.c (main): Avoid unspecified behavior.
* tests/test-pipe.c (child_main): Use dup2 semantics to simplify
test.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 7 +++
modules/dup2-tests | 12 +++++
tests/test-dup2.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/test-open.c | 4 +-
tests/test-pipe.c | 47 ++++-----------------
5 files changed, 147 insertions(+), 40 deletions(-)
create mode 100644 modules/dup2-tests
create mode 100644 tests/test-dup2.c
diff --git a/ChangeLog b/ChangeLog
index 18e1159..9a6dae0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2009-07-21 Eric Blake <e...@byu.net>
+ dup2-tests: test previous patch
+ * modules/dup2-tests: New file.
+ * tests/test-dup2.c: Likewise.
+ * tests/test-open.c (main): Avoid unspecified behavior.
+ * tests/test-pipe.c (child_main): Use dup2 semantics to simplify
+ test.
+
dup2: work around mingw and cygwin 1.5 bug
* m4/dup2.m4 (gl_FUNC_DUP2): Detect mingw bug.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witness.
diff --git a/modules/dup2-tests b/modules/dup2-tests
new file mode 100644
index 0000000..9b1e7db
--- /dev/null
+++ b/modules/dup2-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-dup2.c
+
+Depends-on:
+close
+open
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-dup2
+check_PROGRAMS += test-dup2
diff --git a/tests/test-dup2.c b/tests/test-dup2.c
new file mode 100644
index 0000000..67f49ef
--- /dev/null
+++ b/tests/test-dup2.c
@@ -0,0 +1,117 @@
+/* Test duplicating file descriptors.
+ Copyright (C) 2009 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Eric Blake <e...@byu.net>, 2009. */
+
+#include <config.h>
+
+#include <unistd.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+/* Get declarations of the Win32 API functions. */
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#endif
+
+#define ASSERT(expr) \
+ do \
+ { \
+ if (!(expr)) \
+ { \
+ fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+ fflush (stderr); \
+ abort (); \
+ } \
+ } \
+ while (0)
+
+/* Return non-zero if FD is open. */
+static int
+is_open (int fd)
+{
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+ /* On Win32, the initial state of unassigned standard file
+ descriptors is that they are open but point to an
+ INVALID_HANDLE_VALUE, and there is no fcntl. */
+ return (HANDLE) _get_osfhandle (fd) != INVALID_HANDLE_VALUE;
+#else
+# ifndef F_GETFL
+# error Please port fcntl to your platform
+# endif
+ return 0 <= fcntl (fd, F_GETFL);
+#endif
+}
+
+int
+main ()
+{
+ const char *file = "test-dup2.tmp";
+ char buffer[1];
+ int fd = open (file, O_CREAT | O_RDWR, 0600);
+
+ ASSERT (0 <= fd);
+ ASSERT (is_open (fd));
+ ASSERT (!is_open (fd + 1));
+ ASSERT (!is_open (fd + 2));
+
+ /* Assigning to self must be a no-op. */
+ ASSERT (dup2 (fd, fd) == fd);
+ ASSERT (is_open (fd));
+
+ /* If the source is not open, then the destination is unaffected. */
+ errno = 0;
+ ASSERT (dup2 (fd + 1, fd + 1) == -1);
+ ASSERT (errno == EBADF);
+ ASSERT (!is_open (fd + 1));
+ errno = 0;
+ ASSERT (dup2 (fd + 1, fd) == -1);
+ ASSERT (errno == EBADF);
+ ASSERT (is_open (fd));
+
+ /* The destination must be valid. */
+ errno = 0;
+ ASSERT (dup2 (fd, -2) == -1);
+ ASSERT (errno == EBADF);
+
+ /* Using dup2 can skip fds. */
+ ASSERT (dup2 (fd, fd + 2) == fd + 2);
+ ASSERT (is_open (fd));
+ ASSERT (!is_open (fd + 1));
+ ASSERT (is_open (fd + 2));
+
+ /* Prove that dup2 closes the previous occupant of a fd. */
+ ASSERT (open ("/dev/null", O_WRONLY, 0600) == fd + 1);
+ ASSERT (dup2 (fd + 1, fd) == fd);
+ ASSERT (close (fd + 1) == 0);
+ ASSERT (write (fd, "1", 1) == 1);
+ ASSERT (dup2 (fd + 2, fd) == fd);
+ ASSERT (write (fd + 2, "2", 1) == 1);
+ ASSERT (lseek (fd, SEEK_SET, 0) == 0);
+ ASSERT (read (fd, buffer, 1) == 1);
+ ASSERT (*buffer == '2');
+
+ /* Clean up. */
+ ASSERT (close (fd + 2) == 0);
+ ASSERT (close (fd) == 0);
+ ASSERT (unlink (file) == 0);
+
+ return 0;
+}
diff --git a/tests/test-open.c b/tests/test-open.c
index 0eb8a33..f7bb543 100644
--- a/tests/test-open.c
+++ b/tests/test-open.c
@@ -1,5 +1,5 @@
/* Test of opening a file descriptor.
- Copyright (C) 2007-2008 Free Software Foundation, Inc.
+ Copyright (C) 2007-2009 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -38,7 +38,7 @@
int
main ()
{
- ASSERT (open ("nonexist.ent/", O_CREAT, 0600) < 0);
+ ASSERT (open ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) < 0);
ASSERT (open ("/dev/null/", O_RDONLY) < 0);
ASSERT (open ("/dev/null", O_RDONLY) >= 0);
diff --git a/tests/test-pipe.c b/tests/test-pipe.c
index 16c72f6..404534d 100644
--- a/tests/test-pipe.c
+++ b/tests/test-pipe.c
@@ -20,18 +20,12 @@
#include "pipe.h"
#include "wait-process.h"
-#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-/* Get declarations of the Win32 API functions. */
-# define WIN32_LEAN_AND_MEAN
-# include <windows.h>
-#endif
-
#include <errno.h>
-#include <fcntl.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
/* Depending on arguments, this test intentionally closes stderr or
starts life with stderr closed. So, we arrange to have fd 10
@@ -59,6 +53,7 @@ child_main (int argc, char *argv[])
{
char buffer[2] = { 's', 't' };
int fd;
+ int ret;
ASSERT (argc == 3);
@@ -71,46 +66,22 @@ child_main (int argc, char *argv[])
buffer[0]++;
ASSERT (write (STDOUT_FILENO, buffer, 1) == 1);
-#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
- /* On Win32, the initial state of unassigned standard file descriptors is
- that they are open but point to an INVALID_HANDLE_VALUE. Thus
- close (STDERR_FILENO) would always succeed. */
+ errno = 0;
+ ret = dup2 (STDERR_FILENO, STDERR_FILENO);
switch (atoi (argv[2]))
{
case 0:
- /* Expect fd 2 is open to a valid handle. */
- ASSERT ((HANDLE) _get_osfhandle (STDERR_FILENO) != INVALID_HANDLE_VALUE);
+ /* Expect fd 2 is open. */
+ ASSERT (ret == STDERR_FILENO);
break;
case 1:
- /* Expect fd 2 is pointing to INVALID_HANDLE_VALUE. */
- ASSERT ((HANDLE) _get_osfhandle (STDERR_FILENO) == INVALID_HANDLE_VALUE);
+ /* Expect fd 2 is closed. */
+ ASSERT (ret == -1);
+ ASSERT (errno == EBADF);
break;
default:
ASSERT (false);
}
-#elif defined F_GETFL
- /* On Unix, the initial state of unassigned standard file descriptors is
- that they are closed. */
- {
- int ret;
- errno = 0;
- ret = fcntl (STDERR_FILENO, F_GETFL);
- switch (atoi (argv[2]))
- {
- case 0:
- /* Expect fd 2 is open. */
- ASSERT (ret >= 0);
- break;
- case 1:
- /* Expect fd 2 is closed. */
- ASSERT (ret < 0);
- ASSERT (errno == EBADF);
- break;
- default:
- ASSERT (false);
- }
- }
-#endif
for (fd = 3; fd < 7; fd++)
{
--
1.6.3.3.334.g916e1
