CC: "Tony Finch" <dot@dotat.at>
Date: Wed, 21 Jun 2017 18:41:29 +0100
Message-ID: <alpine.DEB.2.11.1706211840040.21595@grey.csi.cam.ac.uk>
To: bind9-bugs@isc.org
From: "Tony Finch" <dot@dotat.at>
content-type: TEXT/PLAIN; charset="utf-8"
User-Agent: Alpine 2.11 (DEB 23 2013-08-11)
MIME-Version: 1.0
Subject: [PATCH] Deprecate dns_name_hashbylabel() since it isn't a very good hash.
From dot@dotat.at  Wed Jun 21 17:41:33 2017
X-RT-Incoming-Encryption: Not encrypted
X-RT-Original-Encoding: ascii
X-RT-Interface: Email
Content-Length: 3066

The rbt stopped using it in 2004 (change 1740) and it was
not used after that point until RRL started using it in 2013.

I have left the entry points in place for backwards compatibility,
though the hash values will be different.
---
 lib/dns/include/dns/name.h | 16 ----------------
 lib/dns/name.c             | 34 ++++------------------------------
 lib/dns/rrl.c              |  4 ++--
 3 files changed, 6 insertions(+), 48 deletions(-)

diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h
index ab76458..346188e 100644
--- a/lib/dns/include/dns/name.h
+++ b/lib/dns/include/dns/name.h
@@ -349,22 +349,6 @@ dns_name_fullhash(const dns_name_t *name, isc_boolean_t case_sensitive);
  *\li	A hash value
  */

-unsigned int
-dns_name_hashbylabel(const dns_name_t *name, isc_boolean_t case_sensitive);
-/*%<
- * Provide a hash value for 'name', where the hash value is the sum
- * of the hash values of each label.
- *
- * Note: if 'case_sensitive' is ISC_FALSE, then names which differ only in
- * case will have the same hash value.
- *
- * Requires:
- *\li	'name' is a valid name
- *
- * Returns:
- *\li	A hash value
- */
-
 /*
  *** Comparisons
  ***/
diff --git a/lib/dns/name.c b/lib/dns/name.c
index d07b5fc..b41740b 100644
--- a/lib/dns/name.c
+++ b/lib/dns/name.c
@@ -498,38 +498,12 @@ dns_fullname_hash(const dns_name_t *name, isc_boolean_t case_sensitive) {

 unsigned int
 dns_name_hashbylabel(const dns_name_t *name, isc_boolean_t case_sensitive) {
-	unsigned char *offsets;
-	dns_offsets_t odata;
-	dns_name_t tname;
-	unsigned int h = 0;
-	unsigned int i;
-
 	/*
-	 * Provide a hash value for 'name'.
+	 * This function was deprecated because of its poor performance.
+	 * We only keep this internally to provide binary backward
+	 * compatibility.
 	 */
-	REQUIRE(VALID_NAME(name));
-
-	if (name->labels == 0)
-		return (0);
-	else if (name->labels == 1)
-		return (isc_hash_function_reverse(name->ndata, name->length,
-						  case_sensitive, NULL));
-
-	SETUP_OFFSETS(name, offsets, odata);
-	DNS_NAME_INIT(&tname, NULL);
-	tname.labels = 1;
-	h = 0;
-	for (i = 0; i < name->labels; i++) {
-		tname.ndata = name->ndata + offsets[i];
-		if (i == name->labels - 1)
-			tname.length = name->length - offsets[i];
-		else
-			tname.length = offsets[i + 1] - offsets[i];
-		h += isc_hash_function_reverse(tname.ndata, tname.length,
-					       case_sensitive, NULL);
-	}
-
-	return (h);
+	return (dns_name_fullhash(name, case_sensitive));
 }

 dns_namereln_t
diff --git a/lib/dns/rrl.c b/lib/dns/rrl.c
index 252ebd8..53a86d3 100644
--- a/lib/dns/rrl.c
+++ b/lib/dns/rrl.c
@@ -417,10 +417,10 @@ make_key(const dns_rrl_t *rrl, dns_rrl_key_t *key,
 		{
 			dns_name_init(&base, base_offsets);
 			dns_name_getlabelsequence(qname, 1, labels-1, &base);
-			key->s.qname_hash = dns_name_hashbylabel(&base,
+			key->s.qname_hash = dns_name_fullhash(&base,
 							ISC_FALSE);
 		} else {
-			key->s.qname_hash = dns_name_hashbylabel(qname,
+			key->s.qname_hash = dns_name_fullhash(qname,
 							ISC_FALSE);
 		}
 	}
-- 
2.10.1.445.g3cdd5d1