Fix hashing bug

lib/dns/Makefile.am

@@ -78,6 +78,7 @@ libdns_la_HEADERS =			\
 	include/dns/geoip.h		\
 	include/dns/ipkeylist.h		\
 	include/dns/iptable.h		\
+	include/dns/lhashmap.h		\
 	include/dns/journal.h		\
 	include/dns/kasp.h		\
 	include/dns/keydata.h		\
@@ -185,6 +186,7 @@ libdns_la_SOURCES =			\
 	hmac_link.c			\
 	ipkeylist.c			\
 	iptable.c			\
+	lhashmap.c			\
 	journal.c			\
 	kasp.c				\
 	key.c				\

lib/dns/include/dns/lhashmap.h

@@ -0,0 +1,30 @@
+#pragma once
+
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <string.h>
+#include <isc/result.h>
+
+typedef struct {
+    size_t hash;
+    char data[];  // flexible array member
+} isc_lhashmap_entry_t;
+
+typedef uint64_t (*hash_func_t)(const void *);
+typedef bool (*match_func_t)(const void *, const void *);
+
+typedef struct {
+    char* array;
+    size_t size;
+    size_t elem_size;
+    hash_func_t hash_func;
+    match_func_t match_func;
+} isc_lhashmap_t;
+
+bool isc_lhashmap_entry_is_empty(isc_lhashmap_entry_t* entry);
+void isc_lhashmap_entry_put_data(isc_lhashmap_t *map, isc_lhashmap_entry_t* entry, void* elem);
+void* isc_lhashmap_entry_get_data(isc_lhashmap_t *map, isc_lhashmap_entry_t* entry, void* elem);
+isc_lhashmap_t isc_lhashmap_init(size_t size, size_t elem_size, char* array, hash_func_t hash_func, match_func_t match_func);
+isc_lhashmap_entry_t* isc_lhashmap_entry(const isc_lhashmap_t* map, void* elem);
+isc_result_t isc_lhashmap_put(isc_lhashmap_t* map, void* elem);

lib/dns/include/dns/name.h

@@ -117,7 +117,6 @@ struct dns_name {
 	isc_buffer_t  *buffer;
 	ISC_LINK(dns_name_t) link;
 	ISC_LIST(dns_rdataset_t) list;
-	isc_hashmap_t *hashmap;
 };
 
 #define DNS_NAME_MAGIC	  ISC_MAGIC('D', 'N', 'S', 'n')

lib/dns/lhashmap.c

@@ -0,0 +1,105 @@
+// #pragma once
+
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <math.h>
+#include <isc/result.h>
+#include <isc/util.h>
+
+#include <dns/lhashmap.h>
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-function"
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+
+static inline size_t 
+inc_and_saturate(size_t hash) {
+    size_t result;
+    if (__builtin_add_overflow(hash, 1, &result)) {
+        return SIZE_MAX;
+    }
+    return result;
+}
+
+bool
+isc_lhashmap_entry_is_empty(isc_lhashmap_entry_t* entry) {
+    return entry->hash == 0ul;
+}
+
+void
+isc_lhashmap_entry_put_data(isc_lhashmap_t *map, isc_lhashmap_entry_t* entry, void* elem) {
+    size_t elem_hash = map->hash_func(elem);
+    entry->hash = inc_and_saturate(elem_hash);
+    memcpy(entry->data, elem, map->elem_size);
+}
+
+void*
+isc_lhashmap_entry_get_data(isc_lhashmap_t *map, isc_lhashmap_entry_t* entry, void* elem) {
+    REQUIRE(entry != NULL);
+    return entry->data;
+}
+
+
+// Helper function to get entry at a specific index
+static isc_lhashmap_entry_t* 
+lhashmap_entry_pointer(const isc_lhashmap_t* map, size_t elem_hash, size_t offset) {
+    size_t index = (elem_hash + offset) % map->size;
+    return (isc_lhashmap_entry_t*)(map->array + index * (sizeof(size_t) + map->elem_size));
+}
+
+isc_lhashmap_t 
+isc_lhashmap_init(size_t size, size_t elem_size, char* array, hash_func_t hash_func, match_func_t match_func) {
+    REQUIRE(size > 0); // TODO is this precondition even necessary?
+    
+    isc_lhashmap_t map;
+    
+    // Calculate actual size to satisfy load factor > 1.3
+    size_t actual_size = ceil(size / 0.7);
+    
+    // Ensure size is at least the original size and doesn't exceed max
+    if (actual_size < size) {
+        actual_size = size;
+    }
+    
+    map.size = actual_size;
+    map.elem_size = elem_size;
+    map.array = array;
+    map.hash_func = hash_func;
+    map.match_func = match_func;
+    
+    // Zero out the entire hashmap
+    memset(array, 0, actual_size * (sizeof(size_t) + elem_size));
+    
+    return map;
+}
+
+isc_lhashmap_entry_t* 
+isc_lhashmap_entry(const isc_lhashmap_t* map, void* elem) {
+    size_t elem_hash = map->hash_func(elem);
+    size_t saturated_hash = inc_and_saturate(elem_hash);
+    for (size_t i = 0; i < map->size; i++) {
+        isc_lhashmap_entry_t* entry = lhashmap_entry_pointer(map, elem_hash, i);
+        
+        if (entry->hash == saturated_hash && map->match_func(entry->data, elem)) {
+            return entry;
+        } else if (entry->hash == 0ul) {
+            return entry;
+        }
+    }
+    return NULL;
+}
+
+isc_result_t
+isc_lhashmap_put(isc_lhashmap_t* map, void* elem) {
+    isc_lhashmap_entry_t *entry = isc_lhashmap_entry(map, elem);
+    if (entry) {
+        isc_lhashmap_entry_put_data(map, entry, elem);
+        return ISC_R_SUCCESS;
+    }
+    return ISC_R_FAILURE;
+}
+
+#pragma GCC diagnostic pop

