Thread: pgsql-server/ /configure /configure.in rc/Make ...

CVSROOT:    /cvsroot
Module name:    pgsql-server
Changes by:    petere@postgresql.org    03/03/29 06:31:52

Modified files:
    .              : configure configure.in
    src            : Makefile.global.in
    src/backend/libpq: ip.c pqcomm.c
    src/include    : pg_config.h.in
    src/include/libpq: ip.h
    src/interfaces/libpq: Makefile fe-connect.c
Added files:
    src/include    : getaddrinfo.h
    src/port       : getaddrinfo.c

Log message:
    Simplify the socket handling code by supplying a replacement getaddrinfo()
    function if the OS doesn't provide one.

petere@postgresql.org (Peter Eisentraut - PostgreSQL) writes:
>     Simplify the socket handling code by supplying a replacement getaddrinfo()
>     function if the OS doesn't provide one.

Hate to tell you, but this commit has *completely* broken things on
non-IPv6-aware machines ...

            regards, tom lane

I said:
> Hate to tell you, but this commit has *completely* broken things on
> non-IPv6-aware machines ...

... or at least on my machine.  The major difficulty seems to be that
the config setup assumes HAVE_GETADDRINFO means that <netdb.h> is where
to look for 'struct addrinfo' and friends.  This does not fly here.
I have HPUX 10.20, which is not ipv6 aware, but I also have a (plain
vanilla) built-from-source installation of BIND 9.2.2.  This provides
a /usr/lib/libbind.a that contains getaddrinfo() ... but it doesn't
overwrite the stock /usr/include/netdb.h.

There are definitions for the struct and so forth in
/usr/include/lwres/netdb.h, but it looks like to use those we'd also
have to use liblwres.a rather than libbind.a.

Not sure what's the most reasonable way to fix this.  Personally I'd
like to remove the use of -lbind for other reasons, but I suppose that'd
break things on other platforms.

Can we at least change the configure tests so that both 'struct
addrinfo' and 'getaddrinfo()' are checked for before deciding we don't
need our own implementation?  I'd also suggest doing some macro-renaming
so that our version of getaddrinfo is really called pq_getaddrinfo, or
some such, to avoid any possible link conflicts.

            regards, tom lane

Tom Lane writes:

> Not sure what's the most reasonable way to fix this.  Personally I'd
> like to remove the use of -lbind for other reasons, but I suppose that'd
> break things on other platforms.

Ugh, does this check succeed for you:

# BeOS:
AC_CHECK_LIB(bind, __inet_ntoa)

(If not, then where does -lbind come from?)

--
Peter Eisentraut   peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Not sure what's the most reasonable way to fix this.  Personally I'd
>> like to remove the use of -lbind for other reasons, but I suppose that'd
>> break things on other platforms.

> Ugh, does this check succeed for you:

> # BeOS:
> AC_CHECK_LIB(bind, __inet_ntoa)

Yeah, it does, since -lbind is getting into the link.

I'm just about ready to start testing a fix that separately tests
HAVE_STRUCT_ADDRINFO (the struct decl) and HAVE_GETADDRINFO (the
function).  If we don't find the struct, then we force HAVE_GETADDRINFO
off to use our own routines.  Also, the header does

#undef getaddrinfo
#define getaddrinfo pg_getaddrinfo

(likewise for freeaddrinfo, gai_strerror) so that there's no conflict
if getaddrinfo() does exist somewhere in the libraries.  Seem reasonable?

            regards, tom lane

I said:
> I'm just about ready to start testing a fix that separately tests
> HAVE_STRUCT_ADDRINFO (the struct decl) and HAVE_GETADDRINFO (the
> function).

I've got this committed; it seems to work on HPUX and Linux, at least.

There is a residual bit of ugliness that should be cleaned up one way or
the other, and that's the dependency on hstrerror() in getaddrinfo.c.
The scorecard AFAICT is:

1. hstrerror() doesn't exist in HPUX 10.20.

2. But it does exist in libbind.a from BIND 9.2.2.

3. It exists but seems to be deprecated in Linux --- the RHL 8 man page
   says

       The (obsolete) hstrerror() function takes an error number (typically
       h_errno) and returns the corresponding message string.

I set up code in getaddrinfo.c to do something reasonable (but, at the
moment, non-internationalized) if HAVE_HSTRERROR isn't defined.  But it
blew up on me completely in the HPUX+libbind environment --- what
happens is that configure finds hstrerror in libbind.a, so it defines
HAVE_HSTRERROR, so getaddrinfo.c tries to use that (generating an
annoying "no previous prototype" warning), and then the link fails
anyhow!  The reason it fails is that libpgport is linked after libbind,
and since the latter is a static library the necessary module doesn't
get pulled from it.

(This suggests that -lpgport ought to be stuck on the front of LIBS,
not the back, in Makefile.global.  What do you think?)

What I've done for the moment is to not test for hstrerror in configure,
so that the non-HAVE_HSTRERROR path is always taken in getaddrinfo.c.

One way we could go from here is to institutionalize that approach.
I think that would require us to figure out how to do internationalization
of strings that appear in this file (which remember can be compiled in
multiple places).  If there's a reasonably clean way to do that, I'm
leaning to that approach.  The other possibility is to decide to rely on
hstrerror anyway, which will require fixing the link-order problem and
testing to see if we need to provide our own prototype for it.

Comments?

            regards, tom lane

Tom Lane writes:

> > # BeOS:
> > AC_CHECK_LIB(bind, __inet_ntoa)
>
> Yeah, it does, since -lbind is getting into the link.

I would prefer just disabling that check (and reworking it if someone
wants to resurrect the BeOS port).  If a system relies on bind libraries
then it's also the responsibility to set up the corresponding header files
correctly.

--
Peter Eisentraut   peter_e@gmx.net

Tom Lane writes:

> (This suggests that -lpgport ought to be stuck on the front of LIBS,
> not the back, in Makefile.global.  What do you think?)

Correct.

> What I've done for the moment is to not test for hstrerror in configure,
> so that the non-HAVE_HSTRERROR path is always taken in getaddrinfo.c.
>
> One way we could go from here is to institutionalize that approach.
> I think that would require us to figure out how to do internationalization
> of strings that appear in this file (which remember can be compiled in
> multiple places).  If there's a reasonably clean way to do that, I'm
> leaning to that approach.  The other possibility is to decide to rely on
> hstrerror anyway, which will require fixing the link-order problem and
> testing to see if we need to provide our own prototype for it.

I'm not too concerned about that right now.  Modern systems have
getaddrinfo() and semi-modern systems have hstrerror(), and the rest can
do with "error %d".  (I suppose the make-libpq-thread-safe movement will
have its own opinion on that.)

Eventually, the system's message localization facilities will have to be
bypassed, so the server can send different languages to log and clients.
So strerror()  and the like will have to be replaced anyway.

--
Peter Eisentraut   peter_e@gmx.net