diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h index cb92d4f78d..236d63faaf 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -528,8 +528,8 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, isc_result_t dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, - dns_name_t *foundname, dns_name_t *predecessor, - dns_qpchain_t *chain, void **pval_r, uint32_t *ival_r); + dns_name_t *foundname, dns_qpiter_t *iter, dns_qpchain_t *chain, + void **pval_r, uint32_t *ival_r); /*%< * Look up a leaf in a qp-trie that is equal to, or an ancestor domain of, * 'name'. @@ -547,9 +547,9 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, * ISC_R_SUCCESS then it terminates at the name that was requested. * If the result is ISC_R_NOTFOUND, 'chain' will not be updated. * - * If 'predecessor' is not NULL, it will be updated to contain the - * closest predecessor of the searched-for name that exists in the - * trie. + * If 'iter' is not NULL, it will be updated to point to a QP iterator + * which is pointed at the searched-for name if it exists in the trie, + * or the closest predecessor if it doesn't. * * The leaf data for the node that was found will be assigned to * whichever of `*pval_r` and `*ival_r` are not NULL, unless the @@ -559,7 +559,7 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, * \li `qpr` is a pointer to a readable qp-trie * \li `name` is a pointer to a valid `dns_name_t` * \li `foundname` is a pointer to a valid `dns_name_t` with - * buffer and offset space available, or is NULL. + * buffer and offset space available, or is NULL * * Returns: * \li ISC_R_SUCCESS if an exact match was found @@ -666,6 +666,24 @@ dns_qpiter_prev(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, * \li ISC_R_NOMORE otherwise */ +isc_result_t +dns_qpiter_current(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, + uint32_t *ival_r); +/*%< + * Sets the values of `name`, `pval_r` and `ival_r` to those at the + * node currently pointed to by `qpi`, but without moving the iterator + * in either direction. If the iterator is not currently pointed at a + * leaf node, ISC_R_FAILURE is returned. + * Requires: + * + * \li `qpi` is a pointer to a valid qp iterator + * + * Returns: + * \li ISC_R_SUCCESS if a leaf was found and pval_r and ival_r were set + * \li ISC_R_FAILURE if the iterator is not initialized or not pointing + * at a leaf node + */ + void dns_qpchain_init(dns_qpreadable_t qpr, dns_qpchain_t *chain); /*%< @@ -673,7 +691,7 @@ dns_qpchain_init(dns_qpreadable_t qpr, dns_qpchain_t *chain); * * Requires: * \li `qpr` is a pointer to a valid qp-trie - * \li `chain` is not NULL. + * \li `chain` is not NULL */ unsigned int @@ -682,7 +700,7 @@ dns_qpchain_length(dns_qpchain_t *chain); * Returns the length of a QP chain. * * Requires: - * \li `chain` is a pointer to an initialized QP chain object. + * \li `chain` is a pointer to an initialized QP chain object */ void @@ -695,8 +713,8 @@ dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, dns_name_t *name, * are not null. * * Requires: - * \li `chain` is a pointer to an initialized QP chain object. - * \li `level` is less than `chain->len`. + * \li `chain` is a pointer to an initialized QP chain object + * \li `level` is less than `chain->len` */ /*********************************************************************** diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 24be5ce18f..3a6d7147ad 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -1948,6 +1948,24 @@ dns_qpiter_prev(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, return (iterate(false, qpi, name, pval_r, ival_r)); } +isc_result_t +dns_qpiter_current(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, + uint32_t *ival_r) { + dns_qpnode_t *node = NULL; + + REQUIRE(QPITER_VALID(qpi)); + + node = qpi->stack[qpi->sp]; + if (node == NULL || is_branch(node)) { + return (ISC_R_FAILURE); + } + + SET_IF_NOT_NULL(pval_r, leaf_pval(node)); + SET_IF_NOT_NULL(ival_r, leaf_ival(node)); + maybe_set_name(qpi->qp, node, name); + return (ISC_R_SUCCESS); +} + /*********************************************************************** * * search @@ -2020,10 +2038,21 @@ prevleaf(dns_qpiter_t *it) { RUNTIME_CHECK(result == ISC_R_SUCCESS); } +static inline dns_qpnode_t * +greatest_leaf(dns_qpreadable_t qpr, dns_qpnode_t *n, dns_qpiter_t *iter) { + while (is_branch(n)) { + dns_qpref_t ref = branch_twigs_ref(n) + branch_twigs_size(n) - + 1; + iter->stack[++iter->sp] = n; + n = ref_ptr(qpr, ref); + } + return (n); +} + isc_result_t dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, - dns_name_t *foundname, dns_name_t *predecessor, - dns_qpchain_t *chain, void **pval_r, uint32_t *ival_r) { + dns_name_t *foundname, dns_qpiter_t *iter, 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; @@ -2032,6 +2061,7 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, dns_qpchain_t oc; dns_qpiter_t it; bool matched = true; + bool getpred = true; REQUIRE(QP_VALID(qp)); REQUIRE(foundname == NULL || ISC_MAGIC_VALID(name, DNS_NAME_MAGIC)); @@ -2041,14 +2071,18 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, if (chain == NULL) { chain = &oc; } + if (iter == NULL) { + iter = ⁢ + getpred = false; + } dns_qpchain_init(qp, chain); - dns_qpiter_init(qp, &it); + dns_qpiter_init(qp, iter); n = get_root(qp); if (n == NULL) { return (ISC_R_NOTFOUND); } - it.stack[0] = n; + iter->stack[0] = n; /* * Like `dns_qp_insert()`, we must find a leaf. However, we don't make a @@ -2093,37 +2127,66 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, * the loop. */ n = branch_twig_ptr(qp, n, bit); - } else if (predecessor != NULL) { + } else if (getpred) { /* - * this branch is a dead end, but the caller wants - * the predecessor to the name we were searching - * for, so let's go find that. + * this branch is a dead end. however, the caller + * passed us an iterator, so we need to find the + * predecessor of the searched-for-name. + * first step: find out if we've overshot + * the search key; we do that by finding an + * arbitrary leaf to compare against. */ - dns_qpweight_t pos = branch_twig_pos(n, bit); - if (pos == 0) { + size_t to; + dns_qpnode_t *least = n; + while (is_branch(least)) { + least = branch_twigs(qp, least); + } + foundlen = leaf_qpkey(qp, least, found); + to = qpkey_compare(search, searchlen, found, foundlen); + if (to == offset) { /* - * this entire branch is greater than - * the key we wanted, so we step back to - * the predecessor using the iterator. + * we're on the right branch, so find + * the best match. */ - prevleaf(&it); - n = it.stack[it.sp]; - } else { + + dns_qpweight_t pos = branch_twig_pos(n, bit); + if (pos == 0) { + /* + * every leaf in the branch is greater + * than the one we wanted; use the + * iterator to walk back to the + * predecessor. + */ + prevleaf(iter); + n = iter->stack[iter->sp--]; + } else { + /* + * the name we want would've been + * after some twig in this + * branch. point n to that twig, + * then walk down to the highest + * leaf in that subtree to get the + * predecessor. + */ + n = greatest_leaf(qp, twigs + pos - 1, + iter); + } + } else if (to <= searchlen && to <= foundlen && + search[to] < found[to]) + { /* - * the name we want would've been between - * two twigs in this branch. point n to the - * lesser of those, then walk down to the - * highest leaf in that subtree to get the + * every leaf is greater than the one + * we wanted, so iterate back to the * predecessor. */ - n = twigs + pos - 1; - while (is_branch(n)) { - prefetch_twigs(qp, n); - it.stack[++it.sp] = n; - pos = branch_twigs_size(n) - 1; - n = ref_ptr(qp, - branch_twigs_ref(n) + pos); - } + prevleaf(iter); + n = iter->stack[iter->sp--]; + } else { + /* + * every leaf is less than the one we + * wanted, so get the highest. + */ + n = greatest_leaf(qp, n, iter); } } else { /* @@ -2143,7 +2206,7 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, } } - it.stack[++it.sp] = n; + iter->stack[++iter->sp] = n; } /* do the keys differ, and if so, where? */ @@ -2151,20 +2214,30 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name, offset = qpkey_compare(search, searchlen, found, foundlen); /* - * if we've been asked to return the predecessor name, we - * work that out here. + * if we've been passed an iterator, we want it to point + * at the matching name in the case of an exact match, or at + * the predecessor name for a non-exact match. * - * if 'matched' is true, the search ended at a leaf. it's either - * an exact match or the immediate successor of the searched-for - * name, and in either case, we can use the qpiter stack we've - * constructed to step back to the predecessor. if 'matched' is - * false, then the search failed at a branch node, and we would - * have already found the predecessor. + * if 'matched' is true, then the search ended at a leaf. + * if it was not an exact match, then we're now pointing + * at either the immediate predecessor or the immediate + * successor of the searched-for name; if successor, we can + * now use the qpiter stack we've constructed to step back to + * the predecessor. if we're pointed at the predecessor + * or it was an exact match, we don't need to do anything. + * + * if 'matched' is false, then the search failed at a branch + * node, and we would already have positioned the iterator + * at the predecessor. */ - if (predecessor != NULL && matched) { - prevleaf(&it); + if (getpred && matched) { + if (offset != QPKEY_EQUAL && + (offset <= searchlen && offset <= foundlen && + found[offset] > search[offset])) + { + prevleaf(iter); + } } - maybe_set_name(qp, it.stack[it.sp], predecessor); if (offset == QPKEY_EQUAL || offset == foundlen) { SET_IF_NOT_NULL(pval_r, leaf_pval(n)); diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index 39c9b688b5..b78281dfeb 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -204,6 +204,7 @@ ISC_RUN_TEST_IMPL(qpiter) { int inserted, n; uint32_t ival; void *pval = NULL; + isc_result_t result; dns_qp_create(mctx, &qpiter_methods, item, &qp); for (size_t tests = 0; tests < 1234; tests++) { @@ -253,15 +254,30 @@ ISC_RUN_TEST_IMPL(qpiter) { while (dns_qpiter_prev(&qpi, NULL, NULL, &ival) == ISC_R_SUCCESS) { - assert_int_equal(ival, order[--n]); + --n; + + assert_int_equal(ival, order[n]); + + /* and check current iterator value as well */ + result = dns_qpiter_current(&qpi, NULL, NULL, &ival); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(ival, order[n]); } + assert_int_equal(n, 0); /* ...and forward again */ while (dns_qpiter_next(&qpi, NULL, NULL, &ival) == ISC_R_SUCCESS) { - assert_int_equal(ival, order[n++]); + assert_int_equal(ival, order[n]); + + /* and check current iterator value as well */ + result = dns_qpiter_current(&qpi, NULL, NULL, &ival); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(ival, order[n]); + + n++; } assert_int_equal(n, inserted); @@ -575,16 +591,36 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) { dns_name_t *pred = dns_fixedname_initname(&fn2); for (int i = 0; check[i].query != NULL; i++) { + dns_qpiter_t it; char *predname = NULL; dns_test_namefromstring(check[i].query, &fn1); - result = dns_qp_lookup(qp, name, NULL, pred, NULL, NULL, NULL); + result = dns_qp_lookup(qp, name, NULL, &it, NULL, NULL, NULL); #if 0 fprintf(stderr, "%s: expected %s got %s\n", check[i].query, isc_result_totext(check[i].result), isc_result_totext(result)); #endif assert_int_equal(result, check[i].result); + + if (result == ISC_R_SUCCESS) { + /* + * we found an exact match; iterate to find + * the predecessor. + */ + result = dns_qpiter_prev(&it, pred, NULL, NULL); + if (result == ISC_R_NOMORE) { + result = dns_qpiter_prev(&it, pred, NULL, NULL); + } + } else { + /* + * we didn't find a match, so the iterator should + * already be pointed at the predecessor node. + */ + result = dns_qpiter_current(&it, pred, NULL, NULL); + } + assert_int_equal(result, ISC_R_SUCCESS); + result = dns_name_tostring(pred, &predname, mctx); #if 0 fprintf(stderr, "... expected predecessor %s got %s\n", @@ -599,11 +635,11 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) { ISC_RUN_TEST_IMPL(predecessors) { dns_qp_t *qp = NULL; - 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.", - "b.a.", "x.k.c.d.", "" - }; + 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.", "b.a.", + "x.k.c.d.", "moog.", "mook.", + "moon.", "moops.", "" }; int i = 0; dns_qp_create(mctx, &string_methods, NULL, &qp); @@ -613,8 +649,8 @@ ISC_RUN_TEST_IMPL(predecessors) { /* first check: no root label in the database */ static struct check_predecessors check1[] = { - { ".", "a.b.c.d.e.", ISC_R_NOTFOUND }, - { "a.", "a.b.c.d.e.", ISC_R_SUCCESS }, + { ".", "moops.", ISC_R_NOTFOUND }, + { "a.", "moops.", ISC_R_SUCCESS }, { "b.a.", "a.", ISC_R_SUCCESS }, { "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, { "aaa.a.", "a.", DNS_R_PARTIALMATCH }, @@ -622,13 +658,16 @@ ISC_RUN_TEST_IMPL(predecessors) { { "d.c.", "c.b.b.", ISC_R_NOTFOUND }, { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, { "a.b.c.e.f.", "a.b.c.d.e.", ISC_R_NOTFOUND }, - { "z.y.x.", "a.b.c.d.e.", ISC_R_NOTFOUND }, + { "z.y.x.", "moops.", ISC_R_NOTFOUND }, { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, - { "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "0.b.c.d.e.", "x.k.c.d.", ISC_R_NOTFOUND }, { "b.d.", "c.b.b.", ISC_R_NOTFOUND }, + { "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND }, + { "moor.", "moops.", ISC_R_NOTFOUND }, + { "mop.", "moops.", ISC_R_NOTFOUND }, { NULL, NULL, 0 } }; @@ -639,7 +678,7 @@ ISC_RUN_TEST_IMPL(predecessors) { insert_str(qp, root); static struct check_predecessors check2[] = { - { ".", "a.b.c.d.e.", ISC_R_SUCCESS }, + { ".", "moops.", ISC_R_SUCCESS }, { "a.", ".", ISC_R_SUCCESS }, { "b.a.", "a.", ISC_R_SUCCESS }, { "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, @@ -648,12 +687,15 @@ ISC_RUN_TEST_IMPL(predecessors) { { "d.c.", "c.b.b.", DNS_R_PARTIALMATCH }, { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, { "a.b.c.e.f.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, - { "z.y.x.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, + { "z.y.x.", "moops.", DNS_R_PARTIALMATCH }, { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, - { "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "0.b.c.d.e.", "x.k.c.d.", DNS_R_PARTIALMATCH }, + { "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, + { "moor.", "moops.", DNS_R_PARTIALMATCH }, + { "mop.", "moops.", DNS_R_PARTIALMATCH }, { NULL, NULL, 0 } };