more rdataslab refactoring

- there are now two functions for getting rdataslab size:
  dns_rdataslab_size() is for full slabs and dns_rdataslab_sizeraw()
  for raw slabs. there is no longer a need for a reservelen parameter.
- dns_rdataslab_count() also no longer takes a reservelen parameter.
  (currently it's never used for raw slabs, so there is no _countraw()
  function.)
- dns_rdataslab_rdatasize() has been removed, because
  dns_rdataslab_sizeraw() can do the same thing.
- dns_rdataslab_merge() and dns_rdataslab_subtract() both take
  slabheader parameters instead of character buffers, and the
  reservelen parameter has been removed.

bin/tests/system/rpz/tests_sh_rpz.py

@@ -53,6 +53,7 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns5/rpz-switch",
         "ns6/bl.tld2s.db",
         "ns6/empty.db",
+        "ns6/empty.db.jnl",
         "ns6/named.stats",
         "ns7/policy2.db",
         "ns8/manual-update-rpz.db",

lib/dns/include/dns/rdataslab.h

@@ -195,81 +195,84 @@ dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
  */
 
 unsigned int
-dns_rdataslab_size(unsigned char *slab, unsigned int reservelen);
+dns_rdataslab_size(dns_slabheader_t *header);
 /*%<
- * Return the total size of an rdataslab.
+ * Return the total size of the rdataslab following 'header'.
  *
  * Requires:
- *\li	'slab' points to a slab.
+ *\li	'header' points to a slabheader with an rdataslab following it.
  *
  * Returns:
- *\li	The number of bytes in the slab, including the reservelen.
+ *\li	The number of bytes in the slab, plus the header.
  */
 
 unsigned int
-dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen);
+dns_rdataslab_sizeraw(unsigned char *slab);
 /*%<
- * Return the size of the rdata in an rdataslab.
+ * Return the total size of the raw rdataslab 'slab'.
  *
  * Requires:
- *\li	'slab' points to a slab.
+ *\li	'slab' points to a raw slab.
+ *
+ * Returns:
+ *\li	The number of bytes in the slab.
  */
 
 unsigned int
-dns_rdataslab_count(unsigned char *slab, unsigned int reservelen);
+dns_rdataslab_count(dns_slabheader_t *header);
 /*%<
- * Return the number of records in the rdataslab
+ * Return the number of records in the rdataslab following 'header'.
  *
  * Requires:
- *\li	'slab' points to a slab.
+ *\li	'header' points to a slabheader with an rdataslab following it.
  *
  * Returns:
  *\li	The number of records in the slab.
  */
 
 isc_result_t
-dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
+dns_rdataslab_merge(dns_slabheader_t *oheader, dns_slabheader_t *nheader,
-		    unsigned int reservelen, isc_mem_t *mctx,
+		    isc_mem_t *mctx, dns_rdataclass_t rdclass,
-		    dns_rdataclass_t rdclass, dns_rdatatype_t type,
+		    dns_rdatatype_t type, unsigned int flags,
-		    unsigned int flags, uint32_t maxrrperset,
+		    uint32_t maxrrperset, dns_slabheader_t **theaderp);
-		    unsigned char **tslabp);
 /*%<
- * Merge 'oslab' and 'nslab'.
+ * Merge the slabs following 'oheader' and 'nheader'.
  */
 
 isc_result_t
-dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
+dns_rdataslab_subtract(dns_slabheader_t *mheader, dns_slabheader_t *sheader,
-		       unsigned int reservelen, isc_mem_t *mctx,
+		       isc_mem_t *mctx, dns_rdataclass_t rdclass,
-		       dns_rdataclass_t rdclass, dns_rdatatype_t type,
+		       dns_rdatatype_t type, unsigned int flags,
-		       unsigned int flags, unsigned char **tslabp);
+		       dns_slabheader_t **theaderp);
 /*%<
- * Subtract 'sslab' from 'mslab'.  If 'exact' is true then all elements
+ * Subtract the slab following 'sheader' from the one following 'mheader'.
- * of 'sslab' must exist in 'mslab'.
+ * If 'exact' is true then all elements from the 'sheader' slab must exist
+ * in the 'mheader' slab.
  *
  * XXX
  * valid flags are DNS_RDATASLAB_EXACT
  */
 
 bool
