fcntl module (was: O_CLOEXEC support)

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paolo Bonzini on 8/20/2009 2:12 AM:
> Why not just implement a fcntl replacement, supporting only
> F_GETFD/F_SETFD for now (and maybe in the future F_DUPFD and
> F_DUPFD_CLOEXEC, as you mention below)?  You mentioned this later in the
> email, but I read about it on the list in the past too, and it can be
> done as the very first step IMO.

First order of business - we need two modules based on the fcntl name, one
for the header (as fcntl.in.h does other useful things independently of
whether we are fixing cloexec and other fcntl(2) items), and one for the
function.  If we keep the existing fcntl name as the header, then what
should I name the fnctl(2) replacement module?  On the other hand, why not
rename the header module to fcntl-h, and re-purpose the fcntl module name
to cover the function?  My preference is the latter, given that we already
have a gettext/gettext-h module doing a similar job.

Additionally, most users of fcntl.in.h are via indirect dependencies from
other modules, so I doubt many gnulib users are currently directly
requesting 'gnulib-tool --import fcntl'.  And since the function module
will depend on the header module, a client that imports fcntl will still
get the fixed header (and they can later choose to import just fcntl-h
rather than fcntl to reduce how much gets imported).

So, with that background, does this patch series look okay so far?  I've
tested that it passes on mingw, cygwin, and older Linux (I don't yet have
access to a machine running new enough kernel/glibc to test O_CLOEXEC or
F_DUPFD_CLOEXEC).  Also available at:

git pull git://repo.or.cz/gnulib/ericb.git master

Still to be done before this module is complete:
 - work around the mingw behavior that dup() and dup2() copy the
HANDLE_FLAG_INHERIT status (basically, now that we are treating that flag
as equivalent to FD_CLOEXEC, only dup3 and F_DUPFD_CLOEXEC should be
allowed to create duplicate fd's where the flag starts life cleared; at
least the fcntl F_DUPFD replacement gets it right)
 - add code to fcntl.{m4,c} to wrap an existing fcntl implemenation, so
that we can work around a cygwin 1.5.x bug where fcntl(0,F_DUPFD,-1)
succeeds instead of failing with EINVAL
 - using a wrapped existing fcntl implementation, add F_DUPFD_CLOEXEC support
 - update all current gnulib modules that use conditionally use the
supported parts of fcntl to instead use this module
 - add trivial support for O_TTY_INIT

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----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/

iEYEARECAAYFAkqPGXwACgkQ84KuGfSFAYBzkwCgjBOeRs2fBQ/UEuknT2VZwV4b
NrkAn2766Mx0PqWKCWFLZi4jIqauKkXb
=GvnX
-----END PGP SIGNATURE-----

>From ec2f1b3bd5c77dc03b35674d48cf6d13faa8203d Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 21 Aug 2009 08:26:40 -0600
Subject: [PATCH 1/3] fcntl-h: rename from fcntl, in preparation for fcntl(2)

* modules/fcntl: Move <fcntl.h> header replacement...
* modules/fcntl-h: ...to new name, so as not to collide with
like-named function.
* tests/test-fcntl.c: Rename...
* tests/test-fcntl-h.c: ...to this.  Test FD_CLOEXEC.
* modules/fcntl-tests: Rename...
* modules/fcntl-h-tests: ...to this.  Update test file name.
* modules/chdir-long (Depends-on): Update clients.
* modules/chdir-safer (Depends-on): Likewise.
* modules/fcntl-safer (Depends-on): Likewise.
* modules/fts (Depends-on): Likewise.
* modules/mkancesdirs (Depends-on): Likewise.
* modules/mkdir-p (Depends-on): Likewise.
* modules/open (Depends-on): Likewise.
* modules/savewd (Depends-on): Likewise.
* MODULES.html.sh (systems lacking POSIX:2008): Update name.
* doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                              |   21 +++++++++++++++++
 MODULES.html.sh                        |    2 +-
 doc/posix-headers/fcntl.texi           |   39 ++++++++++++++++++++++++++++---
 modules/chdir-long                     |    2 +-
 modules/chdir-safer                    |    2 +-
 modules/fcntl                          |   27 ++-------------------
 modules/{fcntl => fcntl-h}             |    0
 modules/fcntl-h-tests                  |   13 ++++++++++
 modules/fcntl-safer                    |    2 +-
 modules/fcntl-tests                    |   13 ----------
 modules/fts                            |    2 +-
 modules/mkancesdirs                    |    2 +-
 modules/mkdir-p                        |    2 +-
 modules/open                           |    2 +-
 modules/savewd                         |    2 +-
 tests/{test-fcntl.c => test-fcntl-h.c} |    5 +++-
 16 files changed, 85 insertions(+), 51 deletions(-)
 copy modules/{fcntl => fcntl-h} (100%)
 create mode 100644 modules/fcntl-h-tests
 delete mode 100644 modules/fcntl-tests
 rename tests/{test-fcntl.c => test-fcntl-h.c} (89%)

