Re: [Patch] gethostbyname2 again
On Mar 6 10:56, Pierre A. Humblet wrote: > From: "Christopher Faylor" > | 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
Re: [Patch] gethostbyname2 again
- Original Message - From: "Christopher Faylor" | 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
Re: [Patch] gethostbyname2 again
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
Re: [Patch] gethostbyname2 again
- Original Message -
From: "Christopher Faylor"
To:
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 - 1.249
+++ net.cc 6 Mar 2009 14:28:46 -
@@ -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
Description: Binary data
Re: [Patch] gethostbyname2 again
On Tue, Mar 03, 2009 at 12:50:21PM -0500, Pierre A. Humblet wrote: > >- Original Message - >From: "Christopher Faylor" <> >To: >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
Re: [Patch] gethostbyname2 again
- Original Message - From: "Christopher Faylor" <> To: 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
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. cgf
Re: [Patch] gethostbyname2 again
- Original Message - From: "Corinna Vinschen" To: 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
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 :) Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Project Co-Leader cygwin AT cygwin DOT com Red Hat
Re: [Patch] gethostbyname2 again
- Original Message - From: "Corinna Vinschen" To: 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
Re: [Patch] gethostbyname2 again
[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 - 1.202
+++ cygwin.din 3 Mar 2009 11:54:11 -
@@ -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 - 1.249
+++ net.cc 3 Mar 2009 11:54:12 -
@@ -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
extern "C"
{
@@ -53,6 +55,9 @@ extern "C"
int sscanf (const char *, const char *, ...);
i
[Patch] gethostbyname2 again
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 * 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 Description: Binary data try_gethostbyname.c Description: Binary data
