On Tue, 2012-08-14 at 18:52 -0400, Bruce Momjian wrote:
> I assume we didn't feel any action was necessary on this issue.
I propose the attached patch to reduce the redundant code as discussed.
>
> ---------------------------------------------------------------------------
>
> On Thu, Aug 11, 2011 at 01:50:02PM -0400, Tom Lane wrote:
> > Robert Haas <robertmh...@gmail.com> writes:
> > > On Tue, Aug 9, 2011 at 2:16 PM, Peter Eisentraut <pete...@gmx.net> wrote:
> > >> But I'm a little confused by what this code is really trying
> > >> to accomplish: ...
> >
> > > I think the intended behavior of NI_NUMERICHOST is to suppress the
> > > name lookup, and return the text format *even if* the name lookup
> > > would have worked. So the intention of this code may be to ensure
> > > that we convert the the sockaddr to some sort of string even if we
> > > can't resolve the hostname - i.e. if the first call fails, try again
> > > with NI_NUMERICHOST added to make sure that we didn't fail solely due
> > > to some kind of DNS hiccup. I suspect that at some time this was
> > > defending against an actual hazard but I don't know whether it's still
> > > a problem on modern operating systems.
> >
> > POSIX v7 says
> >
> > If the node's name cannot be located, the numeric form of the
> > address contained in the socket address structure pointed to by
> > the sa argument is returned instead of its name.
> >
> > If you read a bit further, apparently this is just supposed to be the
> > default behavior if neither NI_NUMERICHOST nor NI_NAMEREQD is set (in
> > the first case, it doesn't try to locate a node name, and in the second,
> > it fails if it can't locate a node name). But certainly the dance in
> > postmaster.c is not necessary if you believe the spec.
> >
> > I believe that the existing coding is meant to cope with the behavior of
> > our stub version of getnameinfo(), which simply fails outright if
> > NI_NUMERICHOST isn't set. However, if we just removed that test and
> > behaved as per spec (return a numeric address anyway), then we could
> > eliminate the second call in postmaster.c.
> >
> > >> The fix would appear to be using the NI_NAMEREQD flag to getnameinfo.
> >
> > > If you want to do that, you're going to have to fix the version of
> > > getnameinfo() in src/port/getaddrinfo.c, which apparently doesn't
> > > support that flag.
> >
> > Well, it's not just that it "doesn't support that flag". It's
> > fundamentally incapable of returning anything but a numeric address,
> > and therefore the only "support" it could offer would be to fail. So
> > that approach seems like a dead end.
> >
> > I don't really think that there's anything to fix here with respect to
> > Peter's original concern, but it might be worth getting rid of the
> > double call in postmaster.c.
> >
> > regards, tom lane
> >
> > --
> > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
>
> --
> Bruce Momjian <br...@momjian.us> http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + It's impossible for everything to be true. +
>
>
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index dff4c71..e73caa8 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3437,6 +3437,7 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
BackendInitialize(Port *port)
{
int status;
+ int ret;
char remote_host[NI_MAXHOST];
char remote_port[NI_MAXSERV];
char remote_ps_data[NI_MAXHOST];
@@ -3498,21 +3499,13 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
*/
remote_host[0] = '\0';
remote_port[0] = '\0';
- if (pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
+ if ((ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
remote_host, sizeof(remote_host),
remote_port, sizeof(remote_port),
- (log_hostname ? 0 : NI_NUMERICHOST) | NI_NUMERICSERV) != 0)
- {
- int ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
- remote_host, sizeof(remote_host),
- remote_port, sizeof(remote_port),
- NI_NUMERICHOST | NI_NUMERICSERV);
-
- if (ret != 0)
- ereport(WARNING,
- (errmsg_internal("pg_getnameinfo_all() failed: %s",
- gai_strerror(ret))));
- }
+ (log_hostname ? 0 : NI_NUMERICHOST) | NI_NUMERICSERV)) != 0)
+ ereport(WARNING,
+ (errmsg_internal("pg_getnameinfo_all() failed: %s",
+ gai_strerror(ret))));
if (remote_port[0] == '\0')
snprintf(remote_ps_data, sizeof(remote_ps_data), "%s", remote_host);
else
diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c
index c117012..749c35f 100644
--- a/src/port/getaddrinfo.c
+++ b/src/port/getaddrinfo.c
@@ -373,11 +373,6 @@ typedef int (__stdcall * getnameinfo_ptr_t) (const struct sockaddr * sa,
if (sa == NULL || (node == NULL && service == NULL))
return EAI_FAIL;
- /* We don't support those. */
- if ((node && !(flags & NI_NUMERICHOST))
- || (service && !(flags & NI_NUMERICSERV)))
- return EAI_FAIL;
-
#ifdef HAVE_IPV6
if (sa->sa_family == AF_INET6)
return EAI_FAMILY;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