diff --git a/ChangeLog b/ChangeLog
index 841212e..95da7c0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2009-08-21  Eric Blake  <address@hidden>
+
+       fcntl-h: rename from fcntl, in preparation for fcntl(2)
+       * modules/fcntl: Move <fcntl.h> header replacement...
+       * modules/fcntl-h: ...to new name, so as not to collide with
+       like-named function.
+       * tests/test-fcntl.c: Rename...
+       * tests/test-fcntl-h.c: ...to this.  Test FD_CLOEXEC.
+       * modules/fcntl-tests: Rename...
+       * modules/fcntl-h-tests: ...to this.  Update test file name.
+       * modules/chdir-long (Depends-on): Update clients.
+       * modules/chdir-safer (Depends-on): Likewise.
+       * modules/fcntl-safer (Depends-on): Likewise.
+       * modules/fts (Depends-on): Likewise.
+       * modules/mkancesdirs (Depends-on): Likewise.
+       * modules/mkdir-p (Depends-on): Likewise.
+       * modules/open (Depends-on): Likewise.
+       * modules/savewd (Depends-on): Likewise.
+       * MODULES.html.sh (systems lacking POSIX:2008): Update name.
+       * doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.
+
 2009-08-19  Akim Demaille  <address@hidden>

        bootstrap: --help to stdout.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 9d52e42..3c54cec 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2251,7 +2251,7 @@ func_all_modules ()
   func_module errno
   func_module fchdir
   func_module fclose
-  func_module fcntl
+  func_module fcntl-h
   func_module flock
   func_module fopen
   func_module fprintf-posix
diff --git a/doc/posix-headers/fcntl.texi b/doc/posix-headers/fcntl.texi
index 673317b..f2d5fb2 100644
--- a/doc/posix-headers/fcntl.texi
+++ b/doc/posix-headers/fcntl.texi
@@ -3,13 +3,14 @@ fcntl.h

 POSIX specification: @url{http://www.opengroup.org/susv3xbd/fcntl.h.html}

-Gnulib module: fcntl
+Gnulib module: fcntl-h

 Portability problems fixed by Gnulib:
 @itemize
 @item
 @samp{O_NOCTTY}, @samp{O_DSYNC}, @samp{O_NONBLOCK}, @samp{O_RSYNC},
address@hidden are not defined on some platforms.
address@hidden, @samp{O_DIRECTORY}, and @samp{O_NOFOLLOW} are not
+defined on some platforms.

 @item
 @samp{O_BINARY}, @samp{O_TEXT} (not specified by POSIX, but essential for
@@ -17,11 +18,41 @@ fcntl.h
 others.

 @item
address@hidden, @samp{O_DIRECTORY}, @samp{O_NDELAY}, @samp{O_NOATIME},
address@hidden, @samp{O_NOLINKS} (not specified by POSIX) are defined
address@hidden, @samp{O_NDELAY}, @samp{O_NOATIME},
+and @samp{O_NOLINKS} (not specified by POSIX) are defined
 on some platforms but not on others.
+
address@hidden
address@hidden is not defined on some platforms:
+mingw
+
address@hidden
address@hidden and friends are provided by modules like openat.
 @end itemize

 Portability problems not fixed by Gnulib:
 @itemize
address@hidden
address@hidden is not defined on some platforms.  The gnulib
+replacement is not atomic on these platforms.
+
address@hidden
address@hidden, @samp{O_SEARCH}, and @samp{O_EXEC} are not defined
+on some platforms.
+
address@hidden
address@hidden, @samp{F_DUPFD_CLOEXEC}, @samp{F_GETFD}, and
address@hidden are not defined on some platforms:
+mingw
+
address@hidden
address@hidden, @samp{F_SETFL}, @samp{F_GETLK}, @samp{F_SETLK},
address@hidden, @samp{F_GETOWN}, and @samp{F_SETOWN} are not defined
+on some platforms.
+
address@hidden
address@hidden, @samp{POSIX_FADV_NOREUSE},
address@hidden, @samp{POSIX_FADV_RANDOM},
address@hidden, and @samp{POSIX_FADV_WILLNEED} are not
+defined on some platforms.
 @end itemize
diff --git a/modules/chdir-long b/modules/chdir-long
index d06878b..4025b45 100644
--- a/modules/chdir-long
+++ b/modules/chdir-long
@@ -9,7 +9,7 @@ m4/chdir-long.m4
 Depends-on:
 atexit
 fchdir
-fcntl
+fcntl-h
 openat
 memchr
 mempcpy
diff --git a/modules/chdir-safer b/modules/chdir-safer
index 95103e8..b3b47e1 100644
--- a/modules/chdir-safer
+++ b/modules/chdir-safer
@@ -8,7 +8,7 @@ m4/chdir-safer.m4

 Depends-on:
 fchdir
-fcntl
+fcntl-h
 open
 same-inode
 stdbool
diff --git a/modules/fcntl b/modules/fcntl
index de0aeb9..a63c4e3 100644
--- a/modules/fcntl
+++ b/modules/fcntl
@@ -1,35 +1,14 @@
 Description:
-Like <fcntl.h>, but with non-working flags defined to 0.
+Placeholder for eventual fcntl() replacement.

 Files:
-lib/fcntl.in.h
-m4/fcntl_h.m4

 Depends-on:
-include_next
-unistd
-extensions
+fcntl-h

 configure.ac:
-gl_FCNTL_H

 Makefile.am:
-BUILT_SOURCES += $(FCNTL_H)
-
-# We need the following in order to create <fcntl.h> when the system
-# doesn't have one that works with the given compiler.
-fcntl.h: fcntl.in.h
-       rm -f address@hidden $@
-       { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
-         sed -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
-             -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \
-             -e 's|@''NEXT_FCNTL_H''@|$(NEXT_FCNTL_H)|g' \
-             -e 's|@''GNULIB_OPEN''@|$(GNULIB_OPEN)|g' \
-             -e 's|@''REPLACE_OPEN''@|$(REPLACE_OPEN)|g' \
-             < $(srcdir)/fcntl.in.h; \
-       } > address@hidden
-       mv address@hidden $@
-MOSTLYCLEANFILES += fcntl.h fcntl.h-t

 Include:
 #include <fcntl.h>
@@ -38,4 +17,4 @@ License:
 LGPL

 Maintainer:
-all
+Eric Blake
diff --git a/modules/fcntl b/modules/fcntl-h
similarity index 100%
copy from modules/fcntl
copy to modules/fcntl-h
diff --git a/modules/fcntl-h-tests b/modules/fcntl-h-tests
new file mode 100644
index 0000000..f0cf173
--- /dev/null
+++ b/modules/fcntl-h-tests
@@ -0,0 +1,13 @@
+Files:
+tests/test-fcntl-h.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fcntl-h
+check_PROGRAMS += test-fcntl-h
+
+License:
+LGPL
diff --git a/modules/fcntl-safer b/modules/fcntl-safer
index b9da466..918223e 100644
--- a/modules/fcntl-safer
+++ b/modules/fcntl-safer
@@ -10,7 +10,7 @@ m4/fcntl-safer.m4
 m4/mode_t.m4

 Depends-on:
-fcntl
+fcntl-h
 open
 unistd-safer

diff --git a/modules/fcntl-tests b/modules/fcntl-tests
deleted file mode 100644
index 86d91e5..0000000
--- a/modules/fcntl-tests
+++ /dev/null
@@ -1,13 +0,0 @@
-Files:
-tests/test-fcntl.c
-
-Depends-on:
-
-configure.ac:
-
-Makefile.am:
-TESTS += test-fcntl
-check_PROGRAMS += test-fcntl
-
-License:
-LGPL
diff --git a/modules/fts b/modules/fts
index 2cbed1c..38b2256 100644
--- a/modules/fts
+++ b/modules/fts
@@ -13,7 +13,7 @@ d-ino
 d-type
 dirfd
 fchdir
-fcntl
+fcntl-h
 fcntl-safer
 hash
 i-ring
diff --git a/modules/mkancesdirs b/modules/mkancesdirs
index 5bb1ea7..5a5c66a 100644
--- a/modules/mkancesdirs
+++ b/modules/mkancesdirs
@@ -8,7 +8,7 @@ m4/mkancesdirs.m4

 Depends-on:
 dirname
-fcntl
+fcntl-h
 savewd
 stat-macros
 sys_stat
diff --git a/modules/mkdir-p b/modules/mkdir-p
index 31e873c..20e3a23 100644
--- a/modules/mkdir-p
+++ b/modules/mkdir-p
@@ -10,7 +10,7 @@ m4/mkdir-p.m4

 Depends-on:
 error
-fcntl
+fcntl-h
 gettext-h
 lchmod
 lchown
diff --git a/modules/open b/modules/open
index e89efde..601e064 100644
--- a/modules/open
+++ b/modules/open
@@ -7,7 +7,7 @@ m4/open.m4
 m4/mode_t.m4

 Depends-on:
-fcntl
+fcntl-h

 configure.ac:
 gl_FUNC_OPEN
diff --git a/modules/savewd b/modules/savewd
index c2fd42f..360d4b9 100644
--- a/modules/savewd
+++ b/modules/savewd
@@ -11,7 +11,7 @@ dirname
 exit
 fchdir
 fcntl-safer
-fcntl
+fcntl-h
 raise
 stdbool
 xalloc
diff --git a/tests/test-fcntl.c b/tests/test-fcntl-h.c
similarity index 89%
rename from tests/test-fcntl.c
rename to tests/test-fcntl-h.c
index 449984c..127f497 100644
--- a/tests/test-fcntl.c
+++ b/tests/test-fcntl-h.c
@@ -1,5 +1,5 @@
 /* Test of <fcntl.h> substitute.
-   Copyright (C) 2007 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
@@ -28,6 +28,9 @@ int o = O_DIRECT | O_DIRECTORY | O_DSYNC | O_NDELAY | 
O_NOATIME | O_NONBLOCK
 /* Check that the various SEEK_* macros are defined.  */
 int sk[] = { SEEK_CUR, SEEK_END, SEEK_SET };

+/* Check that the FD_* macros are defined.  */
+int fd = FD_CLOEXEC;
+
 int
 main ()
 {
-- 
1.6.3.3.334.g916e1


>From faf85c9f5beb452af09ca4488a4ba36c82c06573 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 21 Aug 2009 13:16:31 -0600
Subject: [PATCH 2/3] fcntl-tests: new module, still fails on mingw

* tests/test-fcntl.c: New file.
* modules/fcntl-tests: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog           |    4 +
 modules/fcntl-tests |   15 ++++
 tests/test-fcntl.c  |  193 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+), 0 deletions(-)
 create mode 100644 modules/fcntl-tests
 create mode 100644 tests/test-fcntl.c

diff --git a/ChangeLog b/ChangeLog
index 95da7c0..0364f65 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2009-08-21  Eric Blake  <address@hidden>

+       fcntl-tests: new module, still fails on mingw
+       * tests/test-fcntl.c: New file.
+       * modules/fcntl-tests: Likewise.
+
        fcntl-h: rename from fcntl, in preparation for fcntl(2)
        * modules/fcntl: Move <fcntl.h> header replacement...
        * modules/fcntl-h: ...to new name, so as not to collide with
diff --git a/modules/fcntl-tests b/modules/fcntl-tests
new file mode 100644
index 0000000..4ecfacd
--- /dev/null
+++ b/modules/fcntl-tests
@@ -0,0 +1,15 @@
+Files:
+tests/test-fcntl.c
+
+Depends-on:
+execute
+intprops
+open
+progname
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fcntl
+check_PROGRAMS += test-fcntl
+test_fcntl_LDADD = $(LDADD) @LIBINTL@
diff --git a/tests/test-fcntl.c b/tests/test-fcntl.c
new file mode 100644
index 0000000..6bbc915
--- /dev/null
+++ b/tests/test-fcntl.c
@@ -0,0 +1,193 @@
+/* Test of fcntl(2).
+   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 <address@hidden>, 2009.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include <fcntl.h>
+
+/* Helpers.  */
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "execute.h"
+#include "intprops.h"
+
+#define ASSERT(expr) \
+  do                                                                        \
+    {                                                                       \
+      if (!(expr))                                                          \
+        {                                                                   \
+          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+          fflush (stderr);                                                  \
+          abort ();                                                         \
+        }                                                                   \
+    }                                                                       \
+  while (0)
+
+/* Use O_CLOEXEC if available, but test works without it.  */
+#ifndef O_CLOEXEC
+# define O_CLOEXEC 0
+#endif
+
+
+/* Ensure that all supported fcntl actions are distinct, and
+   usable in preprocessor expressions.  */
+static void
+check_flags (void)
+{
+  switch (0)
+    {
+    case F_DUPFD:
+#if F_DUPFD
+#endif
+
+#ifdef F_DUPFD_CLOEXEC
+    case F_DUPFD_CLOEXEC:
+# if F_DUPFD_CLOEXEC
+# endif
+#endif
+
+    case F_GETFD:
+#if F_GETFD
+#endif
+
+    case F_SETFD:
+#if F_SETFD
+#endif
+
+#ifdef F_GETFL
+    case F_GETFL:
+# if F_GETFL
+# endif
+#endif
+
+#ifdef F_SETFL
+    case F_SETFL:
+# if F_SETFL
+# endif
+#endif
+
+#ifdef F_GETOWN
+    case F_GETOWN:
+# if F_GETOWN
+# endif
+#endif
+
+#ifdef F_SETOWN
+    case F_SETOWN:
+# if F_SETOWN
+# endif
+#endif
+
+#ifdef F_GETLK
+    case F_GETLK:
+# if F_GETLK
+# endif
+#endif
+
+#ifdef F_SETLK
+    case F_SETLK:
+# if F_SETLK
+# endif
+#endif
+
+#ifdef F_SETLKW
+    case F_SETLKW:
+# if F_SETLKW
+# endif
+#endif
+
+      ;
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  int flags;
+  if (argc == 1)
+    {
+      /* Parent.  Because of incomplete O_CLOEXEC support, we don't
+        know the initial cloexec state of fd0.  But we _do_ know what
+        the state of fd1 and fd2 should be.  We then spawn ourself as
+        a child with fd1 and fd2 as arguments.  */
+      int fd0 = open ("/dev/null", O_RDONLY | O_CLOEXEC);
+      int fd1;
+      int fd2;
+      char *subargv[4];
+      char fdstr1[INT_BUFSIZE_BOUND(int)];
+      char fdstr2[INT_BUFSIZE_BOUND(int)];
+
+      ASSERT (0 <= fd0);
+      flags = fcntl (fd0, F_GETFD);
+      ASSERT (0 <= flags);
+      /* FIXME - for now, we don't work around platforms with this bug.  */
+      /* ASSERT (fcntl (fd0, F_DUPFD, -1) == -1); */
+      /* ASSERT (errno == EINVAL); */
+      ASSERT (fcntl (-1, F_DUPFD, 0) == -1);
+      ASSERT (errno == EBADF);
+
+      fd1 = fcntl (fd0, F_DUPFD, fd0);
+      ASSERT (fd0 < fd1);
+      ASSERT (close (fd0) == 0);
+      flags = fcntl (fd1, F_GETFD);
+      ASSERT (0 <= flags && (flags & FD_CLOEXEC) == 0);
+      ASSERT (fcntl (fd1, F_SETFD, flags | FD_CLOEXEC) != -1);
+      ASSERT (fcntl (fd1, F_GETFD) == (flags | FD_CLOEXEC));
+      ASSERT (fcntl (fd1, F_SETFD, flags & ~FD_CLOEXEC) != -1);
+      ASSERT (fcntl (fd1, F_GETFD) == flags);
+      /* FIXME - remove #else clause once F_DUPFD_CLOEXEC is supported.  */
+#ifdef F_DUPFD_CLOEXEC
+      fd2 = fcntl (fd1, F_DUPFD_CLOEXEC, fd1);
+      ASSERT (fd1 < fd2);
+      flags = fcntl (fd2, F_GETFD);
+      ASSERT (0 <= flags && (flags & FD_CLOEXEC) == FD_CLOEXEC);
+#else
+      fd2 = fcntl (fd1, F_DUPFD, fd1);
+      ASSERT (fd1 < fd2);
+      flags = fcntl (fd2, F_GETFD);
+      ASSERT (0 <= flags);
+      ASSERT (fcntl (fd2, F_SETFD, flags | FD_CLOEXEC) != -1);
+#endif
+
+      /* Have to use execute, rather than simpler fork/exec, so that
+        this test can pass on mingw.  */
+      subargv[0] = argv[0];
+      subargv[1] = fdstr1;
+      subargv[2] = fdstr2;
+      subargv[3] = NULL;
+      ASSERT (0 < snprintf (fdstr1, sizeof fdstr1, "%d", fd1));
+      ASSERT (0 < snprintf (fdstr2, sizeof fdstr2, "%d", fd2));
+      ASSERT (execute (argv[0], argv[0], subargv, false, false, false, false,
+                      true, true, NULL) == 0);
+      check_flags ();
+    }
+  else
+    {
+      /* Child.  argv[1] should be closed, and argv[2] open.  */
+      ASSERT (argc == 3);
+      flags = fcntl (atoi (argv[1]), F_GETFD);
+      ASSERT (flags != -1 && (flags & FD_CLOEXEC) == 0);
+      ASSERT (fcntl (atoi (argv[2]), F_GETFD) == -1);
+      ASSERT (errno == EBADF);
+    }
+  return 0;
+}
-- 
1.6.3.3.334.g916e1


>From e15b8197442c0f174fcdd8045d636a4f21e4e299 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 21 Aug 2009 15:19:25 -0600
Subject: [PATCH 3/3] fcntl: new module, for fd manipulation on mingw

* m4/fcntl.m4 (gl_FUNC_FCNTL): New file.
* lib/fcntl.c: Likewise.
* modules/fcntl (Files): Mention new files.
(configure.ac): Support fcntl on mingw.
* m4/fcntl_h.m4 (gl_FCNTL_H): Remove unused variable.
(gl_FCNTL_H_DEFAULTS): Add witnesses.
* modules/fcntl-h (Makefile.am): Substitute them.
(configure.ac): Make fcntl replacement unconditional.
(Depends-on): Add link-warning.
* lib/fcntl.in.h (fcntl, F_DUPFD, F_GETFD, F_SETFD): Declare
replacements.
* doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.
* doc/posix-functions/fcntl.texi (fcntl): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                      |   15 +++++
 doc/posix-functions/fcntl.texi |   19 +++++-
 doc/posix-headers/fcntl.texi   |    8 +-
 lib/fcntl.c                    |  121 ++++++++++++++++++++++++++++++++++++++++
 lib/fcntl.in.h                 |   38 ++++++++++++-
 m4/fcntl.m4                    |   21 +++++++
 m4/fcntl_h.m4                  |    4 +-
 modules/fcntl                  |    6 ++-
 modules/fcntl-h                |    6 ++-
 9 files changed, 225 insertions(+), 13 deletions(-)
 create mode 100644 lib/fcntl.c
 create mode 100644 m4/fcntl.m4

diff --git a/ChangeLog b/ChangeLog
index 0364f65..79231da 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2009-08-21  Eric Blake  <address@hidden>

+       fcntl: new module, for fd manipulation on mingw
+       * m4/fcntl.m4 (gl_FUNC_FCNTL): New file.
+       * lib/fcntl.c: Likewise.
+       * modules/fcntl (Files): Mention new files.
+       (configure.ac): Support fcntl on mingw.
+       * m4/fcntl_h.m4 (gl_FCNTL_H): Remove unused variable.
+       (gl_FCNTL_H_DEFAULTS): Add witnesses.
+       * modules/fcntl-h (Makefile.am): Substitute them.
+       (configure.ac): Make fcntl replacement unconditional.
+       (Depends-on): Add link-warning.
+       * lib/fcntl.in.h (fcntl, F_DUPFD, F_GETFD, F_SETFD): Declare
+       replacements.
+       * doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.
+       * doc/posix-functions/fcntl.texi (fcntl): Likewise.
+
        fcntl-tests: new module, still fails on mingw
        * tests/test-fcntl.c: New file.
        * modules/fcntl-tests: Likewise.
diff --git a/doc/posix-functions/fcntl.texi b/doc/posix-functions/fcntl.texi
index 143ac63..e4c6d43 100644
--- a/doc/posix-functions/fcntl.texi
+++ b/doc/posix-functions/fcntl.texi
@@ -4,15 +4,28 @@ fcntl

 POSIX specification: 
@url{http://www.opengroup.org/onlinepubs/9699919799/functions/fcntl.html}

-Gnulib module: ---
+Gnulib module: fcntl

 Portability problems fixed by Gnulib:
 @itemize
address@hidden
+This function is missing on some platforms:
+mingw.
 @end itemize

 Portability problems not fixed by Gnulib:
 @itemize
 @item
-This function is missing on some platforms:
-mingw.
+This function does not support @code{F_DUPFD_CLOEXEC} on many
+platforms.
+
address@hidden
+This function does not reject negative targets for @code{F_DUPFD} on
+some platforms.
+
address@hidden
+This function does not support @code{F_GETFL}, @code{F_SETFL},
address@hidden, @code{F_SETOWN}, @code{F_GETLK}, @code{F_SETLK},
+and @code{F_SETLKW} on some platforms:
+mingw
 @end itemize
diff --git a/doc/posix-headers/fcntl.texi b/doc/posix-headers/fcntl.texi
index f2d5fb2..af4a40a 100644
--- a/doc/posix-headers/fcntl.texi
+++ b/doc/posix-headers/fcntl.texi
@@ -23,7 +23,8 @@ fcntl.h
 on some platforms but not on others.

 @item
address@hidden is not defined on some platforms:
address@hidden, @samp{F_DUPFD}, @samp{F_GETFD}, and @samp{F_SETFD}
+are not defined on some platforms:
 mingw

 @item
@@ -41,9 +42,8 @@ fcntl.h
 on some platforms.

 @item
address@hidden, @samp{F_DUPFD_CLOEXEC}, @samp{F_GETFD}, and
address@hidden are not defined on some platforms:
-mingw
address@hidden is not defined on many platforms, and cannot be
+replaced atomically.

 @item
 @samp{F_GETFL}, @samp{F_SETFL}, @samp{F_GETLK}, @samp{F_SETLK},
diff --git a/lib/fcntl.c b/lib/fcntl.c
new file mode 100644
index 0000000..048955a
--- /dev/null
+++ b/lib/fcntl.c
@@ -0,0 +1,121 @@
+/* Provide file descriptor control.
+
+   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 <address@hidden>.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include <fcntl.h>
+
+#include <errno.h>
+#include <stdarg.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+/* Get declarations of the Win32 API functions.  */
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#else
+# error For now, this file is only ported to mingw.
+#endif
+
+/* Duplicate FD into the first available slot of at least DESIRED_FD,
+   which must be positive.  */
+static int
+dupfd (int fd, int desired_fd)
+{
+  /* Although this looks like unbounded recursion, mingw only supports
+     a maximum of 2047, so stack overflow is unlikely.  */
+  int duplicated_fd = dup (fd);
+  if (duplicated_fd < 0 || desired_fd <= duplicated_fd)
+    return duplicated_fd;
+  else
+    {
+      int r = dupfd (fd, desired_fd);
+      int e = errno;
+      close (duplicated_fd);
+      errno = e;
+      return r;
+    }
+}
+
+/* Perform low-level file descriptor ACTION (from F_*) on FD.  */
+int
+rpl_fcntl (int fd, int action, ...)
+{
+  va_list arg;
+  int result = -1;
+  va_start (arg, action);
+  switch (action)
+    {
+    case F_DUPFD:
+      {
+       int minimum = va_arg (arg, int);
+       if (minimum < 0)
+         errno = EINVAL;
+       else
+         {
+           result = dupfd (fd, minimum);
+           if (0 <= result)
+             {
+               /* In mingw, a duplicated handle copies the
+                  O_NOINHERIT state of the original; however, we want
+                  F_DUPFD to clear FD_CLOEXEC status.  */
+               int e = errno;
+               rpl_fcntl (fd, F_SETFD, 0);
+               errno = e;
+             }
+         }
+       break;
+      }
+    case F_GETFD:
+      {
+       HANDLE handle = (HANDLE) _get_osfhandle (fd);
+       DWORD flags;
+       if (handle == INVALID_HANDLE_VALUE
+           || GetHandleInformation (handle, &flags) == 0)
+         errno = EBADF;
+       else
+         result = (flags & HANDLE_FLAG_INHERIT) ? 0 : FD_CLOEXEC;
+       break;
+      }
+    case F_SETFD:
+      {
+       int value = va_arg (arg, int);
+       if (value != 0 && value != FD_CLOEXEC)
+         errno = EINVAL;
+       else
+         {
+           HANDLE handle = (HANDLE) _get_osfhandle (fd);
+           DWORD flags = value ? 0 : HANDLE_FLAG_INHERIT;
+           if (handle == INVALID_HANDLE_VALUE
+               || SetHandleInformation (handle, HANDLE_FLAG_INHERIT,
+                                        flags) == 0)
+             errno = EBADF;
+           else
+             result = 0;
+         }
+       break;
+      }
+    default:
+      result = -1;
+      errno = EINVAL;
+      break;
+    }
+  va_end (arg);
+  return result;
+}
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index a688fb4..be3f4bb 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -1,6 +1,6 @@
 /* Like <fcntl.h>, but with non-working flags defined to 0.

-   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
@@ -44,12 +44,30 @@
 #define _GL_FCNTL_H


+/* The definition of GL_LINK_WARNING is copied here.  */
+
+
 /* Declare overridden functions.  */

 #ifdef __cplusplus
 extern "C" {
 #endif

+#if @GNULIB_FCNTL@
+# if address@hidden@
+#  define fcntl rpl_fcntl
+/* Perform low-level file descriptor ACTION (from F_*) on FD.  */
+extern int fcntl (int fd, int action, ...);
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef fcntl
+# define fcntl \
+    (GL_LINK_WARNING ("fcntl is unportable - " \
+                      "use gnulib module fcntl for better " \
+                      "POSIX compliance"), \
+     fcntl)
+#endif
+
 #if @GNULIB_OPEN@
 # if @REPLACE_OPEN@
 #  undef open
@@ -67,7 +85,23 @@ extern void _gl_register_fd (int fd, const char *filename);
 }
 #endif

-/* Fix up the FD_* macros.  */
+/* Fix up the F_* macros.  Mingw lacks them all, and many platforms
+   lack F_DUPFD_CLOEXEC.  Macros not listed here are intentionally
+   left undefined.  */
+
+#ifndef F_DUPFD
+# define F_DUPFD 1
+#endif
+
+#ifndef F_GETFD
+# define F_GETFD 2
+#endif
+
+#ifndef F_SETFD
+# define F_SETFD 3
+#endif
+
+/* Fix up the FD_* macros, missing only on mingw.  */

 #ifndef FD_CLOEXEC
 # define FD_CLOEXEC 1
diff --git a/m4/fcntl.m4 b/m4/fcntl.m4
new file mode 100644
index 0000000..c0e9070
--- /dev/null
+++ b/m4/fcntl.m4
@@ -0,0 +1,21 @@
+# fcntl.m4 serial 1
+dnl Copyright (C) 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.
+
+# For now, this module ensures
+# - the fcntl() function supports F_GETFD, F_SETFD, F_DUPFD
+# Still to be ported:
+# - F_DUPFD_CLOEXEC
+# - F_GETFL, F_SETFL
+# - F_GETOWN, F_SETOWN
+# - F_GETLK, F_SETLK, F_SETLKW
+AC_DEFUN([gl_FUNC_FCNTL],
+[
+  AC_REQUIRE([gl_FCNTL_H_DEFAULTS])
+  AC_REPLACE_FUNCS([fcntl])
+  if test $ac_cv_func_fcntl = no; then
+    HAVE_FCNTL=0
+  fi
+])
diff --git a/m4/fcntl_h.m4 b/m4/fcntl_h.m4
index 1ae0b15..2fb0746 100644
--- a/m4/fcntl_h.m4
+++ b/m4/fcntl_h.m4
@@ -77,8 +77,6 @@ AC_DEFUN([gl_FCNTL_H],
     [Define to 1 if O_NOFOLLOW works.])

   gl_CHECK_NEXT_HEADERS([fcntl.h])
-  FCNTL_H='fcntl.h'
-  AC_SUBST([FCNTL_H])
 ])

 AC_DEFUN([gl_FCNTL_MODULE_INDICATOR],
@@ -90,7 +88,9 @@ AC_DEFUN([gl_FCNTL_MODULE_INDICATOR],

 AC_DEFUN([gl_FCNTL_H_DEFAULTS],
 [
+  GNULIB_FCNTL=0; AC_SUBST([GNULIB_FCNTL])
   GNULIB_OPEN=0;  AC_SUBST([GNULIB_OPEN])
   dnl Assume proper GNU behavior unless another module says otherwise.
+  HAVE_FCNTL=1;   AC_SUBST([HAVE_FCNTL])
   REPLACE_OPEN=0; AC_SUBST([REPLACE_OPEN])
 ])
diff --git a/modules/fcntl b/modules/fcntl
index a63c4e3..e06444c 100644
--- a/modules/fcntl
+++ b/modules/fcntl
@@ -1,12 +1,16 @@
 Description:
-Placeholder for eventual fcntl() replacement.
+Support for fcntl() actions F_GETFD, F_SETFD, F_DUPFD.

 Files:
+m4/fcntl.m4
+lib/fcntl.c

 Depends-on:
 fcntl-h

 configure.ac:
+gl_FUNC_FCNTL
+gl_FCNTL_MODULE_INDICATOR([fcntl])

 Makefile.am:

diff --git a/modules/fcntl-h b/modules/fcntl-h
index de0aeb9..bae0a9b 100644
--- a/modules/fcntl-h
+++ b/modules/fcntl-h
@@ -7,6 +7,7 @@ m4/fcntl_h.m4

 Depends-on:
 include_next
+link-warning
 unistd
 extensions

@@ -14,7 +15,7 @@ configure.ac:
 gl_FCNTL_H

 Makefile.am:
-BUILT_SOURCES += $(FCNTL_H)
+BUILT_SOURCES += fcntl.h

 # We need the following in order to create <fcntl.h> when the system
 # doesn't have one that works with the given compiler.
@@ -24,8 +25,11 @@ fcntl.h: fcntl.in.h
          sed -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
              -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \
              -e 's|@''NEXT_FCNTL_H''@|$(NEXT_FCNTL_H)|g' \
+             -e 's|@''GNULIB_FCNTL''@|$(GNULIB_FCNTL)|g' \
              -e 's|@''GNULIB_OPEN''@|$(GNULIB_OPEN)|g' \
+             -e 's|@''HAVE_FCNTL''@|$(HAVE_FCNTL)|g' \
              -e 's|@''REPLACE_OPEN''@|$(REPLACE_OPEN)|g' \
+             -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
              < $(srcdir)/fcntl.in.h; \
        } > address@hidden
        mv address@hidden $@
-- 
1.6.3.3.334.g916e1