bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: close vs socket vs getline


From: Bruno Haible
Subject: Re: close vs socket vs getline
Date: Fri, 20 Mar 2009 16:42:43 +0100
User-agent: KMail/1.9.9

Simon Josefsson wrote:
> This may have been discussed before, but I don't recall a solution, so
> maybe a new report will help.

Yes, in the meantime we all had time to think about it.

> i586-mingw32msvc-gcc  -g -O2   -o test-getdelim.exe test-getdelim.o 
> ../gllib/libgnu.a 
> ../gllib/libgnu.a(close.o): In function `_gl_close_fd_maybe_socket':
> /home/jas/src/gnulib/m/gllib/close.c:39: undefined reference to 
> address@hidden'
> /home/jas/src/gnulib/m/gllib/close.c:46: undefined reference to 
> address@hidden'
> ../gllib/libgnu.a(close.o): In function `set_winsock_errno':
> /home/jas/src/gnulib/m/gllib/w32sock.h:34: undefined reference to 
> address@hidden'
> /home/jas/src/gnulib/m/gllib/w32sock.h:35: undefined reference to 
> address@hidden'

The logic behind it is correct: some functions in the package open sockets,
and test-getdelim calls fclose(), which calls close(), which - given the unknown
origin of the file descriptor - needs to call WSAEnumNetworkEvents(), which
requires -lws2_32.

> Some ideas:
> 
> 1) Should the sockets.m4 module unconditionally add LIBSOCKET to LDADD?
> 
> 2) Should the gnulib-tool generated Makefile.am add LIBSOCKET to LDADD?

No, LDADD has an effect on all executables. I don't want to link all my
programs against -lws2_32 just because one of them uses sockets.

No, I don't want to add LIBSOCKET to the link dependencies of the 'close'
and 'fclose' either. It would also have the effect that all or nearly my
programs would link against -lws2_32.

Dynamic loading of ws2_32,dll is not the solution either; it would still
have the effect of loading an extra library in programs that don't need it.

> Is there any other options?

I propose to move the sockets related part of close() into the 'sockets'
module, and connect the close() code with the sockets related code at runtime
rather than at compile time. Like this (tested on mingw).

This uses the "hook" design pattern. It would be possible to optimize it,
knowing that at most one hook will be installed, but there's not much
point in doing so.


2009-03-20  Bruno Haible  <address@hidden>

        Remove dependency from 'close' module to -lws2_32 on native Windows.
        * lib/close-hook.h: New file.
        * lib/close-hook.c: New file.
        * lib/close.c: Include close-hook.h. Don't include <sys/socket.h>,
        w32sock.h.
        (_gl_close_fd_maybe_socket): Remove function.
        (rpl_close): Invoke execute_all_close_hooks instead of
        _gl_close_fd_maybe_socket.
        * lib/sockets.c: Include close-hook.h, w32sock.h.
        (close_fd_maybe_socket): New function, essentially from lib/close.c.
        (close_sockets_hook): New variable.
        (gl_sockets_startup): Register close_fd_maybe_socket as a hook.
        (gl_sockets_cleanup): Unregister it.
        * lib/unistd.in.h (HAVE__GL_CLOSE_FD_MAYBE_SOCKET): Remove macro.
        * m4/close.m4 (gl_REPLACE_CLOSE): Undo 2009-02-05 change.
        * modules/close-hook: New file.
        * modules/close (Files): Remove lib/w32sock.h.
        (Depends-on): Add close-hook.
        (Link): Remove section.
        * modules/sockets (Files): Add lib/w32sock.h.
        (Depends-on): Add close-hook.
        * modules/sys_socket (configure.ac): Remove gl_MODULE_INDICATOR
        invocation.
        * NEWS: Mention that LIB_CLOSE is gone.

============================== lib/close-hook.h ==============================
/* Hook for making the close() function extensible.
   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 Lesser 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
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */


#ifndef CLOSE_HOOK_H
#define CLOSE_HOOK_H

