From: Ondřej Surý <ondrej@isc.org>
Date: Tue, 9 Sep 2025 09:32:17 +0000 (+0200)
Subject: Reduce the code duplication around getting slabheaders from slabtop
X-Git-Tag: v9.21.14~52^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4dc711ef861df3aef14d48e46008c5b5abb257e0;p=thirdparty%2Fbind9.git

Reduce the code duplication around getting slabheaders from slabtop

There was a lot of duplicated code around getting the first header that
exists, is active, and matches the version header from the qpzonedb.
Move the duplicate code into a helper function and unify the same
approach for the qpcache too even though the code is much simpler there.
It should come handy when top->header is something more complicated than
a pointer to first slabheader.
---

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 53be3b4d283..13bc5ecb279 100644
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -472,6 +472,20 @@ rdataset_size(dns_slabheader_t *header) {
 	return sizeof(*header);
 }
 
+static dns_slabheader_t *
+first_header(dns_slabtop_t *top) {
+	return top->header;
+}
+
+static dns_slabheader_t *
+first_existing_header(dns_slabtop_t *top) {
+	dns_slabheader_t *header = first_header(top);
+	if (EXISTS(top->header)) {
+		return header;
+	}
+	return NULL;
+}
+
 static void
 expire_lru_headers(qpcache_t *qpdb, uint32_t idx, size_t requested,
 		   isc_rwlocktype_t *nlocktypep,
@@ -487,7 +501,7 @@ expire_lru_headers(qpcache_t *qpdb, uint32_t idx, size_t requested,
 
 		ISC_SIEVE_UNLINK(qpdb->buckets[idx].sieve, top, link);
 
-		dns_slabheader_t *header = top->header;
+		dns_slabheader_t *header = first_header(top);
 
 		expired += rdataset_size(header);
 
@@ -540,14 +554,17 @@ qpcache_hit(qpcache_t *qpdb ISC_ATTR_UNUSED, dns_slabheader_t *header) {
  */
 
 static void
-clean_stale_headers(dns_slabheader_t *top) {
-	dns_slabheader_t *d = NULL, *down_next = NULL;
+clean_cache_headers(dns_slabtop_t *top) {
+	if (top->header == NULL) {
+		return;
+	}
 
-	for (d = top->down; d != NULL; d = down_next) {
-		down_next = d->down;
-		dns_slabheader_destroy(&d);
+	dns_slabheader_t *header = top->header, *header_down = NULL;
+	for (header = header->down; header != NULL; header = header_down) {
+		header_down = header->down;
+		dns_slabheader_destroy(&header);
 	}
-	top->down = NULL;
+	top->header->down = NULL;
 }
 
 static void
@@ -559,11 +576,11 @@ clean_cache_node(qpcache_t *qpdb, qpcnode_t *node) {
 	 */
 
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		clean_stale_headers(top->header);
+		clean_cache_headers(top);
 
 		/*
-		 * If current header is nonexistent, ancient, or stale and
-		 * we are not keeping stale, we can clean it up.
+		 * If current top header is nonexistent, ancient, or stale
+		 * and we are not keeping stale, we can clean it up too.
 		 */
 		if (!EXISTS(top->header) || ANCIENT(top->header) ||
 		    (STALE(top->header) && !KEEPSTALE(qpdb)))
@@ -572,7 +589,7 @@ clean_cache_node(qpcache_t *qpdb, qpcnode_t *node) {
 		}
 
 		/*
-		 * If current slabtype is empty, we can also clean it up.
+		 * If current slabtop is empty, we can clean it up.
 		 */
 		if (top->header == NULL) {
 			if (top_prev != NULL) {
@@ -1280,11 +1297,16 @@ check_zonecut(qpcnode_t *node, void *arg DNS__DB_FLARG) {
 	 * Look for a DNAME or RRSIG DNAME rdataset.
 	 */
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		if (check_stale_header(top->header, search)) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
+			continue;
+		}
+
+		if (check_stale_header(header, search)) {
 			continue;
 		}
 
-		if (both_headers(top->header, dns_rdatatype_dname, &found,
+		if (both_headers(header, dns_rdatatype_dname, &found,
 				 &foundsig))
 		{
 			break;
@@ -1342,11 +1364,16 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node,
 		 * Look for NS and RRSIG NS rdatasets.
 		 */
 		DNS_SLABTOP_FOREACH(top, node->data) {
-			if (check_stale_header(top->header, search)) {
+			dns_slabheader_t *header = first_header(top);
+			if (header == NULL) {
+				continue;
+			}
+
+			if (check_stale_header(header, search)) {
 				continue;
 			}
 
-			if (both_headers(top->header, dns_rdatatype_ns, &found,
+			if (both_headers(header, dns_rdatatype_ns, &found,
 					 &foundsig))
 			{
 				break;
@@ -1445,12 +1472,16 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name,
 	nlock = &search->qpdb->buckets[node->locknum].lock;
 	NODE_RDLOCK(nlock, &nlocktype);
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		if (check_stale_header(top->header, search)) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
 			continue;
 		}
 
-		if (both_headers(top->header, dns_rdatatype_nsec, &found,
-				 &foundsig))
+		if (check_stale_header(header, search)) {
+			continue;
+		}
+
+		if (both_headers(header, dns_rdatatype_nsec, &found, &foundsig))
 		{
 			break;
 		}
@@ -1645,11 +1676,16 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 	cnamesig = NULL;
 	empty_node = true;
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		if (check_stale_header(top->header, &search)) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
 			continue;
 		}
 
-		if (!EXISTS(top->header) || ANCIENT(top->header)) {
+		if (check_stale_header(header, &search)) {
+			continue;
+		}
+
+		if (!EXISTS(header) || ANCIENT(header)) {
 			continue;
 		}
 
@@ -1659,18 +1695,18 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 		 */
 		empty_node = false;
 
-		if (top->header->noqname != NULL &&
-		    top->header->trust == dns_trust_secure)
+		if (header->noqname != NULL &&
+		    header->trust == dns_trust_secure)
 		{
 			found_noqname = true;
 		}
 
-		if (!NEGATIVE(top->header)) {
+		if (!NEGATIVE(header)) {
 			all_negative = false;
 		}
 
 		bool match = false;
-		if (related_headers(top->header, typepair, sigpair, &found,
+		if (related_headers(header, typepair, sigpair, &found,
 				    &foundsig, &match) &&
 		    !MISSING_ANSWER(found, options))
 		{
@@ -1689,7 +1725,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 			continue;
 		}
 
-		if (NEGATIVE(top->header)) {
+		if (NEGATIVE(header)) {
 			/*
 			 * FIXME: As of now, we are not interested in
 			 * the negative headers.  This could be
@@ -1707,7 +1743,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 				break;
 			}
 
-			found = top->header;
+			found = header;
 			if (cnamesig != NULL) {
 				/* We already have CNAME signature */
 				foundsig = cnamesig;
@@ -1722,28 +1758,28 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 				break;
 			}
 
-			cnamesig = top->header;
+			cnamesig = header;
 			break;
 		case dns_rdatatype_ns:
 			/* Remember the NS rdataset */
-			nsheader = top->header;
+			nsheader = header;
 			break;
 		case DNS_SIGTYPEPAIR(dns_rdatatype_ns):
 			/* ...and its signature */
-			nssig = top->header;
+			nssig = header;
 			break;
 
 		case dns_rdatatype_nsec:
-			nsecheader = top->header;
+			nsecheader = header;
 			break;
 		case DNS_SIGTYPEPAIR(dns_rdatatype_nsec):
-			nsecsig = top->header;
+			nsecsig = header;
 			break;
 
 		default:
 			if (typepair == dns_typepair_any) {
 				/* QTYPE==ANY, so any anwers will do */
-				found = top->header;
+				found = header;
 				break;
 			}
 		}
@@ -1913,7 +1949,12 @@ seek_ns_headers(qpc_search_t *search, qpcnode_t *node, dns_dbnode_t **nodep,
 	DNS_SLABTOP_FOREACH(top, node->data) {
 		bool ns = top->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) ||
 			  top->typepair == DNS_SIGTYPEPAIR(dns_rdatatype_ns);
-		if (check_stale_header(top->header, search)) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
+			continue;
+		}
+
+		if (check_stale_header(header, search)) {
 			if (ns) {
 				/*
 				 * We found a cached NS, but was either
@@ -1928,9 +1969,7 @@ seek_ns_headers(qpc_search_t *search, qpcnode_t *node, dns_dbnode_t **nodep,
 			continue;
 		}
 
-		if (both_headers(top->header, dns_rdatatype_ns, &found,
-				 &foundsig))
-		{
+		if (both_headers(header, dns_rdatatype_ns, &found, &foundsig)) {
 			break;
 		}
 	}
@@ -2081,11 +2120,16 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 						: dns_typepair_none;
 
 	DNS_SLABTOP_FOREACH(top, qpnode->data) {
-		if (check_stale_header(top->header, &search)) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
 			continue;
 		}
 
-		if (related_headers(top->header, typepair, sigpair, &found,
+		if (check_stale_header(header, &search)) {
+			continue;
+		}
+
+		if (related_headers(header, typepair, sigpair, &found,
 				    &foundsig, NULL))
 		{
 			break;
@@ -2546,12 +2590,17 @@ expire_ncache_entry(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabtop_t *top,
 	    (sigpair != dns_rdatatype_none && newheader->typepair == sigpair &&
 	     DNS_TYPEPAIR_TYPE(top->typepair) == covers))
 	{
-		if (trust < top->header->trust) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
+			return DNS_R_CONTINUE;
+		}
+
+		if (trust < header->trust) {
 			/*
 			 * The NXDOMAIN/NODATA(QTYPE=ANY) is more trusted.
 			 */
-			qpcache_hit(qpdb, top->header);
-			bindrdataset(qpdb, qpnode, top->header, now, nlocktype,
+			qpcache_hit(qpdb, header);
+			bindrdataset(qpdb, qpnode, header, now, nlocktype,
 				     tlocktype,
 				     addedrdataset DNS__DB_FLARG_PASS);
 			return DNS_R_UNCHANGED;
@@ -2560,7 +2609,7 @@ expire_ncache_entry(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabtop_t *top,
 		/*
 		 * The new rdataset is better.  Expire the ncache entry.
 		 */
-		mark_ancient(top->header);
+		mark_ancient(header);
 		return DNS_R_CONTINUE;
 	}
 
@@ -2572,7 +2621,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
     unsigned int options, dns_rdataset_t *addedrdataset, isc_stdtime_t now,
     isc_rwlocktype_t nlocktype, isc_rwlocktype_t tlocktype DNS__DB_FLARG) {
 	dns_slabtop_t *priotop = NULL, *expiretop = NULL;
-	dns_slabheader_t *header = NULL, *sigheader = NULL;
+	dns_slabheader_t *oldheader = NULL, *oldsigheader = NULL;
 	dns_trust_t trust;
 	uint32_t ntypes = 0;
 	dns_rdatatype_t rdtype = DNS_TYPEPAIR_TYPE(newheader->typepair);
@@ -2607,6 +2656,11 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 	}
 
 	DNS_SLABTOP_FOREACH(top, qpnode->data) {
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
+			continue;
+		}
+
 		if (EXISTS(newheader) && NEGATIVE(newheader) &&
 		    rdtype == dns_rdatatype_any)
 		{
@@ -2618,7 +2672,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 			 * rdataset that can be found at this node is the
 			 * negative cache entry.
 			 */
-			mark_ancient(top->header);
+			mark_ancient(header);
 		}
 
 		if (EXISTS(newheader) && NEGATIVE(newheader) &&
@@ -2632,13 +2686,12 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 			if (DNS_TYPEPAIR_TYPE(top->typepair) ==
 			    dns_rdatatype_rrsig)
 			{
-				mark_ancient(top->header);
+				mark_ancient(header);
 			}
 		}
 
 		if (EXISTS(newheader) && !NEGATIVE(newheader) &&
-		    NEGATIVE(top->header) && EXISTS(top->header) &&
-		    ACTIVE(top->header, now))
+		    NEGATIVE(header) && EXISTS(header) && ACTIVE(header, now))
 		{
 			/*
 			 * Look for existing active NXDOMAIN or negative
@@ -2657,7 +2710,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 			INSIST(result == DNS_R_CONTINUE);
 		}
 
-		if (ACTIVE(top->header, now)) {
+		if (ACTIVE(header, now)) {
 			++ntypes;
 			expiretop = top;
 		}
@@ -2666,21 +2719,21 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		}
 
 		if (top->typepair == newheader->typepair) {
-			INSIST(header == NULL);
-			header = top->header;
+			INSIST(oldheader == NULL);
+			oldheader = top->header;
 		}
 
 		if (sigpair != dns_rdatatype_none && top->typepair == sigpair) {
-			INSIST(sigheader == NULL);
-			sigheader = top->header;
+			INSIST(oldsigheader == NULL);
+			oldsigheader = top->header;
 		}
 	}
 
-	if (header != NULL) {
+	if (oldheader != NULL) {
 		/*
 		 * Deleting an already non-existent rdataset has no effect.
 		 */
-		if (!EXISTS(header) && !EXISTS(newheader)) {
+		if (!EXISTS(oldheader) && !EXISTS(newheader)) {
 			return DNS_R_UNCHANGED;
 		}
 
@@ -2691,18 +2744,18 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		 * data will supersede it below. Unclear what the best
 		 * policy is here.
 		 */
-		if (trust < header->trust &&
-		    (ACTIVE(header, now) || !EXISTS(header)))
+		if (trust < oldheader->trust &&
+		    (ACTIVE(oldheader, now) || !EXISTS(oldheader)))
 		{
-			qpcache_hit(qpdb, header);
-			bindrdataset(qpdb, qpnode, header, now, nlocktype,
+			qpcache_hit(qpdb, oldheader);
+			bindrdataset(qpdb, qpnode, oldheader, now, nlocktype,
 				     tlocktype,
 				     addedrdataset DNS__DB_FLARG_PASS);
-			if (ACTIVE(header, now) &&
+			if (ACTIVE(oldheader, now) &&
 			    (options & DNS_DBADD_EQUALOK) != 0 &&
 			    dns_rdataslab_equalx(
-				    header, newheader, qpdb->common.rdclass,
-				    DNS_TYPEPAIR_TYPE(header->typepair)))
+				    oldheader, newheader, qpdb->common.rdclass,
+				    DNS_TYPEPAIR_TYPE(oldheader->typepair)))
 			{
 				/*
 				 * Updated by caller to ISC_R_SUCCESS after
@@ -2721,30 +2774,30 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		 * to be done w.r.t stale data; it gets replaced normally
 		 * further down.
 		 */
-		if (ACTIVE(header, now) &&
-		    header->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
-		    EXISTS(header) && EXISTS(newheader) &&
-		    header->trust >= newheader->trust &&
-		    header->expire < newheader->expire &&
-		    dns_rdataslab_equalx(header, newheader,
-					 qpdb->common.rdclass,
-					 DNS_TYPEPAIR_TYPE(header->typepair)))
+		if (ACTIVE(oldheader, now) &&
+		    oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
+		    EXISTS(oldheader) && EXISTS(newheader) &&
+		    oldheader->trust >= newheader->trust &&
+		    oldheader->expire < newheader->expire &&
+		    dns_rdataslab_equalx(
+			    oldheader, newheader, qpdb->common.rdclass,
+			    DNS_TYPEPAIR_TYPE(oldheader->typepair)))
 		{
-			if (header->noqname == NULL &&
+			if (oldheader->noqname == NULL &&
 			    newheader->noqname != NULL)
 			{
-				header->noqname = newheader->noqname;
+				oldheader->noqname = newheader->noqname;
 				newheader->noqname = NULL;
 			}
-			if (header->closest == NULL &&
+			if (oldheader->closest == NULL &&
 			    newheader->closest != NULL)
 			{
-				header->closest = newheader->closest;
+				oldheader->closest = newheader->closest;
 				newheader->closest = NULL;
 			}
 
-			qpcache_hit(qpdb, header);
-			bindrdataset(qpdb, qpnode, header, now, nlocktype,
+			qpcache_hit(qpdb, oldheader);
+			bindrdataset(qpdb, qpnode, oldheader, now, nlocktype,
 				     tlocktype,
 				     addedrdataset DNS__DB_FLARG_PASS);
 			if ((options & DNS_DBADD_EQUALOK) != 0) {
@@ -2762,46 +2815,47 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		 * to be no more than the current NS RRset's TTL.  This
 		 * ensures the delegations that are withdrawn are honoured.
 		 */
-		if (ACTIVE(header, now) &&
-		    header->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
-		    EXISTS(header) && EXISTS(newheader) &&
-		    header->trust <= newheader->trust)
+		if (ACTIVE(oldheader, now) &&
+		    oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
+		    EXISTS(oldheader) && EXISTS(newheader) &&
+		    oldheader->trust <= newheader->trust)
 		{
-			if (newheader->expire > header->expire) {
-				if (ZEROTTL(header)) {
+			if (newheader->expire > oldheader->expire) {
+				if (ZEROTTL(oldheader)) {
 					DNS_SLABHEADER_SETATTR(
 						newheader,
 						DNS_SLABHEADERATTR_ZEROTTL);
 				}
-				newheader->expire = header->expire;
+				newheader->expire = oldheader->expire;
 			}
 		}
-		if (ACTIVE(header, now) &&
+		if (ACTIVE(oldheader, now) &&
 		    (options & DNS_DBADD_PREFETCH) == 0 &&
-		    (header->typepair == DNS_TYPEPAIR(dns_rdatatype_a) ||
-		     header->typepair == DNS_TYPEPAIR(dns_rdatatype_aaaa) ||
-		     header->typepair == DNS_TYPEPAIR(dns_rdatatype_ds) ||
-		     header->typepair == DNS_SIGTYPEPAIR(dns_rdatatype_ds)) &&
-		    EXISTS(header) && EXISTS(newheader) &&
-		    header->trust >= newheader->trust &&
-		    header->expire < newheader->expire &&
-		    dns_rdataslab_equal(header, newheader))
+		    (oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_a) ||
+		     oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_aaaa) ||
+		     oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ds) ||
+		     oldheader->typepair ==
+			     DNS_SIGTYPEPAIR(dns_rdatatype_ds)) &&
+		    EXISTS(oldheader) && EXISTS(newheader) &&
+		    oldheader->trust >= newheader->trust &&
+		    oldheader->expire < newheader->expire &&
+		    dns_rdataslab_equal(oldheader, newheader))
 		{
-			if (header->noqname == NULL &&
+			if (oldheader->noqname == NULL &&
 			    newheader->noqname != NULL)
 			{
-				header->noqname = newheader->noqname;
+				oldheader->noqname = newheader->noqname;
 				newheader->noqname = NULL;
 			}
-			if (header->closest == NULL &&
+			if (oldheader->closest == NULL &&
 			    newheader->closest != NULL)
 			{
-				header->closest = newheader->closest;
+				oldheader->closest = newheader->closest;
 				newheader->closest = NULL;
 			}
 
-			qpcache_hit(qpdb, header);
-			bindrdataset(qpdb, qpnode, header, now, nlocktype,
+			qpcache_hit(qpdb, oldheader);
+			bindrdataset(qpdb, qpnode, oldheader, now, nlocktype,
 				     tlocktype,
 				     addedrdataset DNS__DB_FLARG_PASS);
 			if ((options & DNS_DBADD_EQUALOK) != 0) {
@@ -2814,19 +2868,19 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 			return DNS_R_UNCHANGED;
 		}
 
-		header->top->header = newheader;
-		newheader->top = header->top;
-		newheader->down = header;
+		oldheader->top->header = newheader;
+		newheader->top = oldheader->top;
+		newheader->down = oldheader;
 
 		ISC_SIEVE_UNLINK(qpdb->buckets[qpnode->locknum].sieve,
-				 header->top, link);
+				 oldheader->top, link);
 
 		qpcache_miss(qpdb, newheader, &nlocktype,
 			     &tlocktype DNS__DB_FLARG_PASS);
 
-		mark_ancient(header);
-		if (sigheader != NULL) {
-			mark_ancient(sigheader);
+		mark_ancient(oldheader);
+		if (oldsigheader != NULL) {
+			mark_ancient(oldsigheader);
 		}
 	} else if (!EXISTS(newheader)) {
 		/*
@@ -2873,7 +2927,11 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 				expiretop = newtop;
 			}
 
-			mark_ancient(expiretop->header);
+			dns_slabheader_t *expireheader =
+				first_header(expiretop);
+			if (expireheader != NULL) {
+				mark_ancient(expiretop->header);
+			}
 			/*
 			 * FIXME: In theory, we should mark the RRSIG
 			 * and the header at the same time, but there is
@@ -3343,9 +3401,10 @@ rdatasetiter_first(dns_rdatasetiter_t *it DNS__DB_FLARG) {
 	NODE_RDLOCK(nlock, &nlocktype);
 
 	DNS_SLABTOP_FOREACH(top, qpnode->data) {
-		if (EXISTS(top->header) &&
-		    (EXPIREDOK(iterator) ||
-		     iterator_active(qpdb, iterator, top->header)))
+		dns_slabheader_t *header = first_existing_header(top);
+
+		if (EXPIREDOK(iterator) ||
+		    (header != NULL && iterator_active(qpdb, iterator, header)))
 		{
 			iterator->current = top;
 			break;
@@ -3379,9 +3438,10 @@ rdatasetiter_next(dns_rdatasetiter_t *it DNS__DB_FLARG) {
 	NODE_RDLOCK(nlock, &nlocktype);
 
 	DNS_SLABTOP_FOREACH(top, next) {
-		if (EXISTS(top->header) &&
-		    (EXPIREDOK(iterator) ||
-		     iterator_active(qpdb, iterator, top->header)))
+		dns_slabheader_t *header = first_existing_header(top);
+
+		if (EXPIREDOK(iterator) ||
+		    (header != NULL && iterator_active(qpdb, iterator, header)))
 		{
 			iterator->current = top;
 			break;
@@ -3412,7 +3472,10 @@ rdatasetiter_current(dns_rdatasetiter_t *it,
 
 	NODE_RDLOCK(nlock, &nlocktype);
 
-	bindrdataset(qpdb, qpnode, top->header, iterator->common.now, nlocktype,
+	dns_slabheader_t *header = first_existing_header(top);
+	INSIST(header != NULL);
+
+	bindrdataset(qpdb, qpnode, header, iterator->common.now, nlocktype,
 		     isc_rwlocktype_none, rdataset DNS__DB_FLARG_PASS);
 
 	NODE_UNLOCK(nlock, &nlocktype);
diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index feac3c2a9a4..8c5d31adffb 100644
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -57,6 +57,7 @@
 #include <dns/rdatastruct.h>
 #include <dns/stats.h>
 #include <dns/time.h>
+#include <dns/types.h>
 #include <dns/view.h>
 #include <dns/zone.h>
 
@@ -64,12 +65,13 @@
 #include "qpzone_p.h"
 #include "rdataslab_p.h"
 
-#define CHECK(op)                            \
-	do {                                 \
-		result = (op);               \
-		if (result != ISC_R_SUCCESS) \
-			goto failure;        \
-	} while (0)
+#define CHECK(op)                              \
+	{                                      \
+		result = (op);                 \
+		if (result != ISC_R_SUCCESS) { \
+			goto failure;          \
+		}                              \
+	}
 
 #define HEADERNODE(h) ((qpznode_t *)((h)->node))
 
@@ -805,6 +807,51 @@ qpznode_acquire(qpznode_t *node DNS__DB_FLARG) {
 	qpznode_erefs_increment(node DNS__DB_FLARG_PASS);
 }
 
+static void
+clean_multiple_headers(dns_slabtop_t *top) {
+	dns_slabheader_t *parent = top->header;
+	dns_slabheader_t *header = NULL, *header_down = NULL;
+
+	for (header = parent->down; header != NULL; header = header_down) {
+		header_down = header->down;
+		INSIST(header->serial <= parent->serial);
+		if (header->serial == parent->serial || IGNORE(header)) {
+			parent->down = header->down;
+			dns_slabheader_destroy(&header);
+		} else {
+			parent = header;
+		}
+	}
+}
+
+static void
+check_top_header(dns_slabtop_t *top) {
+	dns_slabheader_t *header = top->header;
+	if (IGNORE(header)) {
+		top->header = header->down;
+		dns_slabheader_destroy(&header);
+	}
+}
+
+static bool
+clean_multiple_versions(dns_slabtop_t *top, uint32_t least_serial) {
+	dns_slabheader_t *parent = top->header;
+	dns_slabheader_t *header = NULL, *header_down = NULL;
+	bool multiple = false;
+
+	for (header = parent->down; header != NULL; header = header_down) {
+		header_down = header->down;
+		if (header->serial < least_serial) {
+			parent->down = header->down;
+			dns_slabheader_destroy(&header);
+		} else {
+			multiple = true;
+			parent = header;
+		}
+	}
+	return multiple;
+}
+
 static void
 clean_zone_node(qpznode_t *node, uint32_t least_serial) {
 	dns_slabtop_t *top_prev = NULL;
@@ -823,59 +870,15 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
 		 * with the same serial number, or that have the IGNORE
 		 * attribute.
 		 */
-		dns_slabheader_t *dcurrent = NULL;
-		dns_slabheader_t *dcurrent_down = NULL, *dparent = NULL;
-
-		dparent = top->header;
-		for (dcurrent = dparent->down; dcurrent != NULL;
-		     dcurrent = dcurrent_down)
-		{
-			dcurrent_down = dcurrent->down;
-			INSIST(dcurrent->serial <= dparent->serial);
-			if (dcurrent->serial == dparent->serial ||
-			    IGNORE(dcurrent))
-			{
-				dparent->down = dcurrent_down;
-				dns_slabheader_destroy(&dcurrent);
-			} else {
-				dparent = dcurrent;
-			}
-		}
-
-		/*
-		 * We've now eliminated all IGNORE datasets with the possible
-		 * exception of current, which we now check.
-		 */
-		dcurrent = top->header;
-		if (IGNORE(dcurrent)) {
-			top->header = dcurrent->down;
-			dns_slabheader_destroy(&dcurrent);
-		}
-
-		if (top->header == NULL) {
-			goto empty;
-		}
+		clean_multiple_headers(top);
 
 		/*
-		 * We now try to find the first down node less than the least
-		 * serial, and if there are such rdatasets, delete it and any
-		 * older versions.
+		 * All IGNORE datasets have been eliminated with the possible
+		 * exception of the top header, which we now check.
 		 */
-		dparent = top->header;
-		for (dcurrent = dparent->down; dcurrent != NULL;
-		     dcurrent = dcurrent_down)
-		{
-			dcurrent_down = dcurrent->down;
-			if (dcurrent->serial < least_serial) {
-				dparent->down = dcurrent_down;
-				dns_slabheader_destroy(&dcurrent);
-			} else {
-				dparent = dcurrent;
-			}
-		}
+		check_top_header(top);
 
 		if (top->header == NULL) {
-		empty:
 			if (top_prev != NULL) {
 				top_prev->next = top->next;
 			} else {
@@ -884,11 +887,17 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
 			dns_slabtop_destroy(node->mctx, &top);
 		} else {
 			/*
-			 * Note.  The serial number of 'current' might be less
-			 * than least_serial too, but we cannot delete it
+			 * Try to find the first down node less than the least
+			 * serial, and if there are such rdatasets, delete it
+			 * and any older versions.
+			 *
+			 * Note: The serial number of the top header might be
+			 * less than least_serial too, but we cannot delete it
 			 * because it is the most recent version.
 			 */
-			still_dirty = true;
+			still_dirty = clean_multiple_versions(top,
+							      least_serial);
+
 			top_prev = top;
 		}
 	}
@@ -1013,13 +1022,34 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header,
 	}
 }
 
+static dns_slabheader_t *
+first_header(dns_slabtop_t *top, uint32_t serial) {
+	for (dns_slabheader_t *header = top->header; header != NULL;
+	     header = header->down)
+	{
+		if (header->serial <= serial && !IGNORE(header)) {
+			return header;
+		}
+	}
+
+	return NULL;
+}
+
+static dns_slabheader_t *
+first_existing_header(dns_slabtop_t *top, uint32_t serial) {
+	dns_slabheader_t *header = first_header(top, serial);
+	if (header != NULL && EXISTS(header)) {
+		return header;
+	}
+	return NULL;
+}
+
 static void
 setnsec3parameters(dns_db_t *db, qpz_version_t *version) {
 	qpznode_t *node = NULL;
 	dns_rdata_nsec3param_t nsec3param;
 	isc_region_t region;
 	isc_result_t result;
-	dns_slabtop_t *top = NULL;
 	unsigned char *raw; /* RDATASLAB */
 	unsigned int count, length;
 	qpzonedb_t *qpdb = (qpzonedb_t *)db;
@@ -1033,21 +1063,11 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) {
 
 	NODE_RDLOCK(nlock, &nlocktype);
 
-	top = node->data;
-	while (top != NULL && top->typepair != dns_rdatatype_nsec3param) {
-		top = top->next;
-	}
-	if (top != NULL) {
-		dns_slabheader_t *header = top->header;
-		while (header != NULL &&
-		       (IGNORE(header) || header->serial > version->serial))
-		{
-			header = header->down;
-		}
-
-		if (header != NULL && EXISTS(header)) {
-			found = header;
+	DNS_SLABTOP_FOREACH(top, node->data) {
+		if (top->typepair != dns_rdatatype_nsec3param) {
+			continue;
 		}
+		found = first_existing_header(top, version->serial);
 	}
 
 	if (found != NULL) {
@@ -1575,17 +1595,7 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 	}
 
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
-		do {
-			if (header->serial <= serial && !IGNORE(header)) {
-				if (!EXISTS(header)) {
-					header = NULL;
-				}
-				break;
-			} else {
-				header = header->down;
-			}
-		} while (header != NULL);
+		dns_slabheader_t *header = first_existing_header(top, serial);
 		if (header != NULL) {
 			/*
 			 * We have an active, extant rdataset.  If it's a
@@ -1694,7 +1704,6 @@ done:
 static bool
 cname_and_other(qpznode_t *node, uint32_t serial) {
 	bool cname = false, other = false;
-	dns_rdatatype_t rdtype;
 
 	/*
 	 * Look for CNAME and "other data" rdatasets active in our version.
@@ -1702,21 +1711,9 @@ cname_and_other(qpznode_t *node, uint32_t serial) {
 	 * or RRSIG.
 	 */
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
-
-		rdtype = DNS_TYPEPAIR_TYPE(top->typepair);
+		dns_rdatatype_t rdtype = DNS_TYPEPAIR_TYPE(top->typepair);
 		if (rdtype == dns_rdatatype_cname) {
-			do {
-				if (header->serial <= serial && !IGNORE(header))
-				{
-					if (!EXISTS(header)) {
-						header = NULL;
-					}
-					break;
-				}
-				header = header->down;
-			} while (header != NULL);
-			if (header != NULL) {
+			if (first_existing_header(top, serial) != NULL) {
 				cname = true;
 			}
 		} else if (rdtype != dns_rdatatype_key &&
@@ -1724,17 +1721,7 @@ cname_and_other(qpznode_t *node, uint32_t serial) {
 			   rdtype != dns_rdatatype_nsec &&
 			   rdtype != dns_rdatatype_rrsig)
 		{
-			do {
-				if (header->serial <= serial && !IGNORE(header))
-				{
-					if (!EXISTS(header)) {
-						header = NULL;
-					}
-					break;
-				}
-				header = header->down;
-			} while (header != NULL);
-			if (header != NULL) {
+			if (first_existing_header(top, serial) != NULL) {
 				if (!prio_type(rdtype)) {
 					/*
 					 * CNAME is in the priority list, so if
@@ -2663,17 +2650,7 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction,
 
 		NODE_RDLOCK(nlock, &nlocktype);
 		DNS_SLABTOP_FOREACH(top, node->data) {
-			dns_slabheader_t *header = top->header;
-			while (header != NULL &&
-			       (IGNORE(header) ||
-				header->serial > search->serial))
-			{
-				header = header->down;
-			}
-			if (header != NULL && EXISTS(header)) {
-				found = header;
-				break;
-			}
+			found = first_existing_header(top, search->serial);
 		}
 		NODE_UNLOCK(nlock, &nlocktype);
 		if (found != NULL) {
@@ -3043,22 +3020,11 @@ again:
 		NODE_RDLOCK(nlock, &nlocktype);
 		empty_node = true;
 		DNS_SLABTOP_FOREACH(top, node->data) {
-			dns_slabheader_t *header = top->header;
 			/*
 			 * Look for an active, extant NSEC or RRSIG NSEC.
 			 */
-			do {
-				if (header->serial <= search->serial &&
-				    !IGNORE(header))
-				{
-					if (!EXISTS(header)) {
-						header = NULL;
-					}
-					break;
-				} else {
-					header = header->down;
-				}
-			} while (header != NULL);
+			dns_slabheader_t *header =
+				first_existing_header(top, search->serial);
 			if (header != NULL) {
 				/*
 				 * We now know that there is at least one
@@ -3186,23 +3152,12 @@ qpzone_check_zonecut(qpznode_t *node, void *arg DNS__DB_FLARG) {
 	 * Look for an NS or DNAME rdataset active in our version.
 	 */
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
 		if (top->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) ||
 		    top->typepair == DNS_TYPEPAIR(dns_rdatatype_dname) ||
 		    top->typepair == DNS_SIGTYPEPAIR(dns_rdatatype_dname))
 		{
-			do {
-				if (header->serial <= search->serial &&
-				    !IGNORE(header))
-				{
-					if (!EXISTS(header)) {
-						header = NULL;
-					}
-					break;
-				} else {
-					header = header->down;
-				}
-			} while (header != NULL);
+			dns_slabheader_t *header =
+				first_existing_header(top, search->serial);
 			if (header != NULL) {
 				if (top->typepair ==
 				    DNS_TYPEPAIR(dns_rdatatype_dname))
@@ -3516,21 +3471,11 @@ found:
 	sigpair = DNS_SIGTYPEPAIR(type);
 	empty_node = true;
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
 		/*
 		 * Look for an active, extant rdataset.
 		 */
-		do {
-			if (header->serial <= search.serial && !IGNORE(header))
-			{
-				if (!EXISTS(header)) {
-					header = NULL;
-				}
-				break;
-			} else {
-				header = header->down;
-			}
-		} while (header != NULL);
+		dns_slabheader_t *header = first_existing_header(top,
+								 search.serial);
 		if (header != NULL) {
 			/*
 			 * We now know that there is at least one active
@@ -4030,16 +3975,10 @@ rdatasetiter_first(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
 	NODE_RDLOCK(nlock, &nlocktype);
 
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
-		while (header != NULL &&
-		       (IGNORE(header) || header->serial > version->serial))
-		{
-			header = header->down;
-		}
+		qrditer->current = first_existing_header(top, version->serial);
 
-		if (header != NULL && EXISTS(header)) {
+		if (qrditer->current != NULL) {
 			qrditer->currenttop = top;
-			qrditer->current = header;
 			break;
 		}
 	}
@@ -4075,16 +4014,9 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
 	 * Find the start of the header chain for the next type.
 	 */
 	DNS_SLABTOP_FOREACH(top, next) {
-		dns_slabheader_t *header = top->header;
-		while (header != NULL &&
-		       (IGNORE(header) || header->serial > version->serial))
-		{
-			header = header->down;
-		}
-
-		if (header != NULL && EXISTS(header)) {
+		qrditer->current = first_existing_header(top, version->serial);
+		if (qrditer->current != NULL) {
 			qrditer->currenttop = top;
-			qrditer->current = header;
 			break;
 		}
 	}