lib/dns/message.c

@@ -49,6 +49,8 @@
 #include <dns/ttl.h>
 #include <dns/view.h>
 
+#include <dns/lhashmap.h>
+
 #ifdef SKAN_MSG_DEBUG
 static void
 hexdump(const char *msg, const char *msg2, void *base, size_t len) {
@@ -786,11 +788,6 @@ ISC_REFCOUNT_TRACE_IMPL(dns_message, dns__message_destroy);
 ISC_REFCOUNT_IMPL(dns_message, dns__message_destroy);
 #endif
 
-static bool
-name_match(void *node, const void *key) {
-	return dns_name_equal(node, key);
-}
-
 static isc_result_t
 findname(dns_name_t **foundname, const dns_name_t *target,
 	 dns_namelist_t *section) {
@@ -808,27 +805,6 @@ findname(dns_name_t **foundname, const dns_name_t *target,
 	return ISC_R_NOTFOUND;
 }
 
-static uint32_t
-rds_hash(dns_rdataset_t *rds) {
-	isc_hash32_t state;
-
-	isc_hash32_init(&state);
-	isc_hash32_hash(&state, &rds->rdclass, sizeof(rds->rdclass), true);
-	isc_hash32_hash(&state, &rds->type, sizeof(rds->type), true);
-	isc_hash32_hash(&state, &rds->covers, sizeof(rds->covers), true);
-
-	return isc_hash32_finalize(&state);
-}
-
-static bool
-rds_match(void *node, const void *key0) {
-	const dns_rdataset_t *rds = node;
-	const dns_rdataset_t *key = key0;
-
-	return rds->rdclass == key->rdclass && rds->type == key->type &&
-	       rds->covers == key->covers;
-}
-
 isc_result_t
 dns_message_findtype(const dns_name_t *name, dns_rdatatype_t type,
 		     dns_rdatatype_t covers, dns_rdataset_t **rdatasetp) {
@@ -950,14 +926,74 @@ getrdata(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		}                            \
 	} while (0)
 
-static void
-cleanup_name_hashmaps(dns_namelist_t *section) {
-	dns_name_t *name = NULL;
-	ISC_LIST_FOREACH (*section, name, link) {
-		if (name->hashmap != NULL) {
-			isc_hashmap_destroy(&name->hashmap);
-		}
+static size_t 
+name_hash(const void *elem) {
+	const dns_name_t *as_name_ptr = *((dns_name_t**) elem);
+	return dns_name_hash(as_name_ptr); 
+}
+
+static bool 
+name_match(const void *lhs, const void *rhs) {
+	const dns_name_t *as_name_lhs = *((dns_name_t**) lhs);
+	const dns_name_t *as_name_rhs = *((dns_name_t**) rhs);
+	return dns_name_equal(as_name_lhs, as_name_rhs); 
+}
+
+typedef struct name_and_rdtype {
+	dns_name_t *name;
+	dns_rdataset_t *rdataset;
+} name_and_rdtype_t;
+
+static size_t
+name_and_rdtype_hash(const void *elem_as_void) {
+	name_and_rdtype_t *elem = (name_and_rdtype_t*) elem_as_void;
+	isc_hash64_t state;
+
+	isc_hash64_init(&state);
+	isc_hash64_hash(&state, elem->name->ndata, elem->name->length, false);
+	isc_hash64_hash(&state, &elem->rdataset->rdclass,
+			sizeof(elem->rdataset->rdclass), true);
+	isc_hash64_hash(&state, &elem->rdataset->type,
+			sizeof(elem->rdataset->type), true);
+	isc_hash64_hash(&state, &elem->rdataset->covers,
+			sizeof(elem->rdataset->covers), true);
+
+	return isc_hash64_finalize(&state);
+}
+
+
+static bool
+name_and_rdtype_match(const void *lhs_as_void, const void *rhs_as_void) {
+	const name_and_rdtype_t *lhs = ((name_and_rdtype_t*) lhs_as_void);
+	const name_and_rdtype_t *rhs = ((name_and_rdtype_t*) rhs_as_void);
+
+	return lhs->rdataset->rdclass == rhs->rdataset->rdclass &&
+	       lhs->rdataset->type == rhs->rdataset->type &&
+	       lhs->rdataset->covers == rhs->rdataset->covers &&
+	       dns_name_equal(lhs->name, rhs->name);
+}
+
+enum {
+	NAME_BUFFER_SIZE = 131072 * (sizeof(size_t) + sizeof(dns_name_t*)),
+	NAME_RDTYPE_BUFFER_SIZE = 131072 * (sizeof(size_t) + sizeof(name_and_rdtype_t)),
+};
+
+static char* 
+thread_local_name_buffer(void) {
+	static _Thread_local char *name_buffer = NULL;
+	if (name_buffer == NULL) {
+		name_buffer = mallocx(NAME_BUFFER_SIZE, 0);
 	}
+	return name_buffer;
+}
+
+static char* 
+thread_local_rd_buffer(void) {
+	static _Thread_local char *rd_buffer = NULL;
+	if (rd_buffer == NULL) {
+		rd_buffer = mallocx(NAME_RDTYPE_BUFFER_SIZE, 0);
+	}
+	return rd_buffer;
 }
 
 static isc_result_t
@@ -966,7 +1002,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	isc_region_t r;
 	unsigned int count;
 	dns_name_t *name = NULL;
-	dns_name_t *found_name = NULL;
 	dns_rdataset_t *rdataset = NULL;
 	dns_rdatalist_t *rdatalist = NULL;
 	isc_result_t result = ISC_R_SUCCESS;
@@ -976,14 +1011,22 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0);
 	bool seen_problem = false;
 	bool free_name = false;
-	bool free_hashmaps = false;
-	isc_hashmap_t *name_map = NULL;
 
-	if (msg->counts[DNS_SECTION_QUESTION] > 1) {
-		isc_hashmap_create(msg->mctx, 1, &name_map);
+	isc_lhashmap_t name_map;
+	isc_lhashmap_t name_rdtype_map;
+	
+	size_t qdcount = msg->counts[DNS_SECTION_QUESTION];
+	if (qdcount == 0) {
+		return ISC_R_SUCCESS;
+	} else if (qdcount >= 2) {
+		char* lhashmap_buffer = thread_local_name_buffer();
+		name_map = isc_lhashmap_init(msg->counts[DNS_SECTION_QUESTION], sizeof(dns_name_t*), lhashmap_buffer, name_hash, name_match);
+		
+		char* lhashmap_rd_buffer = thread_local_rd_buffer();
+		name_rdtype_map = isc_lhashmap_init(msg->counts[DNS_SECTION_QUESTION], sizeof(name_and_rdtype_t), lhashmap_rd_buffer, name_and_rdtype_hash, name_and_rdtype_match);
 	}
 
-	for (count = 0; count < msg->counts[DNS_SECTION_QUESTION]; count++) {
+	for (count = 0; count < qdcount; count++) {
 		name = NULL;
 		dns_message_gettempname(msg, &name);
 		name->offsets = (unsigned char *)newoffsets(msg);
@@ -999,47 +1042,41 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			goto cleanup;
 		}
 
-		/* If there is only one QNAME, skip the duplicity checks */
-		if (name_map == NULL) {
-			result = ISC_R_SUCCESS;
-			goto skip_name_check;
-		}
+		if (qdcount >= 2) {
+			/*
+			 * Run through the section, looking to see if this name
+			 * is already there.  If it is found, put back the allocated
+			 * name since we no longer need it, and set our name pointer
+			 * to point to the name we found.
+			 */
+			isc_lhashmap_entry_t *entry = isc_lhashmap_entry(&name_map, &name);
 
-		/*
-		 * Run through the section, looking to see if this name
-		 * is already there.  If it is found, put back the allocated
-		 * name since we no longer need it, and set our name pointer
-		 * to point to the name we found.
-		 */
-		result = isc_hashmap_add(name_map, dns_name_hash(name),
-					 name_match, name, name,
-					 (void **)&found_name);
+			/*
+			 * If it is the first name in the section, accept it.
+			 *
+			 * If it is not, but is not the same as the name already
+			 * in the question section, append to the section.  Note that
+			 * here in the question section this is illegal, so return
+			 * FORMERR.  In the future, check the opcode to see if
+			 * this should be legal or not.  In either case we no longer
+			 * need this name pointer.
+			 */
+			// TODO this is just checking against the first name, we don't
+			// need a hashmap!
+			if (!isc_lhashmap_entry_is_empty(entry)) {
+				dns_message_puttempname(msg, &name);
 
-		/*
-		 * If it is the first name in the section, accept it.
-		 *
-		 * If it is not, but is not the same as the name already
-		 * in the question section, append to the section.  Note that
-		 * here in the question section this is illegal, so return
-		 * FORMERR.  In the future, check the opcode to see if
-		 * this should be legal or not.  In either case we no longer
-		 * need this name pointer.
-		 */
-	skip_name_check:
-		switch (result) {
-		case ISC_R_SUCCESS:
-			if (!ISC_LIST_EMPTY(*section)) {
-				DO_ERROR(DNS_R_FORMERR);
+				name = *((dns_name_t**) entry->data);
+			} else {
+				if (count > 0) {
+					DO_ERROR(DNS_R_FORMERR);
+				} else {
+					isc_lhashmap_entry_put_data(&name_map, entry, &name);
+					ISC_LIST_APPEND(*section, name, link);
+				}
 			}
+		} else {
 			ISC_LIST_APPEND(*section, name, link);
-			break;
-		case ISC_R_EXISTS:
-			dns_message_puttempname(msg, &name);
-			name = found_name;
-			found_name = NULL;
-			break;
-		default:
-			UNREACHABLE();
 		}
 
 		free_name = false;
@@ -1090,40 +1127,24 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 
 		rdataset->attributes |= DNS_RDATASETATTR_QUESTION;
 
-		/*
-		 * Skip the duplicity check for first rdataset
-		 */
-		if (ISC_LIST_EMPTY(name->list)) {
-			result = ISC_R_SUCCESS;
-			goto skip_rds_check;
+		if (qdcount >= 2) {
+			/*
+			 * Can't ask the same question twice.
+			 */
+			name_and_rdtype_t key = {
+				.name = name,
+				.rdataset = rdataset,
+			};
+			isc_lhashmap_entry_t *rdtype_entry = isc_lhashmap_entry(&name_rdtype_map, &key);
+			// TODO NULL check
+			if (!isc_lhashmap_entry_is_empty(rdtype_entry)) {
+				result = ISC_R_SUCCESS;
+				DO_ERROR(DNS_R_FORMERR);
+			} else {
+				isc_lhashmap_entry_put_data(&name_rdtype_map, rdtype_entry, &key);
+			}
 		}
 
-		/*
-		 * Can't ask the same question twice.
-		 */
-		if (name->hashmap == NULL) {
-			isc_hashmap_create(msg->mctx, 1, &name->hashmap);
-			free_hashmaps = true;
-
-			INSIST(ISC_LIST_HEAD(name->list) ==
-			       ISC_LIST_TAIL(name->list));
-
-			dns_rdataset_t *old_rdataset =
-				ISC_LIST_HEAD(name->list);
-
-			result = isc_hashmap_add(
-				name->hashmap, rds_hash(old_rdataset),
-				rds_match, old_rdataset, old_rdataset, NULL);
-
-			INSIST(result == ISC_R_SUCCESS);
-		}
-		result = isc_hashmap_add(name->hashmap, rds_hash(rdataset),
-					 rds_match, rdataset, rdataset, NULL);
-		if (result == ISC_R_EXISTS) {
-			DO_ERROR(DNS_R_FORMERR);
-		}
-
-	skip_rds_check:
 		ISC_LIST_APPEND(name->list, rdataset, link);
 
 		rdataset = NULL;
@@ -1146,14 +1167,6 @@ cleanup:
 		dns_message_puttempname(msg, &name);
 	}
 
-	if (free_hashmaps) {
-		cleanup_name_hashmaps(section);
-	}
-
-	if (name_map != NULL) {
-		isc_hashmap_destroy(&name_map);
-	}
-
 	return result;
 }
 
@@ -1226,7 +1239,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	isc_region_t r;
 	unsigned int count, rdatalen;
 	dns_name_t *name = NULL;
-	dns_name_t *found_name = NULL;
 	dns_rdataset_t *rdataset = NULL;
 	dns_rdataset_t *found_rdataset = NULL;
 	dns_rdatalist_t *rdatalist = NULL;
@@ -1237,17 +1249,25 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	dns_ttl_t ttl;
 	dns_namelist_t *section = &msg->sections[sectionid];
 	bool free_name = false, seen_problem = false;
-	bool free_hashmaps = false;
 	bool preserve_order = ((options & DNS_MESSAGEPARSE_PRESERVEORDER) != 0);
 	bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0);
 	bool isedns, issigzero, istsig;
-	isc_hashmap_t *name_map = NULL;
-
-	if (msg->counts[sectionid] > 1) {
-		isc_hashmap_create(msg->mctx, 1, &name_map);
+	
+	isc_lhashmap_t name_map;
+	isc_lhashmap_t name_rdtype_map;
+	
+	size_t section_count = msg->counts[sectionid];
+	if (section_count == 0) {
+		return ISC_R_SUCCESS;
+	} else if (section_count >= 2) {
+		char* lhashmap_buffer = thread_local_name_buffer();
+		name_map = isc_lhashmap_init(section_count, sizeof(dns_name_t*), lhashmap_buffer, name_hash, name_match);
+		
+		char* lhashmap_rd_buffer = thread_local_rd_buffer();
+		name_rdtype_map = isc_lhashmap_init(section_count, sizeof(name_and_rdtype_t), lhashmap_rd_buffer, name_and_rdtype_hash, name_and_rdtype_match);
 	}
 
-	for (count = 0; count < msg->counts[sectionid]; count++) {
+	for (count = 0; count < section_count; count++) {
 		int recstart = source->current;
 		bool skip_name_search, skip_type_search;
 
@@ -1483,43 +1503,27 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		 * to the end of the message.
 		 */
 		if (preserve_order || msg->opcode == dns_opcode_update ||
-		    skip_name_search)
+		    skip_name_search || section_count <= 1)
 		{
 			if (!isedns && !istsig && !issigzero) {
 				ISC_LIST_APPEND(*section, name, link);
 				free_name = false;
 			}
 		} else {
-			if (name_map == NULL) {
-				result = ISC_R_SUCCESS;
-				goto skip_name_check;
-			}
-
-			/*
-			 * Run through the section, looking to see if this name
-			 * is already there.  If it is found, put back the
-			 * allocated name since we no longer need it, and set
-			 * our name pointer to point to the name we found.
-			 */
-			result = isc_hashmap_add(name_map, dns_name_hash(name),
-						 name_match, name, name,
-						 (void **)&found_name);
+			isc_lhashmap_entry_t *entry = isc_lhashmap_entry(&name_map, &name);
+			// TODO NULL check
 
 			/*
 			 * If it is a new name, append to the section.
 			 */
-		skip_name_check:
-			switch (result) {
-			case ISC_R_SUCCESS:
-				ISC_LIST_APPEND(*section, name, link);
-				break;
-			case ISC_R_EXISTS:
+			if (!isc_lhashmap_entry_is_empty(entry)) {
 				dns_message_puttempname(msg, &name);
-				name = found_name;
-				found_name = NULL;
-				break;
-			default:
-				UNREACHABLE();
+				name = *((dns_name_t**) entry->data);
+				result = ISC_R_EXISTS;
+			} else {
+				isc_lhashmap_entry_put_data(&name_map, entry, &name);
+				ISC_LIST_APPEND(*section, name, link);
+				result = ISC_R_SUCCESS;
 			}
 			free_name = false;
 		}
@@ -1546,6 +1550,18 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		{
 			result = ISC_R_SUCCESS;
 
+			ISC_LIST_APPEND(name->list, rdataset, link);
+		} else if (section_count <= 1) {
+			/*
+			 * If this is a type that can only occur in
+			 * the question section, fail.
+			 */
+			if (dns_rdatatype_questiononly(rdtype)) {
+				DO_ERROR(DNS_R_FORMERR);
+			}
+			
+			result = ISC_R_SUCCESS;
+
 			ISC_LIST_APPEND(name->list, rdataset, link);
 		} else {
 			/*
@@ -1556,34 +1572,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 				DO_ERROR(DNS_R_FORMERR);
 			}
 
-			if (ISC_LIST_EMPTY(name->list)) {
-				result = ISC_R_SUCCESS;
-				goto skip_rds_check;
-			}
-
-			if (name->hashmap == NULL) {
-				isc_hashmap_create(msg->mctx, 1,
-						   &name->hashmap);
-				free_hashmaps = true;
-
-				INSIST(ISC_LIST_HEAD(name->list) ==
-				       ISC_LIST_TAIL(name->list));
-
-				dns_rdataset_t *old_rdataset =
-					ISC_LIST_HEAD(name->list);
-
-				result = isc_hashmap_add(
-					name->hashmap, rds_hash(old_rdataset),
-					rds_match, old_rdataset, old_rdataset,
-					NULL);
-
-				INSIST(result == ISC_R_SUCCESS);
-			}
-
-			result = isc_hashmap_add(
-				name->hashmap, rds_hash(rdataset), rds_match,
-				rdataset, rdataset, (void **)&found_rdataset);
-
 			/*
 			 * If we found an rdataset that matches, we need to
 			 * append this rdata to that set.  If we did not, we
@@ -1597,33 +1585,35 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			 * order, the opcode is an update, or the type search is
 			 * skipped.
 			 */
-		skip_rds_check:
-			switch (result) {
-			case ISC_R_EXISTS:
+			name_and_rdtype_t key = {
+				.name = name,
+				.rdataset = rdataset,
+			};
+			isc_lhashmap_entry_t *rdtype_entry = isc_lhashmap_entry(&name_rdtype_map, &key);
+			// TODO NULL check
+			if (!isc_lhashmap_entry_is_empty(rdtype_entry)) {
 				/* Free the rdataset we used as the key */
 				dns__message_putassociatedrdataset(msg,
 								   &rdataset);
 				result = ISC_R_SUCCESS;
-				rdataset = found_rdataset;
+				rdataset = ((name_and_rdtype_t*) isc_lhashmap_entry_get_data(&name_rdtype_map, rdtype_entry, &key))->rdataset;
+				found_rdataset = rdataset;
+				REQUIRE(DNS_RDATASET_VALID(rdataset));
 
-				if (!dns_rdatatype_issingleton(rdtype)) {
-					break;
+				if (dns_rdatatype_issingleton(rdtype)) {
+					dns_rdatalist_fromrdataset(rdataset,
+								   &rdatalist);
+					dns_rdata_t *first =
+						ISC_LIST_HEAD(rdatalist->rdata);
+					INSIST(first != NULL);
+					if (dns_rdata_compare(rdata, first) != 0) {
+						DO_ERROR(DNS_R_FORMERR);
+					}
 				}
-
-				dns_rdatalist_fromrdataset(rdataset,
-							   &rdatalist);
-				dns_rdata_t *first =
-					ISC_LIST_HEAD(rdatalist->rdata);
-				INSIST(first != NULL);
-				if (dns_rdata_compare(rdata, first) != 0) {
-					DO_ERROR(DNS_R_FORMERR);
-				}
-				break;
-			case ISC_R_SUCCESS:
+			} else {
+				result = ISC_R_SUCCESS;
+				isc_lhashmap_entry_put_data(&name_rdtype_map, rdtype_entry, &key);
 				ISC_LIST_APPEND(name->list, rdataset, link);
-				break;
-			default:
-				UNREACHABLE();
 			}
 		}
 
@@ -1718,14 +1708,6 @@ cleanup:
 		dns_message_puttempname(msg, &name);
 	}
 
-	if (free_hashmaps) {
-		cleanup_name_hashmaps(section);
-	}
-
-	if (name_map != NULL) {
-		isc_hashmap_destroy(&name_map);
-	}
-
 	return result;
 }
 
@@ -2639,10 +2621,6 @@ dns_message_puttempname(dns_message_t *msg, dns_name_t **itemp) {
 	REQUIRE(!ISC_LINK_LINKED(item, link));
 	REQUIRE(ISC_LIST_HEAD(item->list) == NULL);
 
-	if (item->hashmap != NULL) {
-		isc_hashmap_destroy(&item->hashmap);
-	}
-
 	/*
 	 * we need to check this in case dns_name_dup() was used.
 	 */