[v2] Remove MULTI_PTRS_ARE_ALIASES to fix Wundef warning

Message ID 20140630055909.GN4477@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar June 30, 2014, 5:59 a.m. UTC
  On Fri, Jun 27, 2014 at 09:34:19PM -0700, Roland McGrath wrote:
> 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 */

After reading further, I don't think there is any inconsistency in
functionality.  The gethostbyaddr function also uses the dns-host.c
bits and gethnamaddr.c is currently not used at all.  The only
reference it has is in resolv/README:

    The files gethnamaddr.c, mapv4v6addr.h and mapv4v6hostent.h are
    leftovers from BIND 4.9.7.

Given this newly discovered fact, how about the following patch
instead?  I don't modify gethnamaddr.c because it's not even used.  I
wonder - given that we have decided to own the resolver bits now - if
we should just get rid of gethnamaddr.c and other unused files.  I
could add a note in the README mentioning this.

Siddhesh

	* resolv/nss_dns/dns-host.c (getanswer_r)
	[MULTI_PTRS_ARE_ALIASES]: Remove code.

commit 8e2ad63093a3c1e18122acdb50db5883770a3fc5
Author: Siddhesh Poyarekar <siddhesh@redhat.com>
Date:   Mon Jun 30 11:17:51 2014 +0530

    Remove MULTI_PTRS_ARE_ALIASES in dns-hosts.c
    
    The code in gethnamaddr.c for gethostbyaddr used and set this macro to
    allow multiple PTR records to be added as aliases.  This was useful
    for gethostbyaddr since it returns a hostent structure, which can
    return aliases.
    
    The gethnamaddr.c source however is unused in glibc since pretty much
    forever.  Instead, the DNS lookup bits for gethostbyaddr (as well as
    getnameinfo) are implemented in dns-hosts.c and in that implementation
    all but one (the first one) of the multiple PTR records are ignored.
    Since gethnamaddr.c is essentially dead code, ignore that
    implementation and replace the MULTI_PTRS_ARE_ALIASES bit with a
    comment mentioning that bind adds PTR records as aliases while we
    don't.
  

Comments

Carlos O'Donell June 30, 2014, 3:34 p.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/30/2014 01:59 AM, Siddhesh Poyarekar wrote:
> On Fri, Jun 27, 2014 at 09:34:19PM -0700, Roland McGrath wrote:
>> 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 */
> 
> After reading further, I don't think there is any inconsistency in
> functionality.  The gethostbyaddr function also uses the dns-host.c
> bits and gethnamaddr.c is currently not used at all.  The only
> reference it has is in resolv/README:
> 
>     The files gethnamaddr.c, mapv4v6addr.h and mapv4v6hostent.h are
>     leftovers from BIND 4.9.7.
> 
> Given this newly discovered fact, how about the following patch
> instead?  I don't modify gethnamaddr.c because it's not even used.  I
> wonder - given that we have decided to own the resolver bits now - if
> we should just get rid of gethnamaddr.c and other unused files.  I
> could add a note in the README mentioning this.
> 
> Siddhesh
> 
> 	* resolv/nss_dns/dns-host.c (getanswer_r)
> 	[MULTI_PTRS_ARE_ALIASES]: Remove code.

I'm OK with this v2, but if you're up for it I'd like to remove
gethnameaddr.c and update README. If we don't use the code it
should be immediately removed to prevent future confusion.

Cheers,
Carlos.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTsYN2AAoJECXvCkNsKkr/2CUH/jJjkYve8RysDt/bBRdjYqg0
tfeZ0J9Zi4RkvVaFF9dn0Q40d56bUFi729oQx08WLeNnFTOEc3Lh/ibulaQ/+K4K
SuzPzKQELllSXgQuxHoXNTjXKlP5Iyq+leiTa5rVcx+fkCSSubRhT3h6v+rDLh8N
Rqwcrtue9dCvJ9rZquD7ERIvh4ZKthZkeiIrvtyEaMVFmvieJ/V9EOMb7NsHGuYq
taRU+7tOwb8XxF0uyyzubVZ0scD/TbDtbX8FaLpcBS03eLmTyiMGWtcYbZmaGXlU
Wh/00/rj15ZgZPi3h+NMtJec15RoI6/PbCujp4EK+p4xD9+iinCl4uPAXwnWYms=
=TnzU
-----END PGP SIGNATURE-----
  

Patch

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index a5f2c0a..3258e70 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -869,27 +869,8 @@  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
+	  /* bind would put multiple PTR records as aliases, but we don't do
+	     that.  */
 	  result->h_name = bp;
 	  if (have_to_map)
 	    {
@@ -906,7 +887,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)