remove DNS_QPFIND_NOEXACT

since dns_qp_findname_ancestor() can now return a chain object, it is no
longer necessary to provide a _NOEXACT search option. if we want to look
up the closest ancestor of a name, we can just do a normal search, and
if successful, retrieve the second-to-last node from the QP chain.

this makes ancestor lookups slightly more complicated for the caller,
but allows us to simplify the code in dns_qp_findname_ancestor(), making
it easier to ensure correctness.  this was a fairly rare use case:
outside of unit tests, DNS_QPFIND_NOEXACT was only used in the zone
table, which has now been updated to use the QP chain.  the equivalent
RBT feature is only used by the resolver for cache lookups of 'atparent'
types (i.e, DS records).

lib/dns/forward.c

@@ -168,8 +168,7 @@ dns_fwdtable_find(dns_fwdtable_t *fwdtable, const dns_name_t *name,
 	REQUIRE(VALID_FWDTABLE(fwdtable));
 
 	dns_qpmulti_query(fwdtable->table, &qpr);
-	result = dns_qp_findname_ancestor(&qpr, name, 0, NULL, NULL, &pval,
-					  NULL);
+	result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL);
 	if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
 		dns_forwarders_t *fwdrs = pval;
 		*forwardersp = fwdrs;

lib/dns/include/dns/qp.h

@@ -281,13 +281,6 @@ typedef enum dns_qpgc {
 	DNS_QPGC_ALL,
 } dns_qpgc_t;
 
-/*%
- * Options for fancy searches such as `dns_qp_findname_ancestor()`
- */
-typedef enum dns_qpfind {
-	DNS_QPFIND_NOEXACT = 1 << 0,
-} dns_qpfind_t;
-
 /***********************************************************************
  *
  *  functions - create, destory, enquire
@@ -495,15 +488,12 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r,
 
 isc_result_t
 dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name,
-			 dns_qpfind_t options, dns_name_t *foundname,
-			 dns_qpchain_t *chain, void **pval_r, uint32_t *ival_r);
+			 dns_name_t *foundname, dns_qpchain_t *chain,
+			 void **pval_r, uint32_t *ival_r);
 /*%<
  * Find a leaf in a qp-trie that is an ancestor domain of, or equal to, the
  * given DNS name.
  *
- * If the DNS_QPFIND_NOEXACT option is set, find the closest ancestor
- * domain that is not equal to the search name.
- *
  * If 'foundname' is not NULL, it is updated to contain the name found.
  *
  * If 'chain' is not NULL, it is updated to contain a QP chain with

lib/dns/keytable.c

@@ -518,8 +518,7 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name,
 	REQUIRE(foundname != NULL);
 
 	dns_qpmulti_query(keytable->table, &qpr);
-	result = dns_qp_findname_ancestor(&qpr, name, 0, NULL, NULL, &pval,
-					  NULL);
+	result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL);
 	keynode = pval;
 
 	if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
@@ -548,8 +547,7 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
 	REQUIRE(wantdnssecp != NULL);
 
 	dns_qpmulti_query(keytable->table, &qpr);
-	result = dns_qp_findname_ancestor(&qpr, name, 0, NULL, NULL, &pval,
-					  NULL);
+	result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL);
 	if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
 		keynode = pval;
 		if (foundname != NULL) {

lib/dns/nametree.c

@@ -289,7 +289,7 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name,
 	REQUIRE(VALID_NAMETREE(nametree));
 
 	dns_qpmulti_query(nametree->table, &qpr);
-	result = dns_qp_findname_ancestor(&qpr, name, 0, found, NULL,
+	result = dns_qp_findname_ancestor(&qpr, name, found, NULL,
 					  (void **)&node, NULL);
 	if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
 		switch (nametree->type) {

lib/dns/nta.c

@@ -413,8 +413,7 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now,
 
 	RWLOCK(&ntatable->rwlock, isc_rwlocktype_read);
 	dns_qpmulti_query(ntatable->table, &qpr);
-	result = dns_qp_findname_ancestor(&qpr, name, 0, NULL, NULL, &pval,
-					  NULL);
+	result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL);
 	nta = pval;
 
 	switch (result) {

lib/dns/qp.c

@@ -360,25 +360,6 @@ qpkey_compare(const dns_qpkey_t key_a, const size_t keylen_a,
 	return (QPKEY_EQUAL);
 }
 
-/*
- * Given a key constructed by dns_qpkey_fromname(), trim it down to the last
- * label boundary before the `max` length.
- *
- * This is used when searching a trie for the best match for a name.
- */
-static size_t
-qpkey_trim_label(dns_qpkey_t key, size_t len, size_t max) {
-	size_t stop = 0;
-	for (size_t offset = 0; offset < max; offset++) {
-		if (qpkey_bit(key, len, offset) == SHIFT_NOBYTE &&
-		    qpkey_bit(key, len, offset + 1) != SHIFT_NOBYTE)
-		{
-			stop = offset + 1;
-		}
-	}
-	return (stop);
-}
-
 /***********************************************************************
  *
  *  allocator wrappers
@@ -2023,27 +2004,19 @@ add_link(dns_qpchain_t *chain, qp_node_t *node, size_t offset) {
 
 isc_result_t
 dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name,
-			 dns_qpfind_t options, dns_name_t *foundname,
-			 dns_qpchain_t *chain, void **pval_r,
-			 uint32_t *ival_r) {
+			 dns_name_t *foundname, dns_qpchain_t *chain,
+			 void **pval_r, uint32_t *ival_r) {
 	dns_qpreader_t *qp = dns_qpreader(qpr);
 	dns_qpkey_t search, found;
 	size_t searchlen, foundlen;
 	size_t offset;
 	qp_node_t *n = NULL;
-	isc_result_t result;
 	dns_qpchain_t oc;
 
 	REQUIRE(QP_VALID(qp));
 	REQUIRE(foundname == NULL || ISC_MAGIC_VALID(name, DNS_NAME_MAGIC));
 
 	searchlen = dns_qpkey_fromname(search, name);
-	if ((options & DNS_QPFIND_NOEXACT) != 0) {
-		searchlen = qpkey_trim_label(search, searchlen, searchlen);
-		result = DNS_R_PARTIALMATCH;
-	} else {
-		result = ISC_R_SUCCESS;
-	}
 
 	if (chain == NULL) {
 		chain = &oc;
@@ -2121,11 +2094,9 @@ dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name,
 		SET_IF_NOT_NULL(ival_r, leaf_ival(n));
 		maybe_set_name(qp, n, foundname);
 		if (offset == QPKEY_EQUAL) {
-			if ((options & DNS_QPFIND_NOEXACT) == 0) {
-				/* add the exact match to the chain */
-				add_link(chain, n, offset);
-			}
-			return (result);
+			/* add the exact match to the chain */
+			add_link(chain, n, offset);
+			return (ISC_R_SUCCESS);
 		} else {
 			return (DNS_R_PARTIALMATCH);
 		}

lib/dns/zt.c

@@ -171,21 +171,26 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options,
 	void *pval = NULL;
 	dns_ztfind_t exactmask = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_EXACT;
 	dns_ztfind_t exactopts = options & exactmask;
+	dns_qpchain_t chain;
 
 	REQUIRE(VALID_ZT(zt));
 	REQUIRE(exactopts != exactmask);
 
 	dns_qpmulti_query(zt->multi, &qpr);
 
-	if (exactopts == DNS_ZTFIND_EXACT) {
-		result = dns_qp_getname(&qpr, name, &pval, NULL);
-	} else if (exactopts == DNS_ZTFIND_NOEXACT) {
-		result = dns_qp_findname_ancestor(&qpr, name,
-						  DNS_QPFIND_NOEXACT, NULL,
-						  NULL, &pval, NULL);
-	} else {
-		result = dns_qp_findname_ancestor(&qpr, name, 0, NULL, NULL,
-						  &pval, NULL);
+	result = dns_qp_findname_ancestor(&qpr, name, NULL, &chain, &pval,
+					  NULL);
+	if (exactopts == DNS_ZTFIND_EXACT && result == DNS_R_PARTIALMATCH) {
+		result = ISC_R_NOTFOUND;
+	} else if (exactopts == DNS_ZTFIND_NOEXACT && result == ISC_R_SUCCESS) {
+		/* get pval from the previous chain link */
+		int len = dns_qpchain_length(&chain);
+		if (len >= 2) {
+			dns_qpchain_node(&chain, len - 2, NULL, &pval, NULL);
+			result = DNS_R_PARTIALMATCH;
+		} else {
+			result = ISC_R_NOTFOUND;
+		}
 	}
 	dns_qpread_destroy(zt->multi, &qpr);
 

tests/dns/qp_test.c

@@ -335,7 +335,6 @@ const dns_qpmethods_t string_methods = {
 
 struct check_partialmatch {
 	const char *query;
-	dns_qpfind_t options;
 	isc_result_t result;
 	const char *found;
 };
@@ -350,13 +349,13 @@ check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[]) {
 		void *pval = NULL;
 
 		dns_test_namefromstring(check[i].query, &fn1);
-		result = dns_qp_findname_ancestor(qp, name, check[i].options,
-						  foundname, NULL, &pval, NULL);
+		result = dns_qp_findname_ancestor(qp, name, foundname, NULL,
+						  &pval, NULL);
 
 #if 0
-		fprintf(stderr, "%s (flags %u) %s (expected %s) "
+		fprintf(stderr, "%s %s (expected %s) "
 			"value \"%s\" (expected \"%s\")\n",
-			check[i].query, check[i].options,
+			check[i].query,
 			isc_result_totext(result),
 			isc_result_totext(check[i].result), (char *)pval,
 			check[i].found);
@@ -426,26 +425,19 @@ ISC_RUN_TEST_IMPL(partialmatch) {
 	}
 
 	static struct check_partialmatch check1[] = {
-		{ "a.b.", 0, ISC_R_SUCCESS, "a.b." },
-		{ "a.b.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "b." },
-		{ "b.c.", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL },
-		{ "bar.", 0, ISC_R_NOTFOUND, NULL },
-		{ "f.bar.", 0, ISC_R_NOTFOUND, NULL },
-		{ "foo.bar.", 0, ISC_R_SUCCESS, "foo.bar." },
-		{ "foo.bar.", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL },
-		{ "foooo.bar.", 0, ISC_R_NOTFOUND, NULL },
-		{ "w.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." },
-		{ "www.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." },
-		{ "web.foo.bar.", 0, ISC_R_SUCCESS, "web.foo.bar." },
-		{ "webby.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." },
-		{ "my.web.foo.bar.", 0, DNS_R_PARTIALMATCH, "web.foo.bar." },
-		{ "web.foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH,
-		  "foo.bar." },
-		{ "my.web.foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH,
-		  "web.foo.bar." },
-		{ "my.other.foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH,
-		  "foo.bar." },
-		{ NULL, 0, 0, NULL },
+		{ "a.b.", ISC_R_SUCCESS, "a.b." },
+		{ "b.c.", ISC_R_NOTFOUND, NULL },
+		{ "bar.", ISC_R_NOTFOUND, NULL },
+		{ "f.bar.", ISC_R_NOTFOUND, NULL },
+		{ "foo.bar.", ISC_R_SUCCESS, "foo.bar." },
+		{ "foooo.bar.", ISC_R_NOTFOUND, NULL },
+		{ "w.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." },
+		{ "www.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." },
+		{ "web.foo.bar.", ISC_R_SUCCESS, "web.foo.bar." },
+		{ "webby.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." },
+		{ "my.web.foo.bar.", DNS_R_PARTIALMATCH, "web.foo.bar." },
+		{ "my.other.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." },
+		{ NULL },
 	};
 	check_partialmatch(qp, check1);
 
@@ -454,13 +446,11 @@ ISC_RUN_TEST_IMPL(partialmatch) {
 	insert_str(qp, insert[i++]);
 
 	static struct check_partialmatch check2[] = {
-		{ "b.c.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "." },
-		{ "bar.", 0, DNS_R_PARTIALMATCH, "." },
-		{ "foo.bar.", 0, ISC_R_SUCCESS, "foo.bar." },
-		{ "foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "." },
-		{ "bar", 0, ISC_R_NOTFOUND, NULL },
-		{ "bar", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "." },
-		{ NULL, 0, 0, NULL },
+		{ "b.c.", DNS_R_PARTIALMATCH, "." },
+		{ "bar.", DNS_R_PARTIALMATCH, "." },
+		{ "foo.bar.", ISC_R_SUCCESS, "foo.bar." },
+		{ "bar", ISC_R_NOTFOUND, NULL },
+		{ NULL },
 	};
 	check_partialmatch(qp, check2);
 
@@ -471,35 +461,26 @@ ISC_RUN_TEST_IMPL(partialmatch) {
 	dns_qpkey_t rootkey = { SHIFT_NOBYTE };
 	result = dns_qp_deletekey(qp, rootkey, 1, NULL, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
-	check_partialmatch(
-		qp,
-		(struct check_partialmatch[]){
-			{ "bar", 0, ISC_R_NOTFOUND, NULL },
-			{ "bar", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL },
-			{ "bar.", 0, ISC_R_NOTFOUND, NULL },
-			{ "bar.", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL },
-			{ NULL, 0, 0, NULL },
-		});
+	check_partialmatch(qp, (struct check_partialmatch[]){
+				       { "bar", ISC_R_NOTFOUND, NULL },
+				       { "bar.", ISC_R_NOTFOUND, NULL },
+				       { NULL },
+			       });
 
 	/* what if there's a root node with an empty key? */
 	INSIST(insert[i][0] == '\0');
 	insert_str(qp, insert[i++]);
-	check_partialmatch(
-		qp,
-		(struct check_partialmatch[]){
-			{ "bar", 0, DNS_R_PARTIALMATCH, "" },
-			{ "bar", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "" },
-			{ "bar.", 0, DNS_R_PARTIALMATCH, "" },
-			{ "bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "" },
-			{ NULL, 0, 0, NULL },
-		});
+	check_partialmatch(qp, (struct check_partialmatch[]){
+				       { "bar", DNS_R_PARTIALMATCH, "" },
+				       { "bar.", DNS_R_PARTIALMATCH, "" },
+				       { NULL },
+			       });
 
 	dns_qp_destroy(&qp);
 }
 
 struct check_qpchain {
 	const char *query;
-	dns_qpfind_t options;
 	isc_result_t result;
 	unsigned int length;
 	const char *names[10];
@@ -515,13 +496,12 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
 
 		dns_qpchain_init(qp, &chain);
 		dns_test_namefromstring(check[i].query, &fn1);
-		result = dns_qp_findname_ancestor(qp, name, check[i].options,
-						  NULL, &chain, NULL, NULL);
+		result = dns_qp_findname_ancestor(qp, name, NULL, &chain, NULL,
+						  NULL);
 
 #if 0
-		fprintf(stderr, "%s (%d) %s (expected %s), "
-			"len %d (expected %d)\n",
-			check[i].query, check[i].options,
+		fprintf(stderr, "%s %s (expected %s), "
+			"len %d (expected %d)\n", check[i].query,
 			isc_result_totext(result),
 			isc_result_totext(check[i].result),
 			dns_qpchain_length(&chain), check[i].length);
@@ -557,38 +537,25 @@ ISC_RUN_TEST_IMPL(qpchain) {
 	 */
 	const char insert[][16] = { ".",	  "a.",	    "b.",   "c.b.a.",
 				    "e.d.c.b.a.", "c.b.b.", "c.d.", "a.b.c.d.",
-				    "a.b.c.d.e.", "" };
+				    "a.b.c.d.e.", "b.a.",   "" };
 
 	while (insert[i][0] != '\0') {
 		insert_str(qp, insert[i++]);
 	}
 
 	static struct check_qpchain check1[] = {
-		{ "b.", 0, ISC_R_SUCCESS, 2 },
-		{ "c.", 0, DNS_R_PARTIALMATCH, 1 },
-		{ "e.d.c.b.a.", 0, ISC_R_SUCCESS, 4 },
-		{ "a.b.c.d.", 0, ISC_R_SUCCESS, 3 },
-		{ "a.b.c.d.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, 2 },
-		{ NULL, 0, 0, 0 },
+		{ "b.", ISC_R_SUCCESS, 2, { ".", "b." } },
+		{ "b.a.", ISC_R_SUCCESS, 3, { ".", "a.", "b.a." } },
+		{ "c.", DNS_R_PARTIALMATCH, 1, { "." } },
+		{ "e.d.c.b.a.",
+		  ISC_R_SUCCESS,
+		  5,
+		  { ".", "a.", "b.a.", "c.b.a.", "e.d.c.b.a." } },
+		{ "a.b.c.d.", ISC_R_SUCCESS, 3, { ".", "c.d.", "a.b.c.d." } },
+		{ "b.c.d.", DNS_R_PARTIALMATCH, 2, { ".", "c.d." } },
+		{ NULL, 0, 0, { NULL } },
 	};
 
-	check1[0].names[0] = ".";
-	check1[0].names[1] = "b.";
-
-	check1[1].names[0] = ".";
-
-	check1[2].names[0] = ".";
-	check1[2].names[1] = "a.";
-	check1[2].names[2] = "c.b.a.";
-	check1[2].names[3] = "e.d.c.b.a.";
-
-	check1[3].names[0] = ".";
-	check1[3].names[1] = "c.d.";
-	check1[3].names[2] = "a.b.c.d.";
-
-	check1[4].names[0] = ".";
-	check1[4].names[1] = "c.d.";
-
 	check_qpchain(qp, check1);
 	dns_qp_destroy(&qp);
 }