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).

diff --git a/lib/dns/forward.c b/lib/dns/forward.c
index d885df68b47ececc27a5bae070ecc12c6b856886..fc21a1a4ce22ed28ecef5ba4df245d1698cf5ebf 100644 (file)
--- a/lib/dns/forward.c
+++ b/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;

diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h
index 86ddbd3f45c61449f743e4486b82cf38b6fb2405..51c46cf9641d3da8ed66ed6b9a86a3fc1f9a42c6 100644 (file)
--- a/lib/dns/include/dns/qp.h
+++ b/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

diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c
index 12866f9d9ff5bf879d232b22b76b2c4bfeb6cf01..0b805fd9c4ddab8db525116a3bd152204d42d613 100644 (file)
--- a/lib/dns/keytable.c
+++ b/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) {

diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c
index 0fd1110a16a7986395ec11ee4886f75c7fa5124b..661253f3aa7a2292d32cb07d91340c4cd6027574 100644 (file)
--- a/lib/dns/nametree.c
+++ b/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) {

diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 51364142eade41acf8bda7639c1f60b4ee5d514c..4223ec27f5ae34c89f99a674e2ef31111ef87dee 100644 (file)
--- a/lib/dns/nta.c
+++ b/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) {

diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index 163793c6c7bb091313bc6bcade9fc8039cef3ea5..3f25e8bae669866342569da5f01da11bcb490a58 100644 (file)
--- a/lib/dns/qp.c
+++ b/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);
                }

diff --git a/lib/dns/zt.c b/lib/dns/zt.c
index 60e5d52a086cc2a082e70085f78cdcf02cb62a05..7d317506fba5fc7ea5b8163fd74e0053299b4b0c 100644 (file)
--- a/lib/dns/zt.c
+++ b/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);
 

diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c
index 8f05cfbcdf7081df1d75ffbf3022de497331daf6..782383d1a3f5a61617bad6d5e4b3ab0edcc02fd6 100644 (file)
--- a/tests/dns/qp_test.c
+++ b/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);
 }