#ifdef __cplusplus
extern "C" {
#endif


/* Currently, this entire code is only needed for the handling of sockets
   on native Windows platforms.  */
#if WINDOWS_SOCKETS


/* An element of the list of close hooks.
   The fields of this structure are considered private.  */
struct close_hook
{
  /* Doubly linked list.  */
  struct close_hook *private_next;
  struct close_hook *private_prev;
  /* Function that treats the types of FD that it knows about and calls
     execute_close_hooks (FD, REMAINING_LIST) as a fallback.  */
  int (*private_fn) (int fd, const struct close_hook *remaining_list);
};

/* This type of function closes FD, applying special knowledge for the FD
   types it knows about, and calls execute_close_hooks (FD, REMAINING_LIST)
   for the other FD types.  */
typedef int (*close_hook_fn) (int fd, const struct close_hook *remaining_list);

/* Execute the close hooks in REMAINING_LIST.
   Return 0 or -1, like close() would do.  */
extern int execute_close_hooks (int fd, const struct close_hook 
*remaining_list);

/* Execute all close hooks.
   Return 0 or -1, like close() would do.  */
extern int execute_all_close_hooks (int fd);

/* Add a function to the list of close hooks.
   The LINK variable points to a piece of memory which is guaranteed to be
   accessible until the corresponding call to unregister_close_hook.  */
extern void register_close_hook (close_hook_fn hook, struct close_hook *link);

/* Removes a function from the list of close hooks.  */
extern void unregister_close_hook (struct close_hook *link);


#endif


#ifdef __cplusplus
}
#endif

#endif /* CLOSE_HOOK_H */
============================== lib/close-hook.c ==============================
/* Hook for making the close() function extensible.
   Copyright (C) 2009 Free Software Foundation, Inc.
   Written by Bruno Haible <address@hidden>, 2009.

   This program is free software: you can redistribute it and/or modify it
   under the terms of the GNU Lesser 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
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

#include <config.h>

/* Specification.  */
#include "close-hook.h"

#include <stdlib.h>
#include <unistd.h>

#undef close


/* Currently, this entire code is only needed for the handling of sockets
   on native Windows platforms.  */
#if WINDOWS_SOCKETS

/* The first and last link in the doubly linked list.
   Initially the list is empty.  */
static struct close_hook anchor = { &anchor, &anchor, NULL };

int
execute_close_hooks (int fd, const struct close_hook *remaining_list)
{
  if (remaining_list == &anchor)
    /* End of list reached.  */
    return close (fd);
  else
    return remaining_list->private_fn (fd, remaining_list->private_next);
}

int
execute_all_close_hooks (int fd)
{
  return execute_close_hooks (fd, anchor.private_next);
}

void
register_close_hook (close_hook_fn hook, struct close_hook *link)
{
  if (link->private_next == NULL && link->private_prev == NULL)
    {
      /* Add the link to the doubly linked list.  */
      link->private_next = anchor.private_next;
      link->private_prev = &anchor;
      link->private_fn = hook;
      anchor.private_next->private_prev = link;
      anchor.private_next = link;
    }
  else
    {
      /* The link is already in use.  */
      if (link->private_fn != hook)
        abort ();
    }
}

void
unregister_close_hook (struct close_hook *link)
{
  struct close_hook *next = link->private_next;
  struct close_hook *prev = link->private_prev;

  if (next != NULL && prev != NULL)
    {
      /* The link is in use.  Remove it from the doubly linked list.  */
      prev->private_next = next;
      next->private_prev = prev;
      /* Clear the link, to mark it unused.  */
      link->private_next = NULL;
      link->private_prev = NULL;
      link->private_fn = NULL;
    }
}

#endif
============================= modules/close-hook =============================
Description:
Hook for making close() extensible.

Files:
lib/close-hook.h
lib/close-hook.c

Depends-on:
unistd

configure.ac:

