Re: [PATCH] socklen: do not depend on sys_socket

[Bruno Haible]

Hi Paul,

> Here's the revised proposed patch.

OK, this is now easier to review, now that the unrelated hunks have been
committed separately.

> >      #include <sys/types.h>
> >      #if HAVE_SYS_SOCKET_H
> >      #include <sys/socket.h>
> >      #endif
> 
> Given the changes you proposed in the other part of your comment,
> I expect it should be changed to the following instead.  That reflects
> what the revised test does.
> 
>    #include <sys/types.h>
>    #if HAVE_SYS_SOCKET_H
>    # include <sys/socket.h>
>    #elif HAVE_WS2TCPIP_H
>    # include <ws2tcpip.h>
>    #endif

Good point, yes!

> > Also a test that defines HAVE_SYS_SOCKET_H needs to be added, then.
> 
> That shouldn't be needed, since the test is already present in the
> proposed patch: gl_TYPE_SOCKLEN_T requires gl_PREREQ_TYPE_SOCKLEN_T,
> which invokes AC_CHECK_HEADERS_ONCE([sys/socket.h]), which arranges to
> #define HAVE_SYS_SOCKET_H.

Yes. All right.

>  AC_DEFUN([gl_PREREQ_SYS_H_SOCKET],
>  [
>    dnl Check prerequisites of the <sys/socket.h> replacement.
> -  gl_CHECK_NEXT_HEADERS([sys/socket.h])
> +  AC_REQUIRE([gl_PREREQ_TYPE_SOCKLEN_T])
>    if test $ac_cv_header_sys_socket_h = yes; then

This part has the effect of no longer defining @NEXT_SYS_SOCKET_H@, leading
to compilation errors on glibc systems already:

  $ ./gnulib-tool --test --with-tests sys_socket
  ...
  gcc -DHAVE_CONFIG_H -I. -I../../gltests  -DGNULIB_STRICT_CHECKING=1 -I. 
-I../../gltests -I.. -I../../gltests/.. -I../gllib -I../../gltests/../gllib   
-g -O2 -MT test-sys_socket.o -MD -MP -MF .deps/test-sys_socket.Tpo -c -o 
test-sys_socket.o ../../gltests/test-sys_socket.c
  In file included from ../../gltests/test-sys_socket.c:21:
  ../gllib/sys/socket.h:59:16: error: #include_next expects "FILENAME" or 
<FILENAME>

> +AC_DEFUN([gl_PREREQ_TYPE_SOCKLEN_T],
> +  [AC_CHECK_HEADERS_ONCE([sys/socket.h])
> +   if test $ac_cv_header_sys_socket_h = no; then
> +     dnl We cannot use AC_CHECK_HEADERS_ONCE here, because that would make
> +     dnl the check for those headers unconditional; yet cygwin reports
> +     dnl that the headers are present but cannot be compiled (since on
> +     dnl cygwin, all socket information should come from sys/socket.h).
> +     AC_CHECK_HEADERS([ws2tcpip.h])
> +   fi])

It's good to separate this is a separate macro. But since it's invoked by
sys_socket_h.m4, I think it's name should reflect that it's not only about
socklen_t.

I'm applying your patch, with the following changes merged in:

--- m4/sys_socket_h.m4.orig     Sun Mar 20 13:30:12 2011
+++ m4/sys_socket_h.m4  Sun Mar 20 13:16:15 2011
@@ -101,7 +101,8 @@
 AC_DEFUN([gl_PREREQ_SYS_H_SOCKET],
 [
   dnl Check prerequisites of the <sys/socket.h> replacement.
-  AC_REQUIRE([gl_PREREQ_TYPE_SOCKLEN_T])
+  AC_REQUIRE([gl_CHECK_SOCKET_HEADERS])
+  gl_CHECK_NEXT_HEADERS([sys/socket.h])
   if test $ac_cv_header_sys_socket_h = yes; then
     HAVE_SYS_SOCKET_H=1
     HAVE_WS2TCPIP_H=0
--- m4/socklen.m4.orig  Sun Mar 20 13:30:12 2011
+++ m4/socklen.m4       Sun Mar 20 13:24:48 2011
@@ -12,11 +12,8 @@
 dnl HP-UX 10.20, IRIX 6.5, OSF/1 4.0, Interix 3.5, BeOS.
 dnl So we have to test to find something that will work.
 
-dnl On mingw32, socklen_t is in ws2tcpip.h ('int'), so we try to find
-dnl it there too.  Users of this module should use the same include
-dnl pattern that this test does.
 AC_DEFUN([gl_TYPE_SOCKLEN_T],
-  [AC_REQUIRE([gl_PREREQ_TYPE_SOCKLEN_T])dnl
+  [AC_REQUIRE([gl_CHECK_SOCKET_HEADERS])dnl
    AC_CHECK_TYPE([socklen_t], ,
      [AC_MSG_CHECKING([for socklen_t equivalent])
       AC_CACHE_VAL([gl_cv_socklen_t_equiv],
@@ -44,14 +41,31 @@
       AC_MSG_RESULT([$gl_cv_socklen_t_equiv])
       AC_DEFINE_UNQUOTED([socklen_t], [$gl_cv_socklen_t_equiv],
         [type to use in place of socklen_t if not defined])],
-     [#include <sys/types.h>
-      #if HAVE_SYS_SOCKET_H
-      # include <sys/socket.h>
-      #elif HAVE_WS2TCPIP_H
-      # include <ws2tcpip.h>
-      #endif])])
+     [gl_SOCKET_HEADERS])])
+
+dnl On mingw32, socklen_t is in ws2tcpip.h ('int'), so we try to find
+dnl it there too.  But on Cygwin, wc2tcpip.h must not be included.  Users
+dnl of this module should use the same include pattern as gl_SOCKET_HEADERS.
+dnl When you change this macro, keep also in sync:
+dnl   - gl_CHECK_SOCKET_HEADERS,
+dnl   - the Include section of modules/socklen.
+AC_DEFUN([gl_SOCKET_HEADERS],
+[
+/* <sys/types.h> is not needed according to POSIX, but the
+   <sys/socket.h> in i386-unknown-freebsd4.10 and
+   powerpc-apple-darwin5.5 required it. */
+#include <sys/types.h>
+#if HAVE_SYS_SOCKET_H
+# include <sys/socket.h>
+#elif HAVE_WS2TCPIP_H
+# include <ws2tcpip.h>
+#endif
+])
 
-AC_DEFUN([gl_PREREQ_TYPE_SOCKLEN_T],
+dnl Tests for the existence of the header for socket facilities.
+dnl Defines the C macros HAVE_SYS_SOCKET_H, HAVE_WS2TCPIP_H.
+dnl This macro must match gl_SOCKET_HEADERS.
+AC_DEFUN([gl_CHECK_SOCKET_HEADERS],
   [AC_CHECK_HEADERS_ONCE([sys/socket.h])
    if test $ac_cv_header_sys_socket_h = no; then
      dnl We cannot use AC_CHECK_HEADERS_ONCE here, because that would make
@@ -59,4 +73,5 @@
      dnl that the headers are present but cannot be compiled (since on
      dnl cygwin, all socket information should come from sys/socket.h).
      AC_CHECK_HEADERS([ws2tcpip.h])
-   fi])
+   fi
+  ])

-- 
In memoriam Jan Nepomucký <http://en.wikipedia.org/wiki/John_of_Nepomuk>