[Patch] gethostbyname2 again

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch] gethostbyname2 again

Pierre A. Humblet-2
Corinna,

OK, here we go again.

This version calls res_query and all the work is done in net.cc.
As discussed before that means a lot of work is wasted when using the
Windows resolver.
    That will be improved later.
There is no progress on the issue of resolving local names (NetBIOS
over TCP) for now,
     so it's not a perfect replacement for the native gethostbyname yet.

Misc nits and notes:
- Including resolv.h in net.cc causes havoc because it pulls in
cygwin/in.h which conflicts
   with winsock2.h.   I worked around that by
defining   _CYGWIN_IN_H  before including resolv.h
- Because of the above, asm/byteorder.h doesn't get pulled in either,
and I couldn't use
   some ntoh macros (see memcpy4to6).
- I could have included asm/byteorder separately but that causes
conflicts with the local ntoh
   definitions in net.cc.
- Because arpa/nameser.h is now pulled in,  IN6ADDRSZ etc are now
defined, but in a way different
   from done in the snippet of code cut & pasted from bind. I didn't
want to change that piece (in
   case you want to keep in intact for some reason) and I ended up
undefining  IN6ADDRSZ etc .
-  There is a new helper function dn_length1 which logically belongs
in minires.c, although it shouldn't
    be exported by the dll.  However if I place it in minires.c, then
the linker doesn't find it.
    Fixing that probably involves some Makefile magic.
- I structured the code with a helper function gethostby_helper. That
will make it very easy to support
    a gethostbyaddress some day, if needed.
- The helper function avoids using dup_ent (there is enough copying
already). I created a new
    realloc_ent function, and call it from both dup_ent and the
helper. That caused minor
    changes in the 4 versions of dup_ent, and I don't know exactly
what format to use in the ChangeLog
- This  is much more complex than the first way of doing things. Needs testing!
- The patch is long, see the attachment. There is also a test program attached.

Pierre

2009-03-02  Pierre Humblet <[hidden email]>

        * net.cc: define _CYGWIN_IN_H and include resolv.h.
        (realloc_ent): New function.
        (dup_ent): Remove dst argument and call realloc_ent.
        (memcpy4to6): New function.
        (dn_length1): New function.
        (gethostby_helper): New function.
        (gethostbyname2): New function.
        * cygwin.din: Export gethostbyname2.
        * libc/minires.c (get_options): Look for "inet6" and apply bounds
        to "retry" and "retrans".
        (res_ninit): Set the default options at the beginning.
        (dn_expand): Fix "off by one".



 