Makefile.am:
lib_SOURCES += close-hook.c

Include:
"close-hook.h"

License:
LGPLv2+

Maintainer:
Bruno Haible
==============================================================================
--- lib/close.c.orig    2009-03-20 15:32:21.000000000 +0100
+++ lib/close.c 2009-03-20 15:29:09.000000000 +0100
@@ -1,5 +1,5 @@
 /* close replacement.
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008-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
@@ -19,47 +19,7 @@
 /* Specification.  */
 #include <unistd.h>
 
-#if GNULIB_SYS_SOCKET
-# define WIN32_LEAN_AND_MEAN
-# include <sys/socket.h>
-#endif
-
-#if HAVE__GL_CLOSE_FD_MAYBE_SOCKET
-
-/* Get set_winsock_errno, FD_TO_SOCKET etc. */
-#include "w32sock.h"
-
-static int
-_gl_close_fd_maybe_socket (int fd)
-{
-  SOCKET sock = FD_TO_SOCKET (fd);
-  WSANETWORKEVENTS ev;
-
-  ev.lNetworkEvents = 0xDEADBEEF;
-  WSAEnumNetworkEvents (sock, NULL, &ev);
-  if (ev.lNetworkEvents != 0xDEADBEEF)
-    {
-      /* FIXME: other applications, like squid, use an undocumented
-        _free_osfhnd free function.  But this is not enough: The 'osfile'
-        flags for fd also needs to be cleared, but it is hard to access it.
-        Instead, here we just close twice the file descriptor.  */
-      if (closesocket (sock))
-       {
-         set_winsock_errno ();
-         return -1;
-       }
-      else
-       {
-         /* This call frees the file descriptor and does a
-            CloseHandle ((HANDLE) _get_osfhandle (fd)), which fails.  */
-         _close (fd);
-         return 0;
-       }
-    }
-  else
-    return _close (fd);
-}
-#endif
+#include "close-hook.h"
 
 /* Override close() to call into other gnulib modules.  */
 