-dns_rdataslab_equal(dns_slabheader_t *slab1, dns_slabheader_t *slab2);
+dns_rdataslab_equal(dns_slabheader_t *header1, dns_slabheader_t *header2);
 /*%<
  * Compare two rdataslabs for equality.  This does _not_ do a full
  * DNSSEC comparison.
  *
  * Requires:
- *\li	'slab1' and 'slab2' point to slabs.
+ *\li	'header1' and 'header1' point to slab headers followed by slabs.
  *
  * Returns:
  *\li	true if the slabs are equal, false otherwise.
  */
 bool
-dns_rdataslab_equalx(dns_slabheader_t *slab1, dns_slabheader_t *slab2,
+dns_rdataslab_equalx(dns_slabheader_t *header1, dns_slabheader_t *header2,
 		     dns_rdataclass_t rdclass, dns_rdatatype_t type);
 /*%<
  * Compare two rdataslabs for DNSSEC equality.
  *
  * Requires:
- *\li	'slab1' and 'slab2' point to slabs.
+ *\li	'header1' and 'header2' point to slab headers followed by slabs.
  *
  * Returns:
  *\li	true if the slabs are equal, #false otherwise.

lib/dns/qpcache.c

@@ -2286,8 +2286,7 @@ expiredata(dns_db_t *db, dns_dbnode_t *node, void *data) {
 static size_t
 rdataset_size(dns_slabheader_t *header) {
 	if (EXISTS(header)) {
-		return dns_rdataslab_size((unsigned char *)header,
+		return dns_rdataslab_size(header);
-					  sizeof(*header));
 	}
 
 	return sizeof(*header);

lib/dns/qpzone.c

@@ -1877,28 +1877,23 @@ add_changed(dns_slabheader_t *header, qpz_version_t *version DNS__DB_FLARG) {
 
 static uint64_t
 recordsize(dns_slabheader_t *header, unsigned int namelen) {
-	return dns_rdataslab_rdatasize((unsigned char *)header,
+	return dns_rdataslab_size(header) + sizeof(dns_ttl_t) +
-				       sizeof(*header)) +
+	       sizeof(dns_rdatatype_t) + sizeof(dns_rdataclass_t) + namelen;
-	       sizeof(dns_ttl_t) + sizeof(dns_rdatatype_t) +
-	       sizeof(dns_rdataclass_t) + namelen;
 }
 
 static void
 maybe_update_recordsandsize(bool add, qpz_version_t *version,
 			    dns_slabheader_t *header, unsigned int namelen) {
-	unsigned char *hdr = (unsigned char *)header;
-	size_t hdrsize = sizeof(*header);
-
 	if (NONEXISTENT(header)) {
 		return;
 	}
 
 	RWLOCK(&version->rwlock, isc_rwlocktype_write);
 	if (add) {
-		version->records += dns_rdataslab_count(hdr, hdrsize);
+		version->records += dns_rdataslab_count(header);
 		version->xfrsize += recordsize(header, namelen);
 	} else {
-		version->records -= dns_rdataslab_count(hdr, hdrsize);
+		version->records -= dns_rdataslab_count(header);
 		version->xfrsize -= recordsize(header, namelen);
 	}
 	RWUNLOCK(&version->rwlock, isc_rwlocktype_write);
@@ -1913,7 +1908,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 	dns_slabheader_t *topheader = NULL, *topheader_prev = NULL;
 	dns_slabheader_t *prioheader = NULL;
 	dns_slabheader_t *header = NULL;
-	unsigned char *merged = NULL;
+	dns_slabheader_t *merged = NULL;
 	isc_result_t result;
 	bool merge = false;
 	uint32_t ntypes;
@@ -1979,10 +1974,8 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 			}
 			if (result == ISC_R_SUCCESS) {
 				result = dns_rdataslab_merge(
-					(unsigned char *)header,
+					header, newheader, qpdb->common.mctx,
-					(unsigned char *)newheader,
+					qpdb->common.rdclass,
-					(unsigned int)(sizeof(*newheader)),
-					qpdb->common.mctx, qpdb->common.rdclass,
 					(dns_rdatatype_t)header->type, flags,
 					qpdb->maxrrperset, &merged);
 			}
@@ -1996,7 +1989,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 				 * clean_zone_node() runs.
 				 */
 				dns_slabheader_destroy(&newheader);
-				newheader = (dns_slabheader_t *)merged;
+				newheader = merged;
 				dns_slabheader_reset(newheader,
 						     (dns_db_t *)qpdb,
 						     (dns_dbnode_t *)node);
@@ -4842,7 +4835,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 	dns_name_t *nodename = dns_fixedname_initname(&fname);
 	dns_slabheader_t *topheader = NULL, *topheader_prev = NULL;
 	dns_slabheader_t *header = NULL, *newheader = NULL;
-	unsigned char *subresult = NULL;
+	dns_slabheader_t *subresult = NULL;
 	isc_region_t region;
 	isc_result_t result;
 	qpz_changed_t *changed = NULL;
@@ -4925,16 +4918,14 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 		}
 		if (result == ISC_R_SUCCESS) {
 			result = dns_rdataslab_subtract(
-				(unsigned char *)header,
+				header, newheader, qpdb->common.mctx,
-				(unsigned char *)newheader,
+				qpdb->common.rdclass,
-				(unsigned int)(sizeof(*newheader)),
-				qpdb->common.mctx, qpdb->common.rdclass,
 				(dns_rdatatype_t)header->type, flags,
 				&subresult);
 		}
 		if (result == ISC_R_SUCCESS) {
 			dns_slabheader_destroy(&newheader);
-			newheader = (dns_slabheader_t *)subresult;
+			newheader = subresult;
 			dns_slabheader_reset(newheader, db,
 					     (dns_dbnode_t *)node);
 			dns_slabheader_copycase(newheader, header);

lib/dns/rdataslab.c

@@ -292,10 +292,10 @@ free_rdatas:
 }
 
 unsigned int
-dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) {
+dns_rdataslab_sizeraw(unsigned char *slab) {
 	REQUIRE(slab != NULL);
 
-	unsigned char *current = slab + reservelen;
+	unsigned char *current = slab;
 	uint16_t count = get_uint16(current);
 
 	while (count-- > 0) {
@@ -307,27 +307,18 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) {
 }
 
 unsigned int
-dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen) {
+dns_rdataslab_size(dns_slabheader_t *header) {
-	REQUIRE(slab != NULL);
+	REQUIRE(header != NULL);
 
-	uint16_t rdatalen = 0;
+	unsigned char *s = (unsigned char *)header + sizeof(dns_slabheader_t);
-	unsigned char *current = slab + reservelen;
+	return dns_rdataslab_sizeraw(s) + sizeof(dns_slabheader_t);
-	uint16_t count = get_uint16(current);
-
-	while (count-- > 0) {
-		uint16_t length = get_uint16(current);
-		rdatalen += length;
-		current += length;
-	}
-
-	return rdatalen;
 }
 
 unsigned int
-dns_rdataslab_count(unsigned char *slab, unsigned int reservelen) {
+dns_rdataslab_count(dns_slabheader_t *header) {
-	REQUIRE(slab != NULL);
+	REQUIRE(header != NULL);
 
-	unsigned char *current = slab + reservelen;
+	unsigned char *current = (unsigned char *)header + sizeof(*header);
 	uint16_t count = get_uint16(current);
 
 	return count;
@@ -394,11 +385,12 @@ rdata_in_slab(unsigned char *slab, unsigned int reservelen,
 }
 
 isc_result_t
-dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
+dns_rdataslab_merge(dns_slabheader_t *oheader, dns_slabheader_t *nheader,
-		    unsigned int reservelen, isc_mem_t *mctx,
+		    isc_mem_t *mctx, dns_rdataclass_t rdclass,
-		    dns_rdataclass_t rdclass, dns_rdatatype_t type,
+		    dns_rdatatype_t type, unsigned int flags,
-		    unsigned int flags, uint32_t maxrrperset,
+		    uint32_t maxrrperset, dns_slabheader_t **theaderp) {
-		    unsigned char **tslabp) {
+	unsigned char *oslab = (unsigned char *)oheader;
+	unsigned char *nslab = (unsigned char *)nheader;
 	unsigned char *ocurrent = NULL, *ostart = NULL, *ncurrent = NULL;
 	unsigned char *tstart = NULL, *tcurrent = NULL, *data = NULL;
 	unsigned int ocount, ncount, count, olength, tlength, tcount, length;
@@ -414,13 +406,13 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 	 * or perhaps another merge routine for this purpose.
 	 */
 
-	REQUIRE(tslabp != NULL && *tslabp == NULL);
+	REQUIRE(theaderp != NULL && *theaderp == NULL);
 	REQUIRE(oslab != NULL && nslab != NULL);
 
-	ocurrent = oslab + reservelen;
+	ocurrent = oslab + sizeof(dns_slabheader_t);
 	ocount = get_uint16(ocurrent);
 	ostart = ocurrent;
-	ncurrent = nslab + reservelen;
+	ncurrent = nslab + sizeof(dns_slabheader_t);
 	ncount = get_uint16(ncurrent);
 	INSIST(ocount > 0 && ncount > 0);
 
@@ -445,7 +437,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 	/*
 	 * Start figuring out the target length and count.
 	 */
-	tlength = reservelen + 2 + olength;
+	tlength = sizeof(dns_slabheader_t) + 2 + olength;
 	tcount = ocount;
 
 	/*
@@ -455,7 +447,9 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 	do {
 		dns_rdata_init(&nrdata);
 		rdata_from_slab(&ncurrent, rdclass, type, &nrdata);
-		if (!rdata_in_slab(oslab, reservelen, rdclass, type, &nrdata)) {
+		if (!rdata_in_slab(oslab, sizeof(dns_slabheader_t), rdclass,
+				   type, &nrdata))
+		{
 			/*
 			 * This rdata isn't in the old slab.
 			 */
@@ -499,8 +493,8 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 	 * Copy the reserved area from the new slab.
 	 */
 	tstart = isc_mem_get(mctx, tlength);
-	memmove(tstart, nslab, reservelen);
+	memmove(tstart, nslab, sizeof(dns_slabheader_t));
-	tcurrent = tstart + reservelen;
+	tcurrent = tstart + sizeof(dns_slabheader_t);
 
 	/*
 	 * Write the new count.
@@ -515,14 +509,14 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 	INSIST(ocount != 0);
 	rdata_from_slab(&ocurrent, rdclass, type, &ordata);
 
-	ncurrent = nslab + reservelen + 2;
+	ncurrent = nslab + sizeof(dns_slabheader_t) + 2;
 
 	if (ncount > 0) {
 		do {
 			dns_rdata_reset(&nrdata);
 			rdata_from_slab(&ncurrent, rdclass, type, &nrdata);
-		} while (rdata_in_slab(oslab, reservelen, rdclass, type,
+		} while (rdata_in_slab(oslab, sizeof(dns_slabheader_t), rdclass,
-				       &nrdata));
+				       type, &nrdata));
 	}
 
 	while (oadded < ocount || nadded < ncount) {
@@ -568,7 +562,8 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 					dns_rdata_reset(&nrdata);
 					rdata_from_slab(&ncurrent, rdclass,
 							type, &nrdata);
-				} while (rdata_in_slab(oslab, reservelen,
+				} while (rdata_in_slab(oslab,
+						       sizeof(dns_slabheader_t),
 						       rdclass, type, &nrdata));
 			}
 		}
@@ -576,28 +571,30 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
 
 	INSIST(tcurrent == tstart + tlength);
 
-	*tslabp = tstart;
+	*theaderp = (dns_slabheader_t *)tstart;
 
 	return ISC_R_SUCCESS;
 }
 
 isc_result_t
-dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
+dns_rdataslab_subtract(dns_slabheader_t *mheader, dns_slabheader_t *sheader,
-		       unsigned int reservelen, isc_mem_t *mctx,
+		       isc_mem_t *mctx, dns_rdataclass_t rdclass,
-		       dns_rdataclass_t rdclass, dns_rdatatype_t type,
+		       dns_rdatatype_t type, unsigned int flags,
-		       unsigned int flags, unsigned char **tslabp) {
+		       dns_slabheader_t **theaderp) {
+	unsigned char *mslab = (unsigned char *)mheader;
+	unsigned char *sslab = (unsigned char *)sheader;
 	unsigned char *mcurrent = NULL, *sstart = NULL, *scurrent = NULL;
 	unsigned char *tstart = NULL, *tcurrent = NULL;
 	unsigned int mcount, scount, rcount, count, tlength, tcount, i;
 	dns_rdata_t srdata = DNS_RDATA_INIT;
 	dns_rdata_t mrdata = DNS_RDATA_INIT;
 
-	REQUIRE(tslabp != NULL && *tslabp == NULL);
+	REQUIRE(theaderp != NULL && *theaderp == NULL);
 	REQUIRE(mslab != NULL && sslab != NULL);
 
-	mcurrent = mslab + reservelen;
+	mcurrent = mslab + sizeof(dns_slabheader_t);
 	mcount = get_uint16(mcurrent);
-	scurrent = sslab + reservelen;
+	scurrent = sslab + sizeof(dns_slabheader_t);
 	scount = get_uint16(scurrent);
 	INSIST(mcount > 0 && scount > 0);
 
@@ -608,7 +605,7 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
 	/*
 	 * Start figuring out the target length and count.
 	 */
-	tlength = reservelen + 2;
+	tlength = sizeof(dns_slabheader_t) + 2;
 	tcount = 0;
 	rcount = 0;
 
@@ -668,8 +665,8 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
 	 * Copy the reserved area from the mslab.
 	 */
 	tstart = isc_mem_get(mctx, tlength);
-	memmove(tstart, mslab, reservelen);
+	memmove(tstart, mslab, sizeof(dns_slabheader_t));
-	tcurrent = tstart + reservelen;
+	tcurrent = tstart + sizeof(dns_slabheader_t);
 
 	/*
 	 * Write the new count.
@@ -680,7 +677,7 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
 	/*
 	 * Copy the parts of mslab not in sslab.
 	 */
-	mcurrent = mslab + reservelen;
+	mcurrent = mslab + sizeof(dns_slabheader_t);
 	mcount = get_uint16(mcurrent);
 	for (i = 0; i < mcount; i++) {
 		unsigned char *mrdatabegin = mcurrent;
@@ -708,7 +705,7 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
 
 	INSIST(tcurrent == tstart + tlength);
 
-	*tslabp = tstart;
+	*theaderp = (dns_slabheader_t *)tstart;
 
 	return ISC_R_SUCCESS;
 }
@@ -717,7 +714,6 @@ bool
 dns_rdataslab_equal(dns_slabheader_t *slab1, dns_slabheader_t *slab2) {
 	unsigned char *current1 = NULL, *current2 = NULL;
 	unsigned int count1, count2;
-	unsigned int length1, length2;
 
 	current1 = (unsigned char *)slab1 + sizeof(dns_slabheader_t);
 	count1 = get_uint16(current1);
@@ -732,8 +728,8 @@ dns_rdataslab_equal(dns_slabheader_t *slab1, dns_slabheader_t *slab2) {
 	}
 
 	while (count1-- > 0) {
-		length1 = get_uint16(current1);
+		unsigned int length1 = get_uint16(current1);
-		length2 = get_uint16(current2);
+		unsigned int length2 = get_uint16(current2);
 
 		if (length1 != length2 ||
 		    memcmp(current1, current2, length1) != 0)
@@ -752,8 +748,6 @@ dns_rdataslab_equalx(dns_slabheader_t *slab1, dns_slabheader_t *slab2,
 		     dns_rdataclass_t rdclass, dns_rdatatype_t type) {
 	unsigned char *current1 = NULL, *current2 = NULL;
 	unsigned int count1, count2;
-	dns_rdata_t rdata1 = DNS_RDATA_INIT;
-	dns_rdata_t rdata2 = DNS_RDATA_INIT;
 
 	current1 = (unsigned char *)slab1 + sizeof(dns_slabheader_t);
 	count1 = get_uint16(current1);
@@ -768,13 +762,14 @@ dns_rdataslab_equalx(dns_slabheader_t *slab1, dns_slabheader_t *slab2,
 	}
 
 	while (count1-- > 0) {
+		dns_rdata_t rdata1 = DNS_RDATA_INIT;
+		dns_rdata_t rdata2 = DNS_RDATA_INIT;
+
 		rdata_from_slab(&current1, rdclass, type, &rdata1);
 		rdata_from_slab(&current2, rdclass, type, &rdata2);
 		if (dns_rdata_compare(&rdata1, &rdata2) != 0) {
 			return false;
 		}
-		dns_rdata_reset(&rdata1);
-		dns_rdata_reset(&rdata2);
 	}
 	return true;
 }
@@ -867,8 +862,7 @@ dns_slabheader_destroy(dns_slabheader_t **headerp) {
 	if (NONEXISTENT(header)) {
 		size = sizeof(*header);
 	} else {
-		size = dns_rdataslab_size((unsigned char *)header,
+		size = dns_rdataslab_size(header);
-					  sizeof(*header));
 	}
 
 	isc_mem_put(mctx, header, size);
@@ -881,11 +875,11 @@ dns_slabheader_freeproof(isc_mem_t *mctx, dns_slabheader_proof_t **proof) {
 	}
 	if ((*proof)->neg != NULL) {
 		isc_mem_put(mctx, (*proof)->neg,
-			    dns_rdataslab_size((*proof)->neg, 0));
+			    dns_rdataslab_sizeraw((*proof)->neg));
 	}
 	if ((*proof)->negsig != NULL) {
 		isc_mem_put(mctx, (*proof)->negsig,
-			    dns_rdataslab_size((*proof)->negsig, 0));
+			    dns_rdataslab_sizeraw((*proof)->negsig));
 	}
 	isc_mem_put(mctx, *proof, sizeof(**proof));
 	*proof = NULL;