Thread: pgsql: Define INADDR_NONE on Solaris when it's missing.
pgsql: Define INADDR_NONE on Solaris when it's missing.
From
mha@postgresql.org (Magnus Hagander)
Date:
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/