Message ID | 20140627173501.GA12370@spoyarek.pnq.redhat.com |
---|---|
State | New |
Headers | show |
The status quo would seem to suggest that those two files are out of synch. It would be good for someone to grok what the code really does before we act.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/27/2014 01:35 PM, Siddhesh Poyarekar wrote: > The macro has been unconditionally defined as 1 in gethnamaddr.c and > not defined at all in dns-host.c. As a result, the code excluded by > the macro has been dead for a while (since 1995 for gethnamaddr.c). > Removing the excluded bits do not cause any change to the generated > code on x86_64. Looks OK. > Siddhesh > > * resolv/gethnamaddr.c: Remove definition of > MULTI_PTRS_ARE_ALIASES. > (getanswer) [!MULTI_PTRS_ARE_ALIASES]: Remove code. > * resolv/nss_dns/dns-host.c (getanswer_r) > [MULTI_PTRS_ARE_ALIASES]: Likewise. OK, but there could be a bug here. > --- > resolv/gethnamaddr.c | 18 ------------------ > resolv/nss_dns/dns-host.c | 22 ---------------------- > 2 files changed, 40 deletions(-) > > diff --git a/resolv/gethnamaddr.c b/resolv/gethnamaddr.c > index c73a0dc..9bbed59 100644 > --- a/resolv/gethnamaddr.c > +++ b/resolv/gethnamaddr.c > @@ -73,8 +73,6 @@ static char sccsid[] = "@(#)gethostnamadr.c 8.1 (Berkeley) 6/4/93"; > # define LOG_AUTH 0 > #endif > > -#define MULTI_PTRS_ARE_ALIASES 1 /* XXX - experimental */ > - OK. > #if defined(BSD) && (BSD >= 199103) && defined(AF_INET6) > # include <stdlib.h> > # include <string.h> > @@ -359,7 +357,6 @@ getanswer (const querybuf *answer, int anslen, const char *qname, int qtype) > had_error++; > break; > } > -#if MULTI_PTRS_ARE_ALIASES > cp += n; > if (cp != erdata) { > __set_h_errno (NO_RECOVERY); > @@ -381,21 +378,6 @@ getanswer (const querybuf *answer, int anslen, const char *qname, int qtype) > buflen -= n; > } > break; > -#else > - host.h_name = bp; > - if (_res.options & RES_USE_INET6) { > - n = strlen(bp) + 1; /* for the \0 */ > - if (n >= MAXHOSTNAMELEN) { > - had_error++; > - break; > - } > - bp += n; > - buflen -= n; > - map_v4v6_hostent(&host, &bp, &buflen); > - } > - __set_h_errno (NETDB_SUCCESS); > - return (&host); > -#endif OK. > case T_A: > case T_AAAA: > if (strcasecmp(host.h_name, bp) != 0) { > diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c > index a5f2c0a..258618e 100644 > --- a/resolv/nss_dns/dns-host.c > +++ b/resolv/nss_dns/dns-host.c > @@ -869,27 +869,6 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, > ++had_error; > break; > } > -#if MULTI_PTRS_ARE_ALIASES > - cp += n; > - if (haveanswer == 0) > - result->h_name = bp; > - else if (ap < &host_data->aliases[MAXALIASES-1]) > - *ap++ = bp; > - else > - n = -1; > - if (n != -1) > - { > - n = strlen (bp) + 1; /* for the \0 */ > - if (__builtin_expect (n, 0) >= MAXHOSTNAMELEN) > - { > - ++had_error; > - break; > - } > - bp += n; > - linebuflen -= n; > - } > - break; > -#else OK because it's been this way so long. Do we have a bug here? If the DNS server has multiple PTR records do we return them all? I think we should since DNS doesn't make it invalid. > result->h_name = bp; > if (have_to_map) > { > @@ -906,7 +885,6 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, > } > *h_errnop = NETDB_SUCCESS; > return NSS_STATUS_SUCCESS; > -#endif > case T_A: > case T_AAAA: > if (__builtin_expect (strcasecmp (result->h_name, bp), 0) != 0) > Cheers, Carlos. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTrbX9AAoJECXvCkNsKkr//QQIAMIx4AN4SoWCEdQ+oeubSFgT 7f8G9wq7EJaJX06aN4gAMvt7NGl2UUS56Clz2e+LjW8P6uaVJzcm2PiX/lVZh34Z eahvioxKqvVlkpE1DENHco5vi/bua4i+MVUcFn82ZqKL6SgEgvqPUWo/ZIRSJLBe l+0uDf3f6wHvpLgKzFKfULIiLBXZ+KgRZN2yptnKbDD/68URfOej5t0uvxt1WkGL Qt1T+AifU/y6TKZbd8axftMYZmb9mIx68wihB4bwmnJzCPOtdSmIv12XQbdfRctK 2u4/U5XdcZ8hmhiIX54VYyxq2RV+Vmayd0UGXlyaiOTEwtQ7EW9krDakqUACpuw= =Clsj -----END PGP SIGNATURE-----
On Fri, Jun 27, 2014 at 02:20:45PM -0400, Carlos O'Donell wrote: > OK because it's been this way so long. > Not committing this yet since Roland also raised doubts about the inconsistency. Lets try to get that resolved first. > Do we have a bug here? > > If the DNS server has multiple PTR records do we return them all? > > I think we should since DNS doesn't make it invalid. It is inconsistent behaviour between getnameinfo and gethostbyaddr, but I dont know if we ought to be fixing it now, which is why I went with retaining old behaviour instead. Why would multiple PTR records be useful for getnameinfo (or gethostbyaddr for that matter)? getnameinfo is capable of returning only one name, so running through an entire list seems useless. A regular lookup won't expect a PTR record AFAICT. So I would think that getnameinfo behaviour is not incorrect and gethostbyaddr behaviour (it being a legacy function) be left alone. Siddhesh
I guess for now the inconsistency just makes me want to have the other code forks around as documentation. I suppose comments would do it just as well. Still my inclination is to make these something like: /* Insert long new comment about the weirdness and referring to the other file where the opposite fork is used in equivalent code. */ #if 0 /* was MULTI_PTRS_ARE_ALIASES */
diff --git a/resolv/gethnamaddr.c b/resolv/gethnamaddr.c index c73a0dc..9bbed59 100644 --- a/resolv/gethnamaddr.c +++ b/resolv/gethnamaddr.c @@ -73,8 +73,6 @@ static char sccsid[] = "@(#)gethostnamadr.c 8.1 (Berkeley) 6/4/93"; # define LOG_AUTH 0 #endif -#define MULTI_PTRS_ARE_ALIASES 1 /* XXX - experimental */ - #if defined(BSD) && (BSD >= 199103) && defined(AF_INET6) # include <stdlib.h> # include <string.h> @@ -359,7 +357,6 @@ getanswer (const querybuf *answer, int anslen, const char *qname, int qtype) had_error++; break; } -#if MULTI_PTRS_ARE_ALIASES cp += n; if (cp != erdata) { __set_h_errno (NO_RECOVERY); @@ -381,21 +378,6 @@ getanswer (const querybuf *answer, int anslen, const char *qname, int qtype) buflen -= n; } break; -#else - host.h_name = bp; - if (_res.options & RES_USE_INET6) { - n = strlen(bp) + 1; /* for the \0 */ - if (n >= MAXHOSTNAMELEN) { - had_error++; - break; - } - bp += n; - buflen -= n; - map_v4v6_hostent(&host, &bp, &buflen); - } - __set_h_errno (NETDB_SUCCESS); - return (&host); -#endif case T_A: case T_AAAA: if (strcasecmp(host.h_name, bp) != 0) { diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c index a5f2c0a..258618e 100644 --- a/resolv/nss_dns/dns-host.c +++ b/resolv/nss_dns/dns-host.c @@ -869,27 +869,6 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, ++had_error; break; } -#if MULTI_PTRS_ARE_ALIASES - cp += n; - if (haveanswer == 0) - result->h_name = bp; - else if (ap < &host_data->aliases[MAXALIASES-1]) - *ap++ = bp; - else - n = -1; - if (n != -1) - { - n = strlen (bp) + 1; /* for the \0 */ - if (__builtin_expect (n, 0) >= MAXHOSTNAMELEN) - { - ++had_error; - break; - } - bp += n; - linebuflen -= n; - } - break; -#else result->h_name = bp; if (have_to_map) { @@ -906,7 +885,6 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, } *h_errnop = NETDB_SUCCESS; return NSS_STATUS_SUCCESS; -#endif case T_A: case T_AAAA: if (__builtin_expect (strcasecmp (result->h_name, bp), 0) != 0)