[PATCH] #2 was Re: [PATCH] Add reentrant gethostbyname call

[Justin Erenkrantz <je...@ebuilt.com>]

Improvements and notes since my original posting:

- Use autoconf detection to try to eliminate chances where we may
  not need gethostbyname_r

- Fixed one long line that bugged me.

- Bump up buffersize to 512 since it may be larger than I thought
  (need to give Roy more Dr. Pepper).  I was using it with 256 
  before and didn't see any problem, but 512 is a bit more 
  conservative.  Hopefully if the user runs out of range, whatever 
  calls this function will notice the error (ERANGE) and report 
  it to us and we can bump the #define if need be.

- AFAICT, gethostbyname_r is *not* defined in POSIX 1003.1c-1995, so 
  we can't use _POSIX_THREAD_SAFE_FUNCTIONS as we can for readdir_r.

- Solaris 8, which has getipnodebyname and getaddrinfo, also has
  IPV6, so it takes a completely different code path.  We were 
  using Solaris 7 today, so that may be why I haven't seen problems 
  before now.

- Roy mentioned that the gethostbyname_r in Solaris has different
  semantics than POSIX.  Well, I can't/don't have access to any 
  platforms that uses the different prototype.  So, if there is 
  another platform out there that has gethostbyname_r with a
  different prototype, you'll have to add in the tweaks or submit 
  patches.  Sorry.

I'll plan on committing in the morning unless I receive any
objections.  I wonder how many more of these non-threadsafe 
functions we're using with threads.  =(  -- justin

Index: configure.in
===================================================================
RCS file: /home/cvs/apr/configure.in,v
retrieving revision 1.339
diff -u -r1.339 configure.in
--- configure.in	2001/07/14 00:13:36	1.339
+++ configure.in	2001/07/20 05:56:47
@@ -326,9 +326,12 @@
 fi
 
 ac_cv_define_READDIR_IS_THREAD_SAFE=no
+ac_cv_define_GETHOSTBYNAME_IS_THREAD_SAFE=no
 if test "$threads" = "1"; then
     echo "APR will use threads"
     AC_CHECK_LIB(c_r, readdir, AC_DEFINE(READDIR_IS_THREAD_SAFE))
+    AC_CHECK_LIB(c_r, gethostbyname, AC_DEFINE(GETHOSTBYNAME_IS_THREAD_SAFE))
+    AC_CHECK_FUNCS(gethostbyname_r)
 else
     echo "APR will be non-threaded"
 fi
Index: acconfig.h
===================================================================
RCS file: /home/cvs/apr/acconfig.h,v
retrieving revision 1.46
diff -u -r1.46 acconfig.h
--- acconfig.h	2001/07/02 18:54:05	1.46
+++ acconfig.h	2001/07/20 05:56:47
@@ -23,6 +23,7 @@
 #undef USE_PTHREAD_SERIALIZE
 
 #undef READDIR_IS_THREAD_SAFE
+#undef GETHOSTBYNAME_IS_THREAD_SAFE
 
 #undef NEED_RLIM_T
 #undef USEBCOPY
Index: network_io/unix/sa_common.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sa_common.c,v
retrieving revision 1.34
diff -u -r1.34 sa_common.c
--- network_io/unix/sa_common.c	2001/05/02 02:54:11	1.34
+++ network_io/unix/sa_common.c	2001/07/20 05:56:47
@@ -77,6 +77,15 @@
 #define SET_H_ERRNO(newval) h_errno = (newval)
 #endif
 
+#if APR_HAS_THREADS && !defined(GETHOSTBYNAME_IS_THREAD_SAFE) && \
+    defined(HAVE_GETHOSTBYNAME_R)
+/* This is the maximum size that may be returned from the reentrant
+ * gethostbyname_r function.  If the system tries to use more, it
+ * should return ERANGE.
+ */
+#define GETHOSTBYNAME_BUFLEN 512
+#endif
+
 APR_DECLARE(apr_status_t) apr_sockaddr_port_set(apr_sockaddr_t *sockaddr,
                                        apr_port_t port)
 {
@@ -380,6 +389,12 @@
         struct hostent *hp;
         apr_sockaddr_t *cursa;
         int curaddr;
+#if APR_HAS_THREADS && !defined(GETHOSTBYNAME_IS_THREAD_SAFE) && \
+    defined(HAVE_GETHOSTBYNAME_R)
+        char tmp[GETHOSTBYNAME_BUFLEN];
+        int hosterror;
+        struct hostent hs;
+#endif
 
         if (family == APR_UNSPEC) {
             family = APR_INET; /* we don't support IPv6 here */
@@ -395,11 +410,22 @@
         }
         else {
 #endif
+#if APR_HAS_THREADS && !defined(GETHOSTBYNAME_IS_THREAD_SAFE) && \
+    defined(HAVE_GETHOSTBYNAME_R)
+        hp = gethostbyname_r(hostname, &hs, tmp, GETHOSTBYNAME_BUFLEN - 1, 
+                             &hosterror);
+#else
         hp = gethostbyname(hostname);
+#endif
 
         if (!hp)  {
 #ifdef WIN32
             apr_get_netos_error();
+#elif APR_HAS_THREADS && !defined(GETHOSTBYNAME_IS_THREAD_SAFE) && \
+    defined(HAVE_GETHOSTBYNAME_R)
+            /* If you see ERANGE, that means GETHOSBYNAME_BUFLEN needs to be
+             * bumped. */
+            return (hosterror + APR_OS_START_SYSERR);
 #else
             return (h_errno + APR_OS_START_SYSERR);
 #endif
@@ -412,7 +438,7 @@
         while (hp->h_addr_list[curaddr]) {
             cursa->next = apr_pcalloc(p, sizeof(apr_sockaddr_t));
             cursa = cursa->next;
-            save_addrinfo(p, cursa, *(struct in_addr *)hp->h_addr_list[curaddr], 
+            save_addrinfo(p, cursa, *(struct in_addr *)hp->h_addr_list[curaddr],
                           port);
             ++curaddr;
         }