pgsql: Define INADDR_NONE on Solaris when it's missing.
Log Message:
-----------
Define INADDR_NONE on Solaris when it's missing. Per a couple of buildfarm
members complaining.
Modified Files:
--------------
pgsql/src/include/port:
solaris.h (r1.17 -> r1.18)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/port/solaris.h?r1=1.17&r2=1.18)
mha@postgresql.org (Magnus Hagander) writes:
Log Message:
-----------
Define INADDR_NONE on Solaris when it's missing. Per a couple of buildfarm
members complaining.
This seems likely to break as much as it fixes, since there's no very
good reason to assume that whatever header should define INADDR_NONE
has been included before the os.h header file has been read.
Possibly more to the point, where are we using INADDR_NONE anyway?
regards, tom lane
On Thu, Jan 28, 2010 at 16:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:
mha@postgresql.org (Magnus Hagander) writes:
Log Message:
-----------
Define INADDR_NONE on Solaris when it's missing. Per a couple of buildfarm
members complaining.This seems likely to break as much as it fixes, since there's no very
good reason to assume that whatever header should define INADDR_NONE
has been included before the os.h header file has been read.
Hmm. Where would you suggest it goes?
The addition of such a define is in a lot of places on the net as
fixing just this issue, and was also recommended by Zdenek as the fix
for Solaris. But I can agree it may be in the wrong place :-)
Possibly more to the point, where are we using INADDR_NONE anyway?
In the RADIUS code.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 16:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Possibly more to the point, where are we using INADDR_NONE anyway?
In the RADIUS code.
Oh, that's why it isn't in my tree and has zero portability track record ...
I think what this shows is we should look for a way to avoid using
INADDR_NONE. What's your grounds for believing it's portable at all?
In the Single Unix Spec I only see INADDR_ANY and INADDR_BROADCAST
defined.
regards, tom lane
On Thu, Jan 28, 2010 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 16:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Possibly more to the point, where are we using INADDR_NONE anyway?
In the RADIUS code.
Oh, that's why it isn't in my tree and has zero portability track record ...
I think what this shows is we should look for a way to avoid using
INADDR_NONE. What's your grounds for believing it's portable at all?
In the Single Unix Spec I only see INADDR_ANY and INADDR_BROADCAST
defined.
Um, I don't think I have any specific grounds for it, other than
having seen it in a lot of other software :-)
From some more googling
(http://www.opengroup.org/onlinepubs/000095399/functions/inet_addr.html),
it says it will return (in_addr_t)(-1), though, so maybe we should
just move that #ifdef out to some global place?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think what this shows is we should look for a way to avoid using
INADDR_NONE.
From some more googling
(http://www.opengroup.org/onlinepubs/000095399/functions/inet_addr.html),
it says it will return (in_addr_t)(-1), though, so maybe we should
just move that #ifdef out to some global place?
Given the way that's written, I think we should just compare the result
to (in_addr_t)(-1), and not assume there's any macro provided for that.
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6.
regards, tom lane
On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think what this shows is we should look for a way to avoid using
INADDR_NONE.From some more googling
(http://www.opengroup.org/onlinepubs/000095399/functions/inet_addr.html),
it says it will return (in_addr_t)(-1), though, so maybe we should
just move that #ifdef out to some global place?Given the way that's written, I think we should just compare the result
to (in_addr_t)(-1), and not assume there's any macro provided for that.
Well, that doesn't match all other platforms..
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6.
"Path of least resistance?"
Which method would you suggest?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6.
"Path of least resistance?"
Which method would you suggest?
I haven't actually read the RADIUS patch, but generally we rely on
pg_getaddrinfo_all to interpret strings representing IP addresses.
Is there a reason not to use that?
regards, tom lane
On Thu, Jan 28, 2010 at 21:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6."Path of least resistance?"
Which method would you suggest?
I haven't actually read the RADIUS patch, but generally we rely on
pg_getaddrinfo_all to interpret strings representing IP addresses.
Is there a reason not to use that?
I don't think so. I'll look it over.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
2010/1/28 Magnus Hagander <magnus@hagander.net>:
On Thu, Jan 28, 2010 at 21:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6."Path of least resistance?"
Which method would you suggest?
I haven't actually read the RADIUS patch, but generally we rely on
pg_getaddrinfo_all to interpret strings representing IP addresses.
Is there a reason not to use that?I don't think so. I'll look it over.
Here's what I came up with. Works well on the platforms I've tried,
but I haven't tried on a non-ipv6 capable one yet (need to find one..)
I'll also remove the defines from solaris.h when applying it.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Attachments:
radius_addr.patchapplication/octet-stream; name=radius_addr.patchDownload+59-15
Magnus Hagander <magnus@hagander.net> writes:
Here's what I came up with. Works well on the platforms I've tried,
but I haven't tried on a non-ipv6 capable one yet (need to find one..)
Hmm, well, I have an ipv6-ignorant HPUX box at hand. I do not have a
radius server though. Are you only concerned about whether it compiles,
or do you want actual testing?
regards, tom lane
2010/2/1 Magnus Hagander <magnus@hagander.net>:
2010/1/28 Magnus Hagander <magnus@hagander.net>:
On Thu, Jan 28, 2010 at 21:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
However, now that I know the real issue is you're using inet_addr, I
would like to know why you're not using inet_aton instead; or even
better, something that also copes with IPv6."Path of least resistance?"
Which method would you suggest?
I haven't actually read the RADIUS patch, but generally we rely on
pg_getaddrinfo_all to interpret strings representing IP addresses.
Is there a reason not to use that?I don't think so. I'll look it over.
Here's what I came up with. Works well on the platforms I've tried,
but I haven't tried on a non-ipv6 capable one yet (need to find one..)
I'll also remove the defines from solaris.h when applying it.
Applied with some adjustments needed for non-ipv6 platforms.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/