gethostbyname2_b.diff (22K) Download Attachment
try_gethostbyname.c (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Corinna Vinschen-2
[Chris, can you have a look into Pierre's dup_ent changes?  Thanks.]


Hi Pierre,

On Mar  2 20:36, Pierre A. Humblet wrote:
> Corinna,
>
> OK, here we go again.
>
> This version calls res_query and all the work is done in net.cc.

First of all, thanks for doing that.

> - Because of the above, asm/byteorder.h doesn't get pulled in either,  
> and I couldn't use
>   some ntoh macros (see memcpy4to6).
> - I could have included asm/byteorder separately but that causes  
> conflicts with the local ntoh
>   definitions in net.cc.

I fixed that by moving the definitions of ntohl and friends to the end
of the file and by defining them according to the definitions in
asm/byteorder.h.  That's what POSIX demands anyway.

> - Because arpa/nameser.h is now pulled in,  IN6ADDRSZ etc are now  
> defined, but in a way different
>   from done in the snippet of code cut & pasted from bind. I didn't want
> to change that piece (in
>   case you want to keep in intact for some reason) and I ended up  
> undefining  IN6ADDRSZ etc .

I removed thatto use the definitions from nameser_compat.h.  They are
equivalent anyway.

> -  There is a new helper function dn_length1 which logically belongs in
> minires.c, although it shouldn't
>    be exported by the dll.  However if I place it in minires.c, then the
> linker doesn't find it.

You missed to define it as a `extern "C" function.

>    Fixing that probably involves some Makefile magic.

No, it's sufficent to define it as extern "C".  I changed your patch
so that dn_length1 is now declared in the `extern "C" declaration
block at the start of net.cc and I moved the function into minires.c.

> - I structured the code with a helper function gethostby_helper. That  
> will make it very easy to support
>    a gethostbyaddress some day, if needed.
> - The helper function avoids using dup_ent (there is enough copying  
> already). I created a new
>    realloc_ent function, and call it from both dup_ent and the helper.
> That caused minor
>    changes in the 4 versions of dup_ent, and I don't know exactly what
> format to use in the ChangeLog

The patch looks good to me.  I only changed the definition of realloc_ent
to be a static function.

> - This  is much more complex than the first way of doing things. Needs testing!
> - The patch is long, see the attachment. There is also a test program attached.

All my usual network applications are still working fine.

I attached the entire patch again with my changes.  I had to change the
gethostby_helper function to define some of the variables at the start
of the function, othewise gcc complained about jumps to a label crossing
variable initializations.  The bump of the API minor number in
include/cygwin/version.h was missing.  I also tweaked the formatting a bit.

The ChangeLog entry is the same as in the OP, except for the additional
reference to include/cygwin/version.h.  Please have a look.

Chris, the dup_ent code is yours.  Can you have a look if the realloc_ent
changes are ok with you?


Thanks,
Corinna


  * net.cc: define _CYGWIN_IN_H and include resolv.h.
  (realloc_ent): New function.
  (dup_ent): Remove dst argument and call realloc_ent.
  (memcpy4to6): New function.
  (dn_length1): New function.
  (gethostby_helper): New function.
  (gethostbyname2): New function.
  * cygwin.din: Export gethostbyname2.
  * libc/minires.c (get_options): Look for "inet6" and apply bounds
  to "retry" and "retrans".
  (res_ninit): Set the default options at the beginning.
  (dn_expand): Fix "off by one".
        * include/cygwin/version.h: Bump API minor number.


Index: cygwin.din
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygwin.din,v
retrieving revision 1.202
diff -u -p -r1.202 cygwin.din
--- cygwin.din 19 Feb 2009 09:22:51 -0000 1.202
+++ cygwin.din 3 Mar 2009 11:54:11 -0000
@@ -635,6 +635,7 @@ _getgroups = getgroups SIGFE
 _getgroups32 = getgroups32 SIGFE
 gethostbyaddr = cygwin_gethostbyaddr SIGFE
 gethostbyname = cygwin_gethostbyname SIGFE
+gethostbyname2 SIGFE
 gethostid SIGFE
 gethostname = cygwin_gethostname SIGFE
 _gethostname = cygwin_gethostname SIGFE
Index: net.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/net.cc,v
retrieving revision 1.249
diff -u -p -r1.249 net.cc
--- net.cc 3 Mar 2009 11:44:17 -0000 1.249
+++ net.cc 3 Mar 2009 11:54:12 -0000
@@ -1,7 +1,7 @@
 /* net.cc: network-related routines.
 
    Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-   2005, 2006, 2007 Red Hat, Inc.
+   2005, 2006, 2007, 2008, 2009 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -43,6 +43,8 @@ details. */
 #include "cygwin/in6.h"
 #include "ifaddrs.h"
 #include "tls_pbuf.h"
+#define _CYGWIN_IN_H
+#include <resolv.h>
 
 extern "C"
 {
@@ -53,6 +55,9 @@ extern "C"
   int sscanf (const char *, const char *, ...);
   int cygwin_inet_aton(const char *, struct in_addr *);
   const char *cygwin_inet_ntop (int, const void *, char *, socklen_t);
+  int dn_length1(const unsigned char *, const unsigned char *,
+ const unsigned char *);
+  
 } /* End of "C" section */
 
 const struct in6_addr in6addr_any = {{IN6ADDR_ANY_INIT}};
@@ -264,6 +269,34 @@ struct pservent
 
 static const char *entnames[] = {"host", "proto", "serv"};
 
+static unionent *
+realloc_ent (int sz, unionent::struct_type type)
+{
+  /* Allocate the storage needed.  Allocate a rounded size to attempt to force
+     reuse of this buffer so that a poorly-written caller will not be using
+     a freed buffer. */
+  unsigned rsz = 256 * ((sz + 255) / 256);
+  unionent * dst, * ptr;
+  switch (type)
+    {
+    case unionent::t_hostent:
+      dst = _my_tls.locals.hostent_buf;
+      break;
+    case unionent::t_protoent:
+      dst =_my_tls.locals.protoent_buf;
+      break;
+    case unionent::t_servent:
+      dst =_my_tls.locals.servent_buf;
+      break;
+    default:
+      return NULL;
+    }
+  ptr = (unionent *) realloc (dst, rsz);
+  if (ptr)
+    dst = ptr;
+  return ptr;
+}
+
 /* Generic "dup a {host,proto,serv}ent structure" function.
    This is complicated because we need to be able to free the
    structure at any point and we can't rely on the pointer contents
@@ -278,12 +311,8 @@ static void *
 #else
 static inline void *
 #endif
-dup_ent (unionent *&dst, unionent *src, unionent::struct_type type)
+dup_ent (unionent *src, unionent::struct_type type)
 {
-  if (dst)
-    debug_printf ("old %sent structure \"%s\" %p\n", entnames[type],
-  dst->name, dst);
-
   if (!src)
     {
       set_winsock_errno ();
@@ -310,7 +339,7 @@ dup_ent (unionent *&dst, unionent *src,
       break;
     }
 
-  /* Every *ent begins with a name.  Calculate it's length. */
+  /* Every *ent begins with a name.  Calculate its length. */
   int namelen = strlen_round (src->name);
   sz = struct_sz + namelen;
 
@@ -355,11 +384,7 @@ dup_ent (unionent *&dst, unionent *src,
  }
     }
 
-  /* Allocate the storage needed.  Allocate a rounded size to attempt to force
-     reuse of this buffer so that a poorly-written caller will not be using
-     a freed buffer. */
-  unsigned rsz = 256 * ((sz + 255) / 256);
-  dst = (unionent *) realloc (dst, rsz);
+  unionent *dst = realloc_ent (sz, type);
 
   if (dst)
     {
@@ -425,19 +450,19 @@ dup_ent (unionent *&dst, unionent *src,
 static inline hostent *
 dup_ent (hostent *src)
 {
-  return (hostent *) dup_ent (_my_tls.locals.hostent_buf, (unionent *) src, unionent::t_hostent);
+  return (hostent *) dup_ent ((unionent *) src, unionent::t_hostent);
 }
 
 static inline protoent *
 dup_ent (protoent *src)
 {
-  return (protoent *) dup_ent (_my_tls.locals.protoent_buf, (unionent *) src, unionent::t_protoent);
+  return (protoent *) dup_ent ((unionent *) src, unionent::t_protoent);
 }
 
 static inline servent *
 dup_ent (servent *src)
 {
-  return (servent *) dup_ent (_my_tls.locals.servent_buf, (unionent *) src, unionent::t_servent);
+  return (servent *) dup_ent ((unionent *) src, unionent::t_servent);
 }
 
 /* exported as getprotobyname: standards? */
@@ -857,6 +882,287 @@ cygwin_gethostbyaddr (const char *addr,
   return res;
 }
 
+static void
+memcpy4to6 (char *dst, const u_char *src)
+{
+  const unsigned int h[] = {0, 0, htonl (0xFFFF)};
+  memcpy (dst, h, 12);
+  memcpy (dst + 12, src, NS_INADDRSZ);
+}
+
+static hostent *
+gethostby_helper (const char *name, const int af, const int type,
+  const int addrsize_in, const int addrsize_out)
+{
+  /* Get the data from the name server */
+  const int maxcount = 3;
+  int old_errno, ancount = 0, anlen = 1024, msgsize = 0;
+  u_char *ptr, *msg = NULL;
+  int sz;
+  hostent *ret;
+  char *string_ptr;
+
+  while ((anlen > msgsize) && (ancount++ < maxcount))
+    {
+      msgsize = anlen;
+      ptr = (u_char *) realloc (msg, msgsize);
+      if (ptr == NULL )
+ {
+  old_errno = errno;
+  free (msg);
+  set_errno (old_errno);
+  h_errno = NETDB_INTERNAL;
+  return NULL;
+ }
+      msg = ptr;
+      anlen = res_search (name, ns_c_in, type, msg, msgsize);
+    }
+
+  if (ancount >= maxcount)
+    {
+      free (msg);
+      h_errno = NO_RECOVERY;
+      return NULL;
+    }
+  if (anlen < 0) /* errno and h_errno are set */
+    {
+      old_errno = errno;
+      free (msg);
+      set_errno (old_errno);
+      return NULL;
+    }
+  u_char *eomsg = msg + anlen - 1;
+
+
+  /* We scan the answer records to determine the required memory size.
+     They can be corrupted and we don't fully trust that the message
+     follows the standard exactly. glibc applies some checks that
+     we emulate.
+     The answers are copied in the hostent structure in a second scan.
+     To simplify the second scan we store information as follows:
+     - "class" is replaced by the compressed name size
+     - the first 16 bits of the "ttl" store the expanded name size + 1
+     - the last 16 bits of the "ttl" store the offset to the next valid record.
+     Note that "type" is rewritten in host byte order. */
+  
+  class record {
+  public:
+    unsigned type: 16; // type
+    unsigned complen: 16;       // class or compressed length
+    unsigned namelen1: 16;      // expanded length (with final 0)
+    unsigned next_o: 16;        // offset to next valid
+    unsigned size: 16;          // data size
+    u_char data[];              // data
+    record * next () { return (record *) (((char *) this) + next_o); }
+    void set_next ( record * nxt) { next_o = ((char *) nxt) - ((char *) this); }
+    u_char * name () { return (u_char *) (((char *) this) - complen); }
+  };
+
+  record * anptr = NULL, * prevptr = NULL, * curptr;
+  int i, alias_count = 0, string_size = 0, address_count = 0;
+  int complen, namelen1 = 0, address_len = 0, antype, anclass, ansize;
+
+  /* Get the count of answers */
+  ancount = ntohs (((HEADER *) msg)->ancount);
+
+  /* Skip the question, it was verified by res_send */
+  ptr = msg + sizeof (HEADER);
+  if ((complen = dn_skipname (ptr, eomsg)) < 0)
+    goto corrupted;    
+  /* Point to the beginning of the answer section */
+  ptr += complen + NS_QFIXEDSZ;
+  
+  /* Scan the answer records to determine the sizes */
+  for (i = 0; i < ancount; i++, ptr = curptr->data + ansize)
+    {
+      if ((complen = dn_skipname (ptr, eomsg)) < 0)
+ goto corrupted;
+
+      curptr = (record *) (ptr + complen);
+      antype = ntohs (curptr->type);
+      anclass = ntohs (curptr->complen);
+      ansize = ntohs (curptr->size);
+      /* Class must be internet */
+      if (anclass != ns_c_in)
+ continue;
+
+      curptr->complen = complen;
+      if ((namelen1 = dn_length1 (msg, eomsg, curptr-> name())) <= 0)
+ goto corrupted;
+
+      if (antype == ns_t_cname)
+ {
+  alias_count++;
+  string_size += namelen1;
+ }
+      else if (antype == type)
+ {
+  ansize = ntohs (curptr->size);
+  if (ansize != addrsize_in)
+    continue;
+  if (address_count == 0)
+    {
+      address_len = namelen1;
+      string_size += namelen1;
+    }
+  else if (address_len != namelen1)
+    continue;
+  address_count++;
+ }
+      /* Update the records */
+      curptr->type = antype; /* Host byte order */
+      curptr->namelen1 = namelen1;
+      if (! anptr)
+ anptr = prevptr = curptr;
+      else
+ {
+  prevptr->set_next (curptr);
+  prevptr = curptr;
+ }
+    }
+
+  /* If there is no address, quit */
+  if (address_count == 0)
+    {
+      free (msg);
+      h_errno = NO_DATA;
+      return NULL;
+    }
+  
+  /* Determine the total size */
+  sz = DWORD_round (sizeof(hostent))
+       + sizeof (char *) * (alias_count + address_count + 2)
+       + string_size
+       + address_count * addrsize_out;
+
+  ret = (hostent *) realloc_ent (sz, unionent::t_hostent);
+  if (! ret)
+    {
+      old_errno = errno;
+      free (msg);
+      set_errno (old_errno);
+      h_errno = NETDB_INTERNAL;
+      return NULL;
+    }
+
+  ret->h_addrtype = af;
+  ret->h_length = addrsize_out;
+  ret->h_aliases = (char **) (((char *) ret) + DWORD_round (sizeof(hostent)));
+  ret->h_addr_list = ret->h_aliases + alias_count + 1;
+  string_ptr = (char *) (ret->h_addr_list + address_count + 1);
+
+  /* Rescan the answers */
+  ancount = alias_count + address_count; /* Valid records */
+  alias_count = address_count = 0;
+
+  for (i = 0, curptr = anptr; i < ancount; i++, curptr = curptr->next ())
+    {
+      antype = curptr->type;
+      if (antype == ns_t_cname)
+ {
+  complen = dn_expand (msg, eomsg, curptr->name (), string_ptr, string_size);
+#ifdef DEBUGGING
+  if (complen != curptr->complen)  
+    go to debugging;
+#endif
+  ret->h_aliases[alias_count++] = string_ptr;
+  namelen1 = curptr->namelen1;
+  string_ptr += namelen1;
+  string_size -= namelen1;  
+  continue;
+ }
+      if (antype == type)
+    {
+      if (address_count == 0)
+ {
+  complen = dn_expand (msg, eomsg, curptr->name(), string_ptr, string_size);
+#ifdef DEBUGGING
+  if (complen != curptr->complen)  
+    go to debugging;
+#endif
+  ret->h_name = string_ptr;
+  namelen1 = curptr->namelen1;
+  string_ptr += namelen1;
+  string_size -= namelen1;  
+ }
+      ret->h_addr_list[address_count++] = string_ptr;
+      if (addrsize_in != addrsize_out)
+ memcpy4to6 (string_ptr, curptr->data);
+      else
+ memcpy (string_ptr, curptr->data, addrsize_in);
+      string_ptr += addrsize_out;
+      string_size -= addrsize_out;
+      continue;
+    }
+#ifdef DEBUGGING
+      /* Should not get here */
+      go to debugging;
+#endif
+    }
+#ifdef DEBUGGING
+  if (string_size < 0)  
+    go to debugging;
+#endif      
+  
+  free (msg);
+
+  ret->h_aliases[alias_count] = NULL;
+  ret->h_addr_list[address_count] = NULL;
+
+  return ret;
+
+ corrupted:
+  free (msg);
+  /* Hopefully message corruption errors are temporary.
+     Should it be NO_RECOVERY ? */
+  h_errno = TRY_AGAIN;
+  return NULL;
+
+
+#ifdef DEBUGGING
+ debugging:
+   system_printf ("Please debug.");
+   free (msg);
+   free (ret);
+   h_errno = NO_RECOVERY;
+   return NULL;
+#endif
+}
+
+/* gethostbyname2: standards? */
+extern "C" struct hostent *
+gethostbyname2 (const char *name, int af)
+{
+  sig_dispatch_pending ();
+  myfault efault;
+  if (efault.faulted (EFAULT))
+    return NULL;
+
+  if (!(_res.options & RES_INIT))
+      res_init();
+  bool v4to6 = _res.options & RES_USE_INET6;
+
+  int type, addrsize_in, addrsize_out;
+  switch (af)
+    {
+    case AF_INET:
+      addrsize_in = NS_INADDRSZ;
+      addrsize_out = (v4to6) ? NS_IN6ADDRSZ : NS_INADDRSZ;
+      type = ns_t_a;
+      break;
+    case AF_INET6:
+      addrsize_in = addrsize_out = NS_IN6ADDRSZ;
+      type = ns_t_aaaa;
+      break;
+    default:
+      set_errno (EAFNOSUPPORT);
+      h_errno = NETDB_INTERNAL;
+      return NULL;
+    }
+  
+  return gethostby_helper (name, af, type, addrsize_in, addrsize_out);
+}
+
 /* exported as accept: standards? */
 extern "C" int
 cygwin_accept (int fd, struct sockaddr *peer, socklen_t *len)
@@ -2486,10 +2792,6 @@ cygwin_sendmsg (int fd, const struct msg
  * SOFTWARE.
  */
 
-#define IN6ADDRSZ 16
-#define INADDRSZ 4
-#define INT16SZ 2
-
 /* int
  * inet_pton4(src, dst)
  * like inet_aton() but without all the hexadecimal and shorthand.
Index: include/cygwin/version.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/version.h,v
retrieving revision 1.284
diff -u -p -r1.284 version.h
--- include/cygwin/version.h 19 Feb 2009 09:22:51 -0000 1.284
+++ include/cygwin/version.h 3 Mar 2009 11:54:12 -0000
@@ -349,12 +349,13 @@ details. */
       198: Export reallocf.
       199: Export open_wmemstream.
       200: Export mbsnrtowcs, wcsnrtombs.
+      201: Export gethostbyname2.
      */
 
      /* Note that we forgot to bump the api for ualarm, strtoll, strtoull */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 200
+#define CYGWIN_VERSION_API_MINOR 201
 
      /* There is also a compatibity version number associated with the
  shared memory regions.  It is incremented when incompatible
Index: libc/minires.c
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/libc/minires.c,v
retrieving revision 1.5
diff -u -p -r1.5 minires.c
--- libc/minires.c 3 Dec 2008 16:37:53 -0000 1.5
+++ libc/minires.c 3 Mar 2009 11:54:12 -0000
@@ -1,6 +1,6 @@
 /* minires.c.  Stub synchronous resolver for Cygwin.
 
-   Copyright 2006, 2008 Red Hat, Inc.
+   Copyright 2006, 2008, 2009 Red Hat, Inc.
 
    Written by Pierre A. Humblet <[hidden email]>
 
@@ -99,6 +99,11 @@ static void get_options(res_state statp,
       DPRINTF(statp->options & RES_DEBUG, "%s: 1\n", words[i]);
       continue;
     }
+    if (!strcasecmp("inet6", words[i])) {
+      statp->options |= RES_USE_INET6;
+      DPRINTF(statp->options & RES_DEBUG, "%s: 1\n", words[i]);
+      continue;
+    }
     if (!strcasecmp("osquery", words[i])) {
       statp->use_os = 1;
       DPRINTF(statp->options & RES_DEBUG, "%s: 1\n", words[i]);
@@ -114,16 +119,22 @@ static void get_options(res_state statp,
  continue;
  }
       */
-      if (!strcasecmp("retry", words[i])) {
+      if (!strcasecmp("retry", words[i])
+  || !strcasecmp("attempts", words[i])) {
  if (value < 1)
   value = 1;
+ else if (value > RES_MAXRETRY)
+  value = RES_MAXRETRY;
  statp->retry = value;
  DPRINTF(statp->options & RES_DEBUG, "%s: %d\n", words[i], value);
  continue;
       }
-      if (!strcasecmp("retrans", words[i])) {
+      if (!strcasecmp("retrans", words[i])
+  || !strcasecmp("timeout", words[i])) {
  if (value < 1)
   value = 1;
+ else if (value > RES_MAXRETRANS)
+  value = RES_MAXRETRANS;
  statp->retrans = value;
  DPRINTF(statp->options & RES_DEBUG, "%s: %d\n", words[i], value);
  continue;
@@ -270,6 +281,9 @@ int res_ninit(res_state statp)
   int i;
 
   statp->res_h_errno = NETDB_SUCCESS;
+   /* Only debug may be set before calling init */
+  statp->options &= RES_DEBUG;
+  statp->options |= RES_INIT | RES_DEFAULT;
   statp->nscount = 0;
   statp->os_query = NULL;
   statp->retrans = RES_TIMEOUT; /* timeout in seconds */
@@ -299,9 +313,6 @@ int res_ninit(res_state statp)
     statp->nsaddr_list[i].sin_port = htons(NAMESERVER_PORT);
     bzero(statp->nsaddr_list[i].sin_zero, sizeof(statp->nsaddr_list[i].sin_zero));
   }
-  /* Only debug may be set before calling init */
-  statp->options &= RES_DEBUG;
-  statp->options |= RES_INIT | RES_DEFAULT;
   return 0;
 }
 
@@ -806,7 +817,7 @@ int dn_expand(const unsigned char *msg,
     exp_dn++;
   else do {
     if (len <= MAXLABEL) {
-      if ((length -= (len + 1)) > 0 /* Need space for final . */
+      if ((length -= (len + 1)) >= 0 /* Need space for final . */
   && comp_dn + len <= eomorig) {
  do { *exp_dn++ = *comp_dn++; } while (--len != 0);
  *exp_dn++ = '.';
@@ -836,7 +847,6 @@ expand_fail:
   return -1;
 }
 
-
 /*****************************************************************
  *
  dn_comp
@@ -926,8 +936,7 @@ int dn_comp(const char * exp_dn, u_char
 }
 
 /*****************************************************************
- *
- dn_skipname
+ * dn_skipname
 
  Measures the compressed domain name length and returns it.
  *****************************************************************/
@@ -949,3 +958,38 @@ int dn_skipname(const unsigned char *com
 
   return comp_dn - comp_dn_orig;
 }
+
+/*****************************************************************
+ * dn_length1    For internal use
+
+ Return length of uncompressesed name incl final 0.
+ *****************************************************************/
+
+int dn_length1(const unsigned char *msg, const unsigned char *eomorig,
+       const unsigned char *comp_dn)
+{
+  unsigned int len, length = 0;
+
+  errno = EINVAL;
+  if (comp_dn >= eomorig)
+    goto expand_fail;
+  else while ((len = *comp_dn++) != 0) {
+    if (len <= MAXLABEL) {
+      if ((comp_dn += len) <= eomorig)
+ length += len + 1;
+      else
+ goto expand_fail;
+    }
+    else if (len >= (128+64)) {
+      comp_dn = msg + (((len & ~(128+64)) << 8) + *comp_dn);
+      if (comp_dn >= eomorig)
+ goto expand_fail;
+    }
+    else
+      goto expand_fail;
+  }
+  return length;
+
+expand_fail:
+  return -1;
+}

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Pierre A. Humblet-2

----- Original Message -----
From: "Corinna Vinschen"
To: <cygwin-patches>
Sent: Tuesday, March 03, 2009 7:01 AM
Subject: Re: [Patch] gethostbyname2 again


| [|
| I attached the entire patch again with my changes.  I had to change the
| gethostby_helper function to define some of the variables at the start
| of the function, othewise gcc complained about jumps to a label crossing
| variable initializations.  The bump of the API minor number in
| include/cygwin/version.h was missing.  I also tweaked the formatting a bit.
|
| The ChangeLog entry is the same as in the OP, except for the additional
| reference to include/cygwin/version.h.  Please have a look.
|

Hello Corinna,

Thanks for fixing up the nits. The new patch looks good. I didn't find the new
Changelog entry but I trust you.

Pierre
 
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Corinna Vinschen-2
On Mar  3 10:05, Pierre A. Humblet wrote:
> From: "Corinna Vinschen"
> | The ChangeLog entry is the same as in the OP, except for the additional
> | reference to include/cygwin/version.h.  Please have a look.
>
> Hello Corinna,
>
> Thanks for fixing up the nits. The new patch looks good. I didn't find the new
> Changelog entry but I trust you.

Right between my signature and the patch, the last line :)


Corinna
 
--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Pierre A. Humblet-2

----- Original Message -----
From: "Corinna Vinschen"
To: <cygwin-patches>
Sent: Tuesday, March 03, 2009 10:13 AM
Subject: Re: [Patch] gethostbyname2 again


| On Mar  3 10:05, Pierre A. Humblet wrote:
| > From: "Corinna Vinschen"
| > | The ChangeLog entry is the same as in the OP, except for the additional
| > | reference to include/cygwin/version.h.  Please have a look.
| >
| > Hello Corinna,
| >
| > Thanks for fixing up the nits. The new patch looks good. I didn't find the new
| > Changelog entry but I trust you.
|
| Right between my signature and the patch, the last line :)

Indeed, there it is :) It's fine except that dn_length1 is now a new function in minires.c
instead of net.cc

Pierre

Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Christopher Faylor-8
In reply to this post by Pierre A. Humblet-2
On Mon, Mar 02, 2009 at 08:36:55PM -0500, Pierre A. Humblet wrote:
>    realloc_ent function, and call it from both dup_ent and the helper. That
> caused minor
>    changes in the 4 versions of dup_ent, and I don't know exactly what
> format to use in the ChangeLog

I would rather that you keep dup_ent as is so that there is no need to
do run-time checks on the type.  If you need to do something similar to
what is currently in dupent, then couldn't you still create a
realloc_ent but just pass in the destination pointer?  Or even just make
realloc_ent mimic realloc but do the rounding that seems to be the
impetus for your breaking this out into a separate function.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Pierre A. Humblet-2

----- Original Message -----
From: "Christopher Faylor" <>
To: <[hidden email]>
Sent: Tuesday, March 03, 2009 10:38 AM
Subject: Re: [Patch] gethostbyname2 again


| On Mon, Mar 02, 2009 at 08:36:55PM -0500, Pierre A. Humblet wrote:
| >    realloc_ent function, and call it from both dup_ent and the helper. That
| > caused minor
| >    changes in the 4 versions of dup_ent, and I don't know exactly what
| > format to use in the ChangeLog
|
| I would rather that you keep dup_ent as is so that there is no need to
| do run-time checks on the type.  If you need to do something similar to
| what is currently in dupent, then couldn't you still create a
| realloc_ent but just pass in the destination pointer?  Or even just make
| realloc_ent mimic realloc but do the rounding that seems to be the
| impetus for your breaking this out into a separate function.

Chris,

The impetus is that the new helper function is capable of formatting a fine
hostent in a single block of memory. So it doesn't need to have dup_ent
copy it in yet another memory block.

However it still needs to store a pointer to the block of memory somewhere,
so that it can be freed later, and reusing the tls.locals seems logical. If it does
that, then it must also free or realloc whatever is stored there.
That's what realloc_ent does.
The rounding is not essential, it's just nice to do it consistently in one place.

To avoid real-time checks, I could do as what dup_ent does, and have 4 versions
of the realloc_ent function, one main one with dst and sz arguments (that one
would be called by dup_ent without any  run-time checks) and 3 (actually only
1 is needed for now) that invoke the main one with the correct dst based on the
type of the src argument . The src argument would be null but would have the
right type! That seems to meet your wishes. OK?

Pierre



Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Christopher Faylor-8
On Tue, Mar 03, 2009 at 12:50:21PM -0500, Pierre A. Humblet wrote:

>
>----- Original Message -----
>From: "Christopher Faylor" <>
>To: <[hidden email]>
>Sent: Tuesday, March 03, 2009 10:38 AM
>Subject: Re: [Patch] gethostbyname2 again
>
>
>| On Mon, Mar 02, 2009 at 08:36:55PM -0500, Pierre A. Humblet wrote:
>| >    realloc_ent function, and call it from both dup_ent and the helper. That
>| > caused minor
>| >    changes in the 4 versions of dup_ent, and I don't know exactly what
>| > format to use in the ChangeLog
>|
>| I would rather that you keep dup_ent as is so that there is no need to
>| do run-time checks on the type.  If you need to do something similar to
>| what is currently in dupent, then couldn't you still create a
>| realloc_ent but just pass in the destination pointer?  Or even just make
>| realloc_ent mimic realloc but do the rounding that seems to be the
>| impetus for your breaking this out into a separate function.
>
>Chris,
>
>The impetus is that the new helper function is capable of formatting a fine
>hostent in a single block of memory. So it doesn't need to have dup_ent
>copy it in yet another memory block.
>
>However it still needs to store a pointer to the block of memory somewhere,
>so that it can be freed later, and reusing the tls.locals seems logical. If it does
>that, then it must also free or realloc whatever is stored there.
>That's what realloc_ent does.
>The rounding is not essential, it's just nice to do it consistently in one place.
>
>To avoid real-time checks, I could do as what dup_ent does, and have 4 versions
>of the realloc_ent function, one main one with dst and sz arguments (that one
>would be called by dup_ent without any  run-time checks) and 3 (actually only
>1 is needed for now) that invoke the main one with the correct dst based on the
>type of the src argument . The src argument would be null but would have the
>right type! That seems to meet your wishes. OK?

Yes.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Pierre A. Humblet-2

----- Original Message -----
From: "Christopher Faylor" <cgf-use-the-mailinglist-please>
To: <cygwin-patches>
Sent: Friday, March 06, 2009 12:44 AM
Subject: Re: [Patch] gethostbyname2 again


| On Tue, Mar 03, 2009 at 12:50:21PM -0500, Pierre A. Humblet wrote:
| >
| >To avoid real-time checks, I could do as what dup_ent does, and have 4 versions
| >of the realloc_ent function, one main one with dst and sz arguments (that one
| >would be called by dup_ent without any  run-time checks) and 3 (actually only
| >1 is needed for now) that invoke the main one with the correct dst based on the
| >type of the src argument . The src argument would be null but would have the
| >right type! That seems to meet your wishes. OK?
|
| Yes.

OK, here it the patch for realloc_ent. See also attachement.
The third chunk (the change to dup_ent) is not essential.

In addition in the patch that Corinna sent on March 3, the line
+  ret = (hostent *) realloc_ent (sz, unionent::t_hostent);
should be changed to
ret = realloc_ent (sz,  (hostent *) NULL);

In the Changelog the line
  (dup_ent): Remove dst argument and call realloc_ent.
should either be deleted or "Remove dst argument and c" should
be replaced by "C".

Pierre

Index: net.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/net.cc,v
retrieving revision 1.249
diff -u -p -r1.249 net.cc
--- net.cc 3 Mar 2009 11:44:17 -0000 1.249
+++ net.cc 6 Mar 2009 14:28:46 -0000
@@ -264,6 +264,25 @@ struct pservent
 
 static const char *entnames[] = {"host", "proto", "serv"};
 
+static unionent *
+realloc_ent (unionent *&dst, int sz)
+{
+   /* Allocate the storage needed.  Allocate a rounded size to attempt to force
+      reuse of this buffer so that a poorly-written caller will not be using
+      a freed buffer. */
+   unsigned rsz = 256 * ((sz + 255) / 256);
+   unionent * ptr;
+   if ((ptr = (unionent *) realloc (dst, rsz)))
+     dst = ptr;
+   return ptr;
+}
+
+static inline hostent *
+realloc_ent (int sz, hostent * )
+{
+   return (hostent *) realloc_ent (_my_tls.locals.hostent_buf, sz);
+}
+
 /* Generic "dup a {host,proto,serv}ent structure" function.
    This is complicated because we need to be able to free the
    structure at any point and we can't rely on the pointer contents
@@ -355,13 +374,8 @@ dup_ent (unionent *&dst, unionent *src,
  }
     }
 
-  /* Allocate the storage needed.  Allocate a rounded size to attempt to force
-     reuse of this buffer so that a poorly-written caller will not be using
-     a freed buffer. */
-  unsigned rsz = 256 * ((sz + 255) / 256);
-  dst = (unionent *) realloc (dst, rsz);
-
-  if (dst)
+  /* Allocate the storage needed.  */
+  if (realloc_ent (dst, sz))
     {
       memset (dst, 0, sz);
       /* This field is common to all *ent structures but named differently

realloc_ent.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Christopher Faylor-8
On Fri, Mar 06, 2009 at 09:41:00AM -0500, Pierre A. Humblet wrote:
>
>----- Original Message -----
>[snip]

I see you've joined the throng of people who duplicate bits from the
header in the body of a message.  Do you really need to do this?

>
>| On Tue, Mar 03, 2009 at 12:50:21PM -0500, Pierre A. Humblet wrote:
>| >
>| >To avoid real-time checks, I could do as what dup_ent does, and have 4 versions
>| >of the realloc_ent function, one main one with dst and sz arguments (that one
>| >would be called by dup_ent without any  run-time checks) and 3 (actually only
>| >1 is needed for now) that invoke the main one with the correct dst based on the
>| >type of the src argument . The src argument would be null but would have the
>| >right type! That seems to meet your wishes. OK?
>|
>| Yes.
>
>OK, here it the patch for realloc_ent. See also attachement.
>The third chunk (the change to dup_ent) is not essential.
>
>In addition in the patch that Corinna sent on March 3, the line
>+  ret = (hostent *) realloc_ent (sz, unionent::t_hostent);
>should be changed to
>ret = realloc_ent (sz,  (hostent *) NULL);
>
>In the Changelog the line
>  (dup_ent): Remove dst argument and call realloc_ent.
>should either be deleted or "Remove dst argument and c" should
>be replaced by "C".

This is ok with one very minor formatting nit.  Please check in with an
appropriate changelog.

>+static inline hostent *
>+realloc_ent (int sz, hostent * )
                                ^
                          extra space

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Pierre A. Humblet-2

----- Original Message -----
From: "Christopher Faylor" <cgf-use-the-mailinglist-please>

| On Fri, Mar 06, 2009 at 09:41:00AM -0500, Pierre A. Humblet wrote:
| >
||
| This is ok with one very minor formatting nit.  Please check in with an
| appropriate changelog.
|
| >+static inline hostent *
| >+realloc_ent (int sz, hostent * )
|                                ^
|                          extra space

OK. I can't do that before Mon eve. It would be easier if Corinna could merge this
patch and the previous one (she has the latest version) and apply the whole thing
at once, with one changelog  block.

Pierre
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] gethostbyname2 again

Corinna Vinschen-2
On Mar  6 10:56, Pierre A. Humblet wrote:

> From: "Christopher Faylor" <cgf-use-the-mailinglist-please>
> | This is ok with one very minor formatting nit.  Please check in with an
> | appropriate changelog.
> |
> | >+static inline hostent *
> | >+realloc_ent (int sz, hostent * )
> |                                ^
> |                          extra space
>
> OK. I can't do that before Mon eve. It would be easier if Corinna could merge this
> patch and the previous one (she has the latest version) and apply the whole thing
> at once, with one changelog  block.

Done.  Thanks again for this function.  This will give us a few new
occasions for the future, as I pointed out in my first reply.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat