Re: netdb.h

[Simon Josefsson]

Bruno Haible <address@hidden> writes:

> Hi Simon,
>
> Good to see more include files covered!

Hi!  There are some left to do... ;)

>> +#if @HAVE_NETDB_H@
>> +
>> +/* Declarations for a platform that has <netdb.h>.  */
>> +
>> +#else
>> +
>> +/* Declarations for a platform that has <netdb.h>.  */
>
> The second 'has' should be a 'lacks', no?

Yes.

>> +  AC_CACHE_CHECK([whether <netdb.h> is self-contained],
>> +    [gl_cv_header_netdb_h_selfcontained],
>> +    [
>> +      AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <netdb.h>]],
>> +                                     [[struct hostent h;]])],
>
> Is it a problem on any platform that <netdb.h> is not self-contained?

No, I just copied code from sys_socket.m4.

> IIRC, I have looked for this problem a few months ago, and found no platform
> where <netdb.h> was not self-contained. If you find some such platforms,
> then netdb.in.h must include additional header files; if not, the configure
> check can be simplified.

I think it is better to simplify the configure check.

> Also, you actually test whether the file defines 'struct hostent'. This
> is not needed: all systems that have a <netdb.h> define 'struct hostent'
> in it.

I have pushed the patch below, I appreciate review if it can be made
simpler and whether I understood the idioms properly.

The patch fixes the problem on MinGW where 'struct hostent' would not be
declared by the replacement netdb.h.  This is fixed now, by including
sys/socket.h and depending on the sys_socket module.

Thinking about this, there is some asymmetry between structures and
functions in gnulib: to get a function from a header file, you need to
pull in a module for that function.  To get a structure from a header
file, whether you need it or not, you pull in a module for the header
file.  This creates some excess dependencies, as can be seen here (netdb
wouldn't have to depend on sys_socket).  If there were a 'hostent'
module to get the 'struct hostent' declaration from the netdb.h file,
the netdb module wouldn't need to depend on sys/socket.h, and the
'hostent' module could depend on the sys_socket module to get that
definition on MinGW.  However, this appears to be established procedures
in gnulib though, but it can be useful to be aware of this.

As always, thanks for excellent review!  I wish I had access to as
careful eyes in other projects.  Useful human code review is difficult
to achieve.

/Simon

>From 3cdd75d6d78830ecc5633df781baaf2be2c4a7e9 Mon Sep 17 00:00:00 2001
From: Simon Josefsson <address@hidden>
Date: Thu, 16 Oct 2008 09:26:07 +0200
Subject: [PATCH] Fixes for netdb.h.
 * m4/netdb_h.m4: Assume that if netdb.h exists, it works.
 * lib/netdb.in.h: Fix typo.
 Reported by Bruno Haible  <address@hidden>

* lib/netdb.in.h: Include sys/socket.h for platforms without
netdb.h, to get structures like hostent on MinGW.
* modules/netdb (Depends-on): Add sys_socket.
---
 ChangeLog      |   10 ++++++++++
 lib/netdb.in.h |    5 ++++-
 m4/netdb_h.m4  |   23 ++++++-----------------
 modules/netdb  |    1 +
 4 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 721d69e..8d470e8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2008-10-16  Simon Josefsson  <address@hidden>
+
+       * m4/netdb_h.m4: Assume that if netdb.h exists, it works.
+       * lib/netdb.in.h: Fix typo.
+       Reported by Bruno Haible  <address@hidden>
+
+       * lib/netdb.in.h: Include sys/socket.h for platforms without
+       netdb.h, to get structures like hostent on MinGW.
+       * modules/netdb (Depends-on): Add sys_socket.
+
 2008-10-15  Simon Josefsson  <address@hidden>
 
        * modules/netdb, modules/netdb-tests: New file.
diff --git a/lib/netdb.in.h b/lib/netdb.in.h
index 0aa0691..9a637f2 100644
--- a/lib/netdb.in.h
+++ b/lib/netdb.in.h
@@ -40,7 +40,10 @@
 
 #else
 
-/* Declarations for a platform that has <netdb.h>.  */
+/* Get netdb.h definitions such as struct hostent for MinGW.  */
+#include <sys/socket.h>
+
+/* Declarations for a platform that lacks <netdb.h>.  */
 
 #endif /* HAVE_NETDB_H */
 
diff --git a/m4/netdb_h.m4 b/m4/netdb_h.m4
index d6d729f..8e8ff4a 100644
--- a/m4/netdb_h.m4
+++ b/m4/netdb_h.m4
@@ -1,4 +1,4 @@
-# netdb_h.m4 serial 1
+# netdb_h.m4 serial 2
 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,
@@ -7,26 +7,15 @@ dnl with or without modifications, as long as this notice is 
preserved.
 AC_DEFUN([gl_HEADER_NETDB],
 [
   AC_REQUIRE([gl_NETDB_H_DEFAULTS])
-  AC_CACHE_CHECK([whether <netdb.h> is self-contained],
-    [gl_cv_header_netdb_h_selfcontained],
-    [
-      AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <netdb.h>]],
-                                        [[struct hostent h;]])],
-        [gl_cv_header_netdb_h_selfcontained=yes],
-        [gl_cv_header_netdb_h_selfcontained=no])
-    ])
-  if test $gl_cv_header_netdb_h_selfcontained = yes; then
+  gl_CHECK_NEXT_HEADERS([netdb.h])
+  if test $ac_cv_header_netdb_h = yes; then
     NETDB_H=''
+    HAVE_NETDB_H=1
   else
     NETDB_H='netdb.h'
-    gl_CHECK_NEXT_HEADERS([netdb.h])
-    if test $ac_cv_header_netdb_h = yes; then
-      HAVE_NETDB_H=1
-    else
-      HAVE_NETDB_H=0
-    fi
-    AC_SUBST([HAVE_NETDB_H])
+    HAVE_NETDB_H=0
   fi
+  AC_SUBST([HAVE_NETDB_H])
   AC_SUBST([NETDB_H])
 ])
 
diff --git a/modules/netdb b/modules/netdb
index 21e4ff4..3c2e9b4 100644
--- a/modules/netdb
+++ b/modules/netdb
@@ -7,6 +7,7 @@ m4/netdb_h.m4
 
 Depends-on:
 include_next
+sys_socket
 
 configure.ac:
 gl_HEADER_NETDB
-- 
1.5.6.5