Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
On Sat, Jun 21, 2003 at 07:47:37PM -0400, Alan DeKok wrote: Oliver Graf [EMAIL PROTECTED] wrote: This patchs enables the detection of the correct gethostby(name|addr)_r command, which is needed by a threaded radiusd. Unless I'm greatly mistaken, that functionality was added many months ago. See 'src/main/misc.c' I don't see anything for gethostbyname_r. Or I have a different cvs than you. I haven't looked at the diffs though, so it may not be relevant. For me its relevant. Without freeradius is not thread-safe. But the server ALREADY uses the '..._r' functions. What's the problem? Or am I missing something? The BSD style check is done last. BSD style gethostby is not thread safe on glibc2 systems. Cause the BSD check is done last, the configure script overwrites the previous detected GNU style gethostbyaddr_r. Just look at the diff, you will see that the sequence is changed (and a warning is added). Oliver. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
On Sun, Jun 22, 2003 at 08:47:25AM +0200, Oliver Graf wrote: On Sat, Jun 21, 2003 at 07:47:37PM -0400, Alan DeKok wrote: Oliver Graf [EMAIL PROTECTED] wrote: This patchs enables the detection of the correct gethostby(name|addr)_r command, which is needed by a threaded radiusd. Unless I'm greatly mistaken, that functionality was added many months ago. See 'src/main/misc.c' I don't see anything for gethostbyname_r. Or I have a different cvs than you. I double checked this. There is no support in src/lib/misc.c for glibc2 style gethostbyname_r (hostent as param, not as return value) in cvs (I can't find src/main/misc.c, but this perhaps an typo of you). I haven't looked at the diffs though, so it may not be relevant. For me its relevant. Without freeradius is not thread-safe. But the server ALREADY uses the '..._r' functions. What's the problem? Or am I missing something? The BSD style check is done last. BSD style gethostby is not thread safe on glibc2 systems. Cause the BSD check is done last, the configure script overwrites the previous detected GNU style gethostbyaddr_r. Just look at the diff, you will see that the sequence is changed (and a warning is added). again, an cvs upd shows that the order is wrong and was wrong. at least for me. Perhaps an autoconf oddity? I use autoconf 2.57. But I can't remember that autoconf 1.x did it in another way. The last definition goes, and as glibc2 systems both have gethostby_r and gethostby, and the gethostby check is done last, the configure script will always prefer the bsd variant which uses static storage and so is bad for threading. iOliver. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
RE: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
From: Alan DeKok
Sent: Sunday, 22 June 2003 9:48 AM
Oliver Graf [EMAIL PROTECTED] wrote:
This patchs enables the detection of the correct
gethostby(name|addr)_r command, which is needed by a threaded radiusd.
Unless I'm greatly mistaken, that functionality was added many
months ago. See 'src/main/misc.c'
(That's src/lib/misc.c)
Currently this file uses gethostbyaddr{,_r} either BSD, GNU or SYSV
However, gethostbyname{,_r} is only using BSD or SYSV
I haven't looked at the diffs though, so it may not be relevant.
For me its relevant. Without freeradius is not thread-safe.
But the server ALREADY uses the '..._r' functions. What's the
problem? Or am I missing something?
The problems are:
gethostbyaddr_r determination will pick BSD (The non-_r version)
over GNU or SYSV. This is because the BSD version was added to configure
after the other two, and added _after_.
gethostbyname_r detection was backed out of configure (meaning only the
BSD (non-_r) version is used) because it was using a simple Does
gethostbyname_r exist test, at the link I posted earlier... and hence
not able to tell the difference between the SYSV and GNU versions.
However, problems with the patch:
the patch to src/lib/misc.c shows a cut and paste oversight by testing
GETHOSTBYADDRRSTYLE when choosing the gethostbyname_r function to call.
Oh, and when submitting patches to configure.in, I'd suggest submitting
the equivalent patch to configure. Otherwise if someone overlooks the
regeneration, it appears to not work for no apparent reason.
The rest looks OK. I'm going to apply it to my copy, try it out,
and commit it this evening barring problems.
--
=
Paul TBBle Hampson
Bubblesworth Pty Ltd (ABN: 51 095 284 361)
[EMAIL PROTECTED]
The Creation of the Universe was made
possible by a grant from Texas Instruments.
-- PBS
-
Random signature generator 3.0 by Paul TBBle Hampson
=
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
Oliver Graf [EMAIL PROTECTED] wrote: The BSD style check is done last. BSD style gethostby is not thread safe on glibc2 systems. Cause the BSD check is done last, the configure script overwrites the previous detected GNU style gethostbyaddr_r. Just look at the diff, you will see that the sequence is changed (and a warning is added). The solution is NOT to re-order the checks so that they *accidentally* work. The solution is to check for one re-entrant version, if that fails, check for another, etc. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
Oliver Graf [EMAIL PROTECTED] wrote: Oh, and when submitting patches to configure.in, I'd suggest submitting the equivalent patch to configure. Otherwise if someone overlooks the regeneration, it appears to not work for no apparent reason. Hmmm. I did not do this, cause other projects I'm participiating in do not keep configure in cvs cause its an autogenerated thing. But I will try to keep this in mind, if I have to do another configure.in patch sometimes. Personally, I'd prefer to *not* see patches to 'configure'. They tend to be huge and pointless, as they can be re-generated from 'configure.in'. The reason that 'configure' is in CVS is that it's easier that way. I've seen projects where the instructions for the snapshots are run autoconf, then ./configure But if you have a different version of autoconf than they do, it doesn't work. And even if you have the same version of autoconf, they didn't bother to explain which extra magic parameters you need to pass to it, etc. Having 'configure' in CVS means that the developers need to take an extra step, involving ~15 seconds when they make (rare) changes to the configure scripts. NOT having it in CVS means that endless other developers and users will curse your name. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
RE: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
From: Alan DeKok Sent: Sunday, 22 June 2003 10:50 PM Oliver Graf [EMAIL PROTECTED] wrote: The BSD style check is done last. BSD style gethostby is not thread safe on glibc2 systems. Cause the BSD check is done last, the configure script overwrites the previous detected GNU style gethostbyaddr_r. Just look at the diff, you will see that the sequence is changed (and a warning is added). The solution is NOT to re-order the checks so that they *accidentally* work. The solution is to check for one re-entrant version, if that fails, check for another, etc. OK, that's done now. I hope what's there now is more acceptable. Consider me somewhat chastised. :-) -- = Paul TBBle Hampson Bubblesworth Pty Ltd (ABN: 51 095 284 361) [EMAIL PROTECTED] When the DM smiles, it's too late -- Ancient Geek Proverb - Random signature generator 3.0 by Paul TBBle Hampson = - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
On Sat, Jun 21, 2003 at 02:52:50AM +1000, Paul Hampson wrote:
From: Oliver Graf
Sent: Saturday, 21 June 2003 12:39 AM
This patchs enables the detection of the correct
gethostby(name|addr)_r command, which is needed by a threaded radiusd.
Which patch?
Hmpf... now its attached...
Oh, did you have a look at the CVS history for configure.in and
configure.in? I noticed the other day that gethostbyname_r seems
to have been in and out (and shaken all about) about a year ago.
Yep, I did.
I haven't looked at the diffs though, so it may not be relevant.
For me its relevant. Without freeradius is not thread-safe.
Oliver.
--- freeradius-snapshot-20030529/src/include/autoconf.h.in.orig Fri Jun 20 16:24:14
2003
+++ freeradius-snapshot-20030529/src/include/autoconf.h.in Fri Jun 20 16:25:11
2003
@@ -48,7 +48,9 @@
/* style of gethost*_r functions */
#define GNUSTYLE 1
#define SYSVSTYLE 2
+#define BSDSTYLE 3
#undef GETHOSTBYADDRRSTYLE
+#undef GETHOSTBYNAMERSTYLE
/* Do we have the crypt function ? */
#undef HAVE_CRYPT
--- freeradius-snapshot-20030529/src/lib/misc.c.origFri Jun 20 16:24:00 2003
+++ freeradius-snapshot-20030529/src/lib/misc.c Fri Jun 20 16:24:20 2003
@@ -83,20 +83,29 @@
{
struct hostent *hp;
uint32_t a;
-#ifdef HAVE_GETHOSTBYNAME_R
+#ifdef GETHOSTBYNAMERSTYLE
+#if (GETHOSTBYNAMERSTYLE == SYSVSTYLE) || (GETHOSTBYNAMERSTYLE == GNUSTYLE)
struct hostent result;
int error;
char buffer[2048];
#endif
+#endif
if ((a = ip_addr(host)) != htonl(INADDR_NONE))
return a;
-#ifndef HAVE_GETHOSTBYNAME_R
+#ifdef GETHOSTBYADDRRSTYLE
+#if GETHOSTBYADDRRSTYLE == SYSVSTYLE
+ hp = gethostbyname_r(host, result, buffer, sizeof(buffer), error);
+#elif GETHOSTBYADDRRSTYLE == GNUSTYLE
+ gethostbyname_r(host, result, buffer, sizeof(buffer), hp, error);
+#else
hp = gethostbyname(host);
+#endif
#else
- hp = gethostbyname_r(host, result, buffer, sizeof(buffer), error);
+ hp = gethostbyname(host);
#endif
+
if (hp == NULL) {
return htonl((uint32_t) INADDR_NONE);
}
--- freeradius-snapshot-20030529/acconfig.h.origFri Jun 20 16:23:44 2003
+++ freeradius-snapshot-20030529/acconfig.h Fri Jun 20 16:24:32 2003
@@ -11,7 +11,9 @@
/* style of gethost*_r functions */
#define GNUSTYLE 1
#define SYSVSTYLE 2
+#define BSDSTYLE 3
#undef GETHOSTBYADDRRSTYLE
+#undef GETHOSTBYNAMERSTYLE
/* Do we have the crypt function ? */
#undef HAVE_CRYPT
--- freeradius-snapshot-20030529/configure.in.orig Fri Jun 20 16:23:50 2003
+++ freeradius-snapshot-20030529/configure.in Fri Jun 20 16:24:20 2003
@@ -648,6 +648,10 @@
gethostbyaddrrstyle=
AC_MSG_CHECKING([gethostbyaddr_r() syntax])
+AC_TRY_COMPILE([#include netdb.h], [ gethostbyaddr(NULL, 0, 0) ], [
+ AC_DEFINE(GETHOSTBYADDRRSTYLE, BSDSTYLE)
+ gethostbyaddrrstyle=BSD
+])
AC_TRY_COMPILE([#include netdb.h], [ gethostbyaddr_r(NULL, 0, 0, NULL, NULL, 0,
NULL) ] , [
AC_DEFINE(GETHOSTBYADDRRSTYLE, SYSVSTYLE)
gethostbyaddrrstyle=SYSV
@@ -657,15 +661,38 @@
gethostbyaddrrstyle=GNU
])
-AC_TRY_COMPILE([#include netdb.h], [ gethostbyaddr(NULL, 0, 0) ], [
- AC_DEFINE(GETHOSTBYADDRRSTYLE, BSDSTYLE)
- gethostbyaddrrstyle=BSD
-])
-
if test x$gethostbyaddrrstyle = x; then
AC_MSG_RESULT([none! It must not exist, here.])
else
AC_MSG_RESULT([${gethostbyaddrrstyle}-style])
+ if test $gethostbyaddrrstyle = BSD; then
+ AC_MSG_WARN([ ** BSD Style gethostbyaddr might NOT be thread-safe!
** ])
+ fi
+fi
+
+
+gethostbynamerstyle=
+AC_MSG_CHECKING([gethostbyname_r() syntax])
+AC_TRY_COMPILE([#include netdb.h], [ gethostbyname(NULL) ], [
+ AC_DEFINE(GETHOSTBYNAMERSTYLE, BSDSTYLE)
+ gethostbynamerstyle=BSD
+])
+AC_TRY_COMPILE([#include netdb.h], [ gethostbyname_r(NULL, NULL, NULL, 0, NULL) ] ,
[
+ AC_DEFINE(GETHOSTBYNAMERSTYLE, SYSVSTYLE)
+ gethostbyaddrrstyle=SYSV
+])
+AC_TRY_COMPILE([#include netdb.h], [ gethostbyname_r(NULL, NULL, NULL, 0, NULL,
NULL) ], [
+ AC_DEFINE(GETHOSTBYNAMERSTYLE, GNUSTYLE)
+ gethostbynamerstyle=GNU
+])
+
+if test x$gethostbynamerstyle = x; then
+ AC_MSG_RESULT([none! It must not exist, here.])
+else
+ AC_MSG_RESULT([${gethostbynamerstyle}-style])
+ if test $gethostbynamerstyle = BSD; then
+ AC_MSG_WARN([ ** BSD Style gethostbyname might NOT be thread-safe!
** ])
+ fi
fi
Re: [PATCH] gethostbyaddr_r and gethostbyname_r corrections
Oliver Graf [EMAIL PROTECTED] wrote: This patchs enables the detection of the correct gethostby(name|addr)_r command, which is needed by a threaded radiusd. Unless I'm greatly mistaken, that functionality was added many months ago. See 'src/main/misc.c' I haven't looked at the diffs though, so it may not be relevant. For me its relevant. Without freeradius is not thread-safe. But the server ALREADY uses the '..._r' functions. What's the problem? Or am I missing something? Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