@@ -67,8 +27,8 @@
 rpl_close (int fd)
 #undef close
 {
-#if HAVE__GL_CLOSE_FD_MAYBE_SOCKET
-  int retval = _gl_close_fd_maybe_socket (fd);
+#if WINDOWS_SOCKETS
+  int retval = execute_all_close_hooks (fd);
 #else
   int retval = close (fd);
 #endif
--- lib/sockets.c.orig  2009-03-20 15:32:21.000000000 +0100
+++ lib/sockets.c       2009-03-20 15:29:50.000000000 +0100
@@ -22,9 +22,55 @@
 /* Specification.  */
 #include "sockets.h"
 
+#if WINDOWS_SOCKETS
+
 /* This includes winsock2.h on MinGW. */
 #include <sys/socket.h>
 
+#include "close-hook.h"
+
+/* Get set_winsock_errno, FD_TO_SOCKET etc. */
+#include "w32sock.h"
+
+static int
+close_fd_maybe_socket (int fd, const struct close_hook *remaining_list)
+{
+  SOCKET sock;
+  WSANETWORKEVENTS ev;
+
+  /* Test whether fd refers to a socket.  */
+  sock = FD_TO_SOCKET (fd);
+  ev.lNetworkEvents = 0xDEADBEEF;
+  WSAEnumNetworkEvents (sock, NULL, &ev);
+  if (ev.lNetworkEvents != 0xDEADBEEF)
+    {
+      /* fd refers to a socket.  */
+      /* FIXME: other applications, like squid, use an undocumented
+        _free_osfhnd free function.  But this is not enough: The 'osfile'
+        flags for fd also needs to be cleared, but it is hard to access it.
+        Instead, here we just close twice the file descriptor.  */
+      if (closesocket (sock))
+       {
+         set_winsock_errno ();
+         return -1;
+       }
+      else
+       {
+         /* This call frees the file descriptor and does a
+            CloseHandle ((HANDLE) _get_osfhandle (fd)), which fails.  */
+         _close (fd);
+         return 0;
+       }
+    }
+  else
+    /* Some other type of file descriptor.  */
+    return execute_close_hooks (fd, remaining_list);
+}
+
+static struct close_hook close_sockets_hook;
+
+#endif
+
 int
 gl_sockets_startup (int version)
 {
@@ -38,6 +84,8 @@
 
   if (data.wVersion < version)
     return 2;
+
+  register_close_hook (close_fd_maybe_socket, &close_sockets_hook);
 #endif
 
   return 0;
@@ -49,6 +97,8 @@
 #if WINDOWS_SOCKETS
   int err;
 
+  unregister_close_hook (&close_sockets_hook);
+
   err = WSACleanup ();
   if (err != 0)
     return 1;
--- lib/unistd.in.h.orig        2009-03-20 15:32:21.000000000 +0100
+++ lib/unistd.in.h     2009-03-20 15:29:09.000000000 +0100
@@ -131,10 +131,6 @@
 
 
 #if @GNULIB_CLOSE@
-# if @UNISTD_H_HAVE_WINSOCK2_H@
-/* Need a gnulib internal function.  */
-#  define HAVE__GL_CLOSE_FD_MAYBE_SOCKET 1
-# endif
 # if @REPLACE_CLOSE@
 /* Automatically included by modules that need a replacement for close.  */
 #  undef close
--- m4/close.m4.orig    2009-03-20 15:32:21.000000000 +0100
+++ m4/close.m4 2009-03-20 15:29:09.000000000 +0100
@@ -1,5 +1,5 @@
-# close.m4 serial 2
-dnl Copyright (C) 2008 Free Software Foundation, Inc.
+# close.m4 serial 3
+dnl Copyright (C) 2008-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.
@@ -22,6 +22,4 @@
   fi
   REPLACE_CLOSE=1
   gl_REPLACE_FCLOSE
-  LIB_CLOSE="-lws2_32"
-  AC_SUBST([LIB_CLOSE])
 ])
--- modules/close.orig  2009-03-20 15:32:21.000000000 +0100
+++ modules/close       2009-03-20 15:29:09.000000000 +0100
@@ -3,11 +3,11 @@
 
 Files:
 lib/close.c
-lib/w32sock.h
 m4/close.m4
 
 Depends-on:
 unistd
+close-hook
 fclose
 
 configure.ac:
@@ -19,9 +19,6 @@
 Include:
 <unistd.h>
 
-Link:
-$(LIB_CLOSE)
-
 License:
 LGPLv2+
 
--- modules/sockets.orig        2009-03-20 15:32:21.000000000 +0100
+++ modules/sockets     2009-03-20 15:29:09.000000000 +0100
@@ -4,10 +4,12 @@
 Files:
 lib/sockets.c
 lib/sockets.h
+lib/w32sock.h
 m4/sockets.m4
 
 Depends-on:
 sys_socket
+close-hook
 
 configure.ac:
 gl_SOCKETS
--- modules/sys_socket.orig     2009-03-20 15:32:21.000000000 +0100
+++ modules/sys_socket  2009-03-20 15:29:09.000000000 +0100
@@ -13,7 +13,6 @@
 
 configure.ac:
 gl_HEADER_SYS_SOCKET
-gl_MODULE_INDICATOR([sys_socket])
 AC_PROG_MKDIR_P
 
 Makefile.am:
--- NEWS.orig   2009-03-20 15:32:21.000000000 +0100
+++ NEWS        2009-03-20 15:32:18.000000000 +0100
@@ -6,6 +6,8 @@
 
 Date        Modules         Changes
 
+2009-03-20  close           The substituted variable LIB_CLOSE is removed.
+
 2009-03-05  filevercmp      Move hidden files up in ordering.
 
 2009-01-22  c-strtod        This function no longer calls xalloc_die(). If




reply via email to

[Prev in Thread] Current Thread [Next in Thread]