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(at)postgresql(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> mha(at)postgresql(dot)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(at)hagander(dot)net> writes:
> On Thu, Jan 28, 2010 at 16:46, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Thu, Jan 28, 2010 at 16:46, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)hagander(dot)net> writes:
> On Thu, Jan 28, 2010 at 17:04, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Thu, Jan 28, 2010 at 17:04, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)hagander(dot)net> writes:
> On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)hagander(dot)net>:
> On Thu, Jan 28, 2010 at 21:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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/

Magnus Hagander <magnus(at)hagander(dot)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(at)hagander(dot)net>:
> 2010/1/28 Magnus Hagander <magnus(at)hagander(dot)net>:
>> On Thu, Jan 28, 2010 at 21:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>>> On Thu, Jan 28, 2010 at 17:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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/