[PATCH] use gnulib modules close (new) and open to hook into open/close

[Paolo Bonzini]

This patch replaces the two separate replacements of close in winsock.c
and fchdir.c with a module close that knows how to weave the two
preexisting pieces of code.  Likewise, it changes the open replacement
of fchdir.c into a small function called by the generic open function.

As a nice side effect, knowledge of fchdir disappears from the fcntl_h
module, and so does duplicated code between open.c and fchdir.c

The Winsock replacement of close is triggered by the socket and accept
modules.  It could be possible to make it always trigger on mingw, but
that would make fchdir dependent on sys_socket_h and winsock.c, which I
explicitly wanted to avoid.  I also wanted to avoid other ugly solutions
such as making a "close-impl" module on which fchdir depends and making
"close" trigger the Winsock replacement.  Hints on how to fix this, and
whether it is worth it, are welcome.

Most of the code is just boring setup for the new things to work
properly.  In particular, whenever I need to force the replacement of
open and close I use a macro gl_REPLACE_{OPEN,CLOSE} to avoid breaking
encapsulation of m4/{open,close}.m4.

Ok?

Paolo

2008-10-07  Paolo Bonzini  <address@hidden>
    
        * lib/fchdir.c (gl_free_fd): New, from part of rpl_close.
        (_gl_register_fd): New, from part of rpl_open.
        (rpl_opendir, rpl_closedir): Use them.
        * lib/winsock.c (_gl_close_fd): New name of rpl_close.
        * lib/close.c: New.
        * lib/open.c: Include the function independent of FCHDIR_REPLACEMENT.
        Use _gl_register_fd if needed.
        * lib/fcntl.in.h: Do not check FCHDIR_REPLACEMENT to check whether
        open should be replaced.
        * lib/sys_socket.in.h: Do not replace close.
        * lib/unistd.in.h: Add separate if-ology for close, instead of having
        it as part of the fchdir replacement.
    
        * m4/close.m4: New.
        * m4/fchdir.m4: Force replacing open and close if fchdir.c is needed.
        * m4/open.m4: Provide and use gl_REPLACE_OPEN.  Add a : to make the
        body of gl_PREREQ_OPEN non-empty.
        * m4/unistd_h.m4: Add substed variables for close.
    
        * modules/close: New.
        * modules/accept: Depend on close and force its replacement if Winsock.
        * modules/fchdir: Depend on close.
        * modules/socket: Depend on close and force its replacement if Winsock.
        * modules/unistd: Replace substed variables for close in unistd.h.

diff --git a/lib/close.c b/lib/close.c
new file mode 100644
index 0000000..e87a32f
--- /dev/null
+++ b/lib/close.c
@@ -0,0 +1,47 @@
+/* close replacement.
+   Copyright (C) 2008 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/>.  */
+
+#include <config.h>
+#include <unistd.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# if defined GNULIB_SOCKET || defined GNULIB_ACCEPT
+#  define GNULIB_NEED_CLOSE_FD 1
+# endif
+#endif
+
+/* Override close() to call into other gnulib modules.  */
+
+int
+rpl_close (int fd)
+#undef close
+{
+#ifdef GNULIB_NEED_CLOSE_FD
+  extern int _gl_close_fd (int fd);
+  int retval = _gl_close_fd (fd);
+#else
+  int retval = close (fd);
+#endif
+
+#ifdef FCHDIR_REPLACEMENT
+  if (retval >= 0)
+    {
+      extern int _gl_free_fd (int fd);
+      _gl_free_fd (fd);
+    }
+#endif
+  return retval;
+}
diff --git a/lib/fchdir.c b/lib/fchdir.c
index 246987c..eb06aed 100644
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -75,64 +75,39 @@ ensure_dirs_slot (size_t fd)
     }
 }
 
-/* Override open() and close(), to keep track of the open file descriptors.  */
+/* Hook into the gnulib replacements for open() and close() to keep track
+   of the open file descriptors.  */
+
+/* We do not want to declare these anywhere, but we want prototypes to
+   shut up -Wimplicit.  */
+extern int _gl_free_fd (int fd);
+extern void _gl_register_fd (int fd, const char *filename);
 
 int
-rpl_close (int fd)
-#undef close
+_gl_free_fd (int fd)
 {
-  int retval = close (fd);
-
-  if (retval >= 0 && fd >= 0 && fd < dirs_allocated)
+  if (fd >= 0 && fd < dirs_allocated)
     {
       if (dirs[fd].name != NULL)
        free (dirs[fd].name);
       dirs[fd].name = NULL;
       dirs[fd].saved_errno = ENOTDIR;
     }
-  return retval;
 }
 
-int
-rpl_open (const char *filename, int flags, ...)
-#undef open
+void
+_gl_register_fd (int fd, const char *filename)
 {
-  mode_t mode;
-  int fd;
   struct stat statbuf;
 
-  mode = 0;
-  if (flags & O_CREAT)
+  ensure_dirs_slot (fd);
+  if (fd < dirs_allocated
+      && fstat (fd, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode))
     {
-      va_list arg;
-      va_start (arg, flags);
-
-      /* If mode_t is narrower than int, use the promoted type (int),
-        not mode_t.  Use sizeof to guess whether mode_t is narrower;
-        we don't know of any practical counterexamples.  */
-      mode = (sizeof (mode_t) < sizeof (int)
-             ? va_arg (arg, int)
-             : va_arg (arg, mode_t));
-
-      va_end (arg);
-    }
-#if defined GNULIB_OPEN && ((defined _WIN32 || defined __WIN32__) && ! defined 
__CYGWIN__)
-  if (strcmp (filename, "/dev/null") == 0)
-    filename = "NUL";
-#endif
-  fd = open (filename, flags, mode);
-  if (fd >= 0)
-    {
-      ensure_dirs_slot (fd);
-      if (fd < dirs_allocated
-         && fstat (fd, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode))
-       {
-         dirs[fd].name = canonicalize_file_name (filename);
-         if (dirs[fd].name == NULL)
-           dirs[fd].saved_errno = errno;
-       }
+      dirs[fd].name = canonicalize_file_name (filename);
+      if (dirs[fd].name == NULL)
+        dirs[fd].saved_errno = errno;
     }
-  return fd;
 }
 
 /* Override opendir() and closedir(), to keep track of the open file
@@ -146,12 +121,7 @@ rpl_closedir (DIR *dp)
   int retval = closedir (dp);
 
   if (retval >= 0 && fd >= 0 && fd < dirs_allocated)
-    {
-      if (dirs[fd].name != NULL)
-       free (dirs[fd].name);
-      dirs[fd].name = NULL;
-      dirs[fd].saved_errno = ENOTDIR;
-    }
+    _gl_free_fd (fd);
   return retval;
 }
 
@@ -166,15 +136,7 @@ rpl_opendir (const char *filename)
     {
       int fd = dirfd (dp);
       if (fd >= 0)
-       {
-         ensure_dirs_slot (fd);
-         if (fd < dirs_allocated)
-           {
-             dirs[fd].name = canonicalize_file_name (filename);
-             if (dirs[fd].name == NULL)
-               dirs[fd].saved_errno = errno;
-           }
-       }
+       _gl_register_fd (fd, filename);
     }
   return dp;
 }
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index 79b8f16..4f94195 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -48,10 +48,12 @@
 extern "C" {
 #endif
 
-#if (@GNULIB_OPEN@ && @REPLACE_OPEN@) || defined FCHDIR_REPLACEMENT
-# undef open
-# define open rpl_open
+#if @GNULIB_OPEN@
+# if @REPLACE_OPEN@
+#  undef open
+#  define open rpl_open
 extern int open (const char *filename, int flags, ...);
+# endif
 #endif
 
 #ifdef __cplusplus
diff --git a/lib/open.c b/lib/open.c
index 9a35f5b..6295c52 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -33,9 +33,6 @@ orig_open (const char *filename, int flags, mode_t mode)
 /* Specification.  */
 #include <fcntl.h>
 
-/* If the fchdir replacement is used, open() is defined in fchdir.c.  */
-#ifndef FCHDIR_REPLACEMENT
-
 # include <errno.h>
 # include <stdarg.h>
 # include <string.h>
@@ -43,7 +40,7 @@ orig_open (const char *filename, int flags, mode_t mode)
 # include <sys/stat.h>
 
 int
-open (const char *filename, int flags, ...)
+rpl_open (const char *filename, int flags, ...)
 {
   mode_t mode;
   int fd;
@@ -131,7 +128,13 @@ open (const char *filename, int flags, ...)
        }
     }
 # endif
+#ifdef FCHDIR_REPLACEMENT
+  if (fd >= 0)
+    {
+      extern void _gl_register_fd (int, const char *);
+      _gl_register_fd (fd, filename);
+    }
+#endif
 
   return fd;
 }
-#endif
diff --git a/lib/sys_socket.in.h b/lib/sys_socket.in.h
index 13729c6..d364cca 100644
--- a/lib/sys_socket.in.h
+++ b/lib/sys_socket.in.h
@@ -138,12 +138,6 @@ rpl_fd_isset (int fd, fd_set * set)
 
 /* Wrap everything else to use libc file descriptors for sockets.  */
 
-# if @HAVE_WINSOCK2_H@
-#  undef close
-#  define close                        rpl_close
-extern int rpl_close(int);
-# endif
-
 # if @GNULIB_SOCKET@
 #  if @HAVE_WINSOCK2_H@
 #   undef socket
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index 82b1f0a..06f3d69 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -112,6 +112,14 @@ extern char **environ;
      environ)
 #endif
 
+#if @GNULIB_CLOSE@
+# if @REPLACE_CLOSE@
+/* Automatically included by modules that need a replacement for close.  */
+
+#  define close rpl_close
+extern int close (int);
+# endif
+#endif
 
 #if @GNULIB_FCHDIR@
 # if @REPLACE_FCHDIR@
@@ -123,8 +131,6 @@ extern char **environ;
    <http://www.opengroup.org/susv3xsh/fchdir.html>.  */
 extern int fchdir (int /*fd*/);
 
-#  define close rpl_close
-extern int close (int);
 #  define dup rpl_dup
 extern int dup (int);
 #  define dup2 rpl_dup2
diff --git a/lib/winsock.c b/lib/winsock.c
index 00609cf..ec8e239 100644
--- a/lib/winsock.c
+++ b/lib/winsock.c
@@ -44,10 +44,12 @@
 # define SOCKET_TO_FD(fh)   (_open_osfhandle ((long) (fh), O_RDWR | O_BINARY))
 
 
-/* Wrappers for libc functions.  */
+/* Hook for gnulib module close.  */
+
+extern int _gl_close_fd (int fd);
 
 int
-rpl_close (int fd)
+_gl_close_fd (int fd)
 {
   SOCKET sock = FD_TO_SOCKET (fd);
   WSANETWORKEVENTS ev;
diff --git a/m4/close.m4 b/m4/close.m4
new file mode 100644
index 0000000..90be78a
--- /dev/null
+++ b/m4/close.m4
@@ -0,0 +1,18 @@
+# close.m4 serial 1
+dnl Copyright (C) 2008 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.
+
+AC_DEFUN([gl_FUNC_CLOSE],
+[
+  AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+])
+
+AC_DEFUN([gl_REPLACE_CLOSE],
+[
+  if test $REPLACE_CLOSE != 1; then
+    AC_LIBOBJ([close])
+  fi
+  REPLACE_CLOSE=1
+])
diff --git a/m4/fchdir.m4 b/m4/fchdir.m4
index fd170d2..98bb5fa 100644
--- a/m4/fchdir.m4
+++ b/m4/fchdir.m4
@@ -7,11 +7,20 @@ dnl with or without modifications, as long as this notice is 
preserved.
 AC_DEFUN([gl_FUNC_FCHDIR],
 [
   AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+  AC_REQUIRE([gl_FUNC_CLOSE])
+  AC_REQUIRE([gl_FUNC_OPEN])
+  AC_REQUIRE([gl_FUNC_FCHDIR_BODY])
+])
+
+AC_DEFUN([gl_FUNC_FCHDIR_BODY],
+[
   AC_CHECK_FUNCS_ONCE([fchdir])
   if test $ac_cv_func_fchdir = no; then
     REPLACE_FCHDIR=1
     AC_LIBOBJ([fchdir])
     gl_PREREQ_FCHDIR
+    gl_REPLACE_OPEN
+    gl_REPLACE_CLOSE
     AC_DEFINE([FCHDIR_REPLACEMENT], 1,
       [Define if gnulib's fchdir() replacement is used.])
     gl_CHECK_NEXT_HEADERS([dirent.h])
diff --git a/m4/open.m4 b/m4/open.m4
index fab0ad9..9967c2a 100644
--- a/m4/open.m4
+++ b/m4/open.m4
@@ -10,8 +10,7 @@ AC_DEFUN([gl_FUNC_OPEN],
   AC_REQUIRE([AC_CANONICAL_HOST])
   case "$host_os" in
     mingw* | pw*)
-      REPLACE_OPEN=1
-      AC_LIBOBJ([open])
+      gl_REPLACE_OPEN
       ;;
     *)
       dnl open("foo/") should not create a file when the file name has a
@@ -43,17 +42,25 @@ changequote([,])dnl
         *no)
           AC_DEFINE([OPEN_TRAILING_SLASH_BUG], 1,
             [Define to 1 if open() fails to recognize a trailing slash.])
-          REPLACE_OPEN=1
-          AC_LIBOBJ([open])
-          gl_PREREQ_OPEN
+          gl_REPLACE_OPEN
           ;;
       esac
       ;;
   esac
 ])
 
+AC_DEFUN([gl_REPLACE_OPEN],
+[
+  if test $REPLACE_OPEN != 1; then
+    AC_LIBOBJ([open])
+    gl_PREREQ_OPEN
+  fi
+  REPLACE_OPEN=1
+])
+
 # Prerequisites of lib/open.c.
 AC_DEFUN([gl_PREREQ_OPEN],
 [
   AC_REQUIRE([AC_C_INLINE])
+  :
 ])
diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
index e6eb045..093179c 100644
--- a/m4/unistd_h.m4
+++ b/m4/unistd_h.m4
@@ -33,6 +33,7 @@ AC_DEFUN([gl_UNISTD_MODULE_INDICATOR],
 AC_DEFUN([gl_UNISTD_H_DEFAULTS],
 [
   GNULIB_CHOWN=0;            AC_SUBST([GNULIB_CHOWN])
+  GNULIB_CLOSE=0;            AC_SUBST([GNULIB_CLOSE])
   GNULIB_DUP2=0;             AC_SUBST([GNULIB_DUP2])
   GNULIB_ENVIRON=0;          AC_SUBST([GNULIB_ENVIRON])
   GNULIB_FCHDIR=0;           AC_SUBST([GNULIB_FCHDIR])
@@ -61,6 +62,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS],
   HAVE_OS_H=0;            AC_SUBST([HAVE_OS_H])
   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_FCHDIR=0;       AC_SUBST([REPLACE_FCHDIR])
   REPLACE_GETCWD=0;       AC_SUBST([REPLACE_GETCWD])
   REPLACE_GETPAGESIZE=0;  AC_SUBST([REPLACE_GETPAGESIZE])
diff --git a/modules/accept b/modules/accept
index f786882..1270a88 100644
--- a/modules/accept
+++ b/modules/accept
@@ -6,12 +6,15 @@ lib/winsock.c
 
 Depends-on:
 sys_socket
+close
 errno
 
 configure.ac:
 AC_REQUIRE([gl_HEADER_SYS_SOCKET])
+AC_REQUIRE([gl_FUNC_CLOSE])
 if test "$ac_cv_header_winsock2_h" = yes; then
   AC_LIBOBJ([winsock])
+  gl_REPLACE_CLOSE
 fi
 gl_SYS_SOCKET_MODULE_INDICATOR([accept])
 
diff --git a/modules/close b/modules/close
new file mode 100644
index 0000000..392936f
--- /dev/null
+++ b/modules/close
@@ -0,0 +1,21 @@
+Description:
+close() function: close a file or socket.
+
+Files:
+lib/close.c
+m4/close.m4
+
+Depends-on:
+
+configure.ac:
+gl_FUNC_CLOSE
+
+Makefile.am:
+
+Include:
+
+License:
+LGPLv2+
+
+Maintainer:
+Paolo Bonzini, Simon Josefsson, Bruno Haible
diff --git a/modules/fchdir b/modules/fchdir
index 3668314..e7d8eec 100644
--- a/modules/fchdir
+++ b/modules/fchdir
@@ -8,6 +8,7 @@ m4/fchdir.m4
 
 Depends-on:
 canonicalize-lgpl
+close
 dirfd
 dup2
 fcntl
diff --git a/modules/socket b/modules/socket
index 900d9ec..6cf6915 100644
--- a/modules/socket
+++ b/modules/socket
@@ -6,12 +6,15 @@ lib/winsock.c
 
 Depends-on:
 sys_socket
+close
 errno
 
 configure.ac:
 AC_REQUIRE([gl_HEADER_SYS_SOCKET])
+AC_REQUIRE([gl_FUNC_CLOSE])
 if test "$ac_cv_header_winsock2_h" = yes; then
   AC_LIBOBJ([winsock])
+  gl_REPLACE_CLOSE
 fi
 gl_SYS_SOCKET_MODULE_INDICATOR([socket])
 
diff --git a/modules/unistd b/modules/unistd
index 492f88f..95e10d9 100644
--- a/modules/unistd
+++ b/modules/unistd
@@ -25,6 +25,7 @@ unistd.h: unistd.in.h
              -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \
              -e 's|@''NEXT_UNISTD_H''@|$(NEXT_UNISTD_H)|g' \
              -e 's|@''GNULIB_CHOWN''@|$(GNULIB_CHOWN)|g' \
+             -e 's|@''GNULIB_CLOSE''@|$(GNULIB_CLOSE)|g' \
              -e 's|@''GNULIB_DUP2''@|$(GNULIB_DUP2)|g' \
              -e 's|@''GNULIB_ENVIRON''@|$(GNULIB_ENVIRON)|g' \
              -e 's|@''GNULIB_FCHDIR''@|$(GNULIB_FCHDIR)|g' \
@@ -52,6 +53,7 @@ unistd.h: unistd.in.h
              -e 's|@''HAVE_OS_H''@|$(HAVE_OS_H)|g' \
              -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_FCHDIR''@|$(REPLACE_FCHDIR)|g' \
              -e 's|@''REPLACE_GETCWD''@|$(REPLACE_GETCWD)|g' \
              -e 's|@''REPLACE_GETPAGESIZE''@|$(REPLACE_GETPAGESIZE)|g' \