From: Alessio Podda Date: Mon, 1 Dec 2025 14:12:01 +0000 (+0100) Subject: Remove maybe_set_name X-Git-Tag: v9.21.17~43^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46e25bd0db8e79c9e286aca69d3a97b263bd8370;p=thirdparty%2Fbind9.git Remove maybe_set_name Outside of unit tests, the name parameter in dns_qpiter_<...> and dns_qpchain_<...> is only used in context where the name can be extracted directly from the underlying node. This commits modifies the signatures of dns_qpiter_<...> and dns_qpchain_<...> not to have a name parameter. Where the name parameter was needed, we now query the node and copy the name directly from it. This allows us to remove maybe_set_name from qp.c. Besides simplifying the API, this leads to a performance speedup for NXDOMAIN handling, as we avoid calling maybe_set_name inside step, and maybe_set_name is very inefficient. A copy of the implementation maybe_set_name is retained for the unit tests. --- diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h index 2ae5b58ce42..00dc4610670 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -650,17 +650,14 @@ dns_qpiter_init(dns_qpreadable_t qpr, dns_qpiter_t *qpi); */ isc_result_t -dns_qpiter_next(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, - uint32_t *ival_r); +dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); isc_result_t -dns_qpiter_prev(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, - uint32_t *ival_r); +dns_qpiter_prev(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); /*%< * Iterate forward/backward through a QP trie in lexicographic order. * * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` - * are not null, unless the return value is ISC_R_NOMORE. Similarly, - * if `name` is not null, it is updated to contain the node name. + * are not null, unless the return value is ISC_R_NOMORE. * * NOTE: see the safety note under `dns_qpiter_init()`. * @@ -682,16 +679,16 @@ 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); +dns_qpiter_current(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); /*%< - * Sets the values of `name`, `pval_r` and `ival_r` to those at the + * Sets the values of `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: * + * Requires: * \li `qpi` is a pointer to a valid qp iterator * * Returns: @@ -700,6 +697,7 @@ dns_qpiter_current(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, * at a leaf node */ + void dns_qpchain_init(dns_qpreadable_t qpr, dns_qpchain_t *chain); /*%< @@ -720,7 +718,7 @@ dns_qpchain_length(dns_qpchain_t *chain); */ void -dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, dns_name_t *name, +dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, void **pval_r, uint32_t *ival_r); /*%< * Sets 'name' to the name of the leaf referenced at `chain->stack[level]`. diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 5890ffdcd62..04fe2277b7c 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -150,7 +150,7 @@ destroy_keytable(dns_keytable_t *keytable) { dns_qpmulti_query(keytable->table, &qpr); dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns_keynode_t *n = pval; dns_keynode_detach(&n); } @@ -648,7 +648,7 @@ dns_keytable_totext(dns_keytable_t *keytable, isc_buffer_t *text) { dns_qpmulti_query(keytable->table, &qpr); dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns_keynode_t *knode = pval; if (knode->dslist != NULL) { result = keynode_dslist_totext(knode, text); @@ -676,7 +676,7 @@ dns_keytable_forall(dns_keytable_t *keytable, dns_qpmulti_query(keytable->table, &qpr); dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns_keynode_t *knode = pval; (*func)(keytable, knode, &knode->name, arg); } diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 2ad9c07ea01..e4cf9ab46d6 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -475,7 +475,7 @@ dns_ntatable_totext(dns_ntatable_t *ntatable, const char *view, dns_qpmulti_query(ntatable->table, &qpr); dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns__nta_t *n = pval; char nbuf[DNS_NAME_FORMATSIZE]; char tbuf[ISC_FORMATHTTPTIMESTAMP_SIZE]; @@ -528,7 +528,7 @@ dns_ntatable_save(dns_ntatable_t *ntatable, FILE *fp) { dns_qpmulti_query(ntatable->table, &qpr); dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns__nta_t *n = pval; isc_buffer_t b; char nbuf[DNS_NAME_FORMATSIZE + 1], tbuf[80]; @@ -614,7 +614,7 @@ dns_ntatable_shutdown(dns_ntatable_t *ntatable) { ntatable->shuttingdown = true; dns_qpiter_init(&qpr, &iter); - while (dns_qpiter_next(&iter, NULL, &pval, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { dns__nta_t *n = pval; dns__nta_shutdown(n); dns__nta_detach(&n); diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 091b691e2f6..b38ee5f5c2d 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -1857,20 +1857,6 @@ dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name, dns_namespace_t space, /*********************************************************************** * chains */ -void -maybe_set_name(dns_qpreader_t *qp, dns_qpnode_t *node, dns_name_t *name) { - dns_qpkey_t key; - size_t len; - - if (name == NULL) { - return; - } - - dns_name_reset(name); - len = leaf_qpkey(qp, node, key); - dns_qpkey_toname(key, len, name, NULL); -} - void dns_qpchain_init(dns_qpreadable_t qpr, dns_qpchain_t *chain) { dns_qpreader_t *qp = dns_qpreader(qpr); @@ -1895,7 +1881,7 @@ dns_qpchain_length(dns_qpchain_t *chain) { } void -dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, dns_name_t *name, +dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, void **pval_r, uint32_t *ival_r) { dns_qpnode_t *node = NULL; @@ -1903,7 +1889,6 @@ dns_qpchain_node(dns_qpchain_t *chain, unsigned int level, dns_name_t *name, REQUIRE(level < chain->len); node = chain->chain[level].node; - maybe_set_name(chain->qp, node, name); SET_IF_NOT_NULL(pval_r, leaf_pval(node)); SET_IF_NOT_NULL(ival_r, leaf_ival(node)); } @@ -1956,8 +1941,7 @@ last_twig(dns_qpiter_t *qpi, bool forward) { * a mutable view of the trie which is altered while iterating */ static isc_result_t -iterate(bool forward, dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, - uint32_t *ival_r) { +iterate(bool forward, dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { dns_qpnode_t *node = NULL; bool initial_branch = true; @@ -2032,25 +2016,23 @@ iterate(bool forward, dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, /* we're at a leaf: return its data to the caller */ 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; } isc_result_t -dns_qpiter_next(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, - uint32_t *ival_r) { - return iterate(true, qpi, name, pval_r, ival_r); +dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { + return iterate(true, qpi, pval_r, ival_r); } isc_result_t -dns_qpiter_prev(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, - uint32_t *ival_r) { - return iterate(false, qpi, name, pval_r, ival_r); +dns_qpiter_prev(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { + return iterate(false, qpi, 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_qpiter_current(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { dns_qpnode_t *node = NULL; REQUIRE(QPITER_VALID(qpi)); @@ -2062,10 +2044,10 @@ dns_qpiter_current(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, 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 @@ -2131,9 +2113,9 @@ add_link(dns_qpchain_t *chain, dns_qpnode_t *node, size_t offset) { static inline void prevleaf(dns_qpiter_t *it) { - isc_result_t result = dns_qpiter_prev(it, NULL, NULL, NULL); + isc_result_t result = dns_qpiter_prev(it, NULL, NULL); if (result == ISC_R_NOMORE) { - result = dns_qpiter_prev(it, NULL, NULL, NULL); + result = dns_qpiter_prev(it, NULL, NULL); } RUNTIME_CHECK(result == ISC_R_SUCCESS); } diff --git a/lib/dns/qp_p.h b/lib/dns/qp_p.h index 51d2be6ea79..6ce46f3c221 100644 --- a/lib/dns/qp_p.h +++ b/lib/dns/qp_p.h @@ -945,8 +945,6 @@ extern uint8_t dns_qp_byte_for_bit[]; /**********************************************************************/ -void -maybe_set_name(dns_qpreader_t *qp, dns_qpnode_t *node, dns_name_t *name); void dns__qp_initialize(void); diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 92d00a2595d..660d592da13 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -1419,7 +1419,7 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL); + dns_qpchain_node(&search->chain, i, (void **)&node, NULL); nlock = &qpdb->buckets[node->locknum].lock; NODE_RDLOCK(nlock, &nlocktype); @@ -1502,10 +1502,11 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, /* * Extract predecessor from iterator. */ - result = dns_qpiter_current(&iter, predecessor, NULL, NULL); + result = dns_qpiter_current(&iter, (void **)&node, NULL); if (result != ISC_R_SUCCESS) { return ISC_R_NOTFOUND; } + dns_name_copy(&node->name, predecessor); /* * Lookup the predecessor in the normal namespace. @@ -1636,7 +1637,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, isc_result_t zcresult; qpcnode_t *encloser = NULL; - dns_qpchain_node(&search.chain, i, NULL, (void **)&encloser, + dns_qpchain_node(&search.chain, i, (void **)&encloser, NULL); zcresult = check_zonecut(encloser, @@ -2071,7 +2072,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, INSIST(len >= 2); node = NULL; - dns_qpchain_node(&search.chain, len - 2, NULL, + dns_qpchain_node(&search.chain, len - 2, (void **)&node, NULL); search.chain.len = len - 1; } @@ -3566,7 +3567,7 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); dns_qpiter_init(qpdb->tree, &qpdbiter->iter); - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); if (result == ISC_R_SUCCESS && @@ -3656,7 +3657,7 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) { dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); if (result == ISC_R_SUCCESS && diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 405a7c55513..44a5addfb5a 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -2684,7 +2684,10 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction, qpznode_t *node = NULL; isc_result_t result = ISC_R_SUCCESS; - result = dns_qpiter_current(it, nodename, (void **)&node, NULL); + result = dns_qpiter_current(it, (void **)&node, NULL); + if (result == ISC_R_SUCCESS) { + dns_name_copy(&node->name, nodename); + } while (result == ISC_R_SUCCESS) { isc_rwlock_t *nlock = qpzone_get_lock(node); isc_rwlocktype_t nlocktype = isc_rwlocktype_none; @@ -2700,11 +2703,12 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction, } if (direction == FORWARD) { - result = dns_qpiter_next(it, nodename, (void **)&node, - NULL); + result = dns_qpiter_next(it, (void **)&node, NULL); } else { - result = dns_qpiter_prev(it, nodename, (void **)&node, - NULL); + result = dns_qpiter_prev(it, (void **)&node, NULL); + } + if (result == ISC_R_SUCCESS) { + dns_name_copy(&node->name, nodename); } }; if (result == ISC_R_SUCCESS) { @@ -2730,7 +2734,7 @@ activeempty(qpz_search_t *search, dns_qpiter_t *it, const dns_name_t *current) { * subdomain of the one we were looking for, then we're * at an active empty nonterminal node. */ - isc_result_t result = dns_qpiter_next(it, NULL, NULL, NULL); + isc_result_t result = dns_qpiter_next(it, NULL, NULL); if (result != ISC_R_SUCCESS) { /* An ENT at the end of the zone is impossible */ return false; @@ -2776,7 +2780,7 @@ wildcard_blocked(qpz_search_t *search, const dns_name_t *qname, /* Now reset the iterator and look for a successor with data. */ it = search->iter; - result = dns_qpiter_next(&it, NULL, NULL, NULL); + result = dns_qpiter_next(&it, NULL, NULL); if (result == ISC_R_SUCCESS) { check_next = step(search, &it, FORWARD, next); } @@ -2841,7 +2845,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, const dns_name_t *qname, isc_rwlocktype_t nlocktype = isc_rwlocktype_none; bool wild, active; - dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL); + dns_qpchain_node(&search->chain, i, (void **)&node, NULL); nlock = qpzone_get_lock(node); NODE_RDLOCK(nlock, &nlocktype); @@ -2939,8 +2943,10 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, REQUIRE(type == dns_rdatatype_nsec3 || firstp != NULL); if (type == dns_rdatatype_nsec3) { - result = dns_qpiter_prev(&search->iter, name, (void **)nodep, - NULL); + result = dns_qpiter_prev(&search->iter, (void **)nodep, NULL); + if (result == ISC_R_SUCCESS) { + dns_name_copy(&(*nodep)->name, name); + } return result; } @@ -2955,6 +2961,8 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, *firstp = false; result = dns_qp_lookup(&qpr, name, DNS_DBNAMESPACE_NSEC, nit, NULL, NULL, NULL); + qpznode_t *node = NULL; + INSIST(result != ISC_R_NOTFOUND); if (result == ISC_R_SUCCESS) { /* @@ -2965,7 +2973,10 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, * NSEC record; we want the previous node * in the NSEC tree. */ - result = dns_qpiter_prev(nit, name, NULL, NULL); + result = dns_qpiter_prev(nit, (void **)&node, NULL); + if (result == ISC_R_SUCCESS) { + dns_name_copy(&node->name, name); + } } else if (result == DNS_R_PARTIALMATCH) { /* * This was a partial match, so the @@ -2973,7 +2984,10 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, * node in the NSEC namespace, which is * what we want. */ - dns_qpiter_current(nit, name, NULL, NULL); + isc_result_t iresult = dns_qpiter_current(nit, (void **)&node, NULL); + if (iresult == ISC_R_SUCCESS) { + dns_name_copy(&node->name, name); + } result = ISC_R_SUCCESS; } } else { @@ -2984,7 +2998,11 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, * work; perhaps they lacked signature records. * Keep searching. */ - result = dns_qpiter_prev(nit, name, NULL, NULL); + qpznode_t *tempnode = NULL; + result = dns_qpiter_prev(nit, (void **)&tempnode, NULL); + if (result == ISC_R_SUCCESS) { + dns_name_copy(&tempnode->name, name); + } } if (result != ISC_R_SUCCESS) { break; @@ -3050,10 +3068,11 @@ find_closest_nsec(qpz_search_t *search, dns_dbnode_t **nodep, * then we start using the auxiliary NSEC namespace to find * the next predecessor. */ - result = dns_qpiter_current(&search->iter, name, (void **)&node, NULL); + result = dns_qpiter_current(&search->iter, (void **)&node, NULL); if (result != ISC_R_SUCCESS) { return result; } + dns_name_copy(&node->name, name); again: do { dns_slabheader_t *found = NULL, *foundsig = NULL; @@ -3158,9 +3177,10 @@ again: } while (empty_node && result == ISC_R_SUCCESS); if (result == ISC_R_NOMORE && wraps) { - result = dns_qpiter_prev(&search->iter, name, (void **)&node, + result = dns_qpiter_prev(&search->iter, (void **)&node, NULL); if (result == ISC_R_SUCCESS) { + dns_name_copy(&node->name, name); wraps = false; goto again; } @@ -3395,7 +3415,7 @@ qpzone_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, qpznode_t *n = NULL; isc_result_t tresult; - dns_qpchain_node(&search.chain, i, NULL, (void **)&n, NULL); + dns_qpchain_node(&search.chain, i, (void **)&n, NULL); tresult = qpzone_check_zonecut(n, &search DNS__DB_FLARG_PASS); if (tresult != DNS_R_CONTINUE) { result = tresult; @@ -3677,7 +3697,7 @@ found: unsigned int len = search.chain.len - 1; if (len > 0) { NODE_UNLOCK(nlock, &nlocktype); - dns_qpchain_node(&search.chain, len - 1, NULL, + dns_qpchain_node(&search.chain, len - 1, (void **)&node, NULL); dns_name_copy(&node->name, foundname); goto partial_match; @@ -4133,7 +4153,7 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); dns_qpiter_init(qpdbiter->snap, &qpdbiter->iter); - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); switch (qpdbiter->nsec3mode) { @@ -4154,7 +4174,7 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS && QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } @@ -4186,7 +4206,7 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { /* skip the NSEC3 origin node (or its predecessor) */ - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } @@ -4221,7 +4241,7 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) { dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); dns_qpiter_init(qpdbiter->snap, &qpdbiter->iter); - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); switch (qpdbiter->nsec3mode) { @@ -4245,7 +4265,7 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS && QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } @@ -4275,7 +4295,7 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS) { INSIST(QPDBITER_NSEC_ORIGIN_NODE(qpdb, qpdbiter)); /* skip the NSEC origin node */ - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } else { @@ -4284,7 +4304,7 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) { * should point to its predecessor, which is the node we * want. */ - result = dns_qpiter_current(&qpdbiter->iter, NULL, + result = dns_qpiter_current(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); INSIST(result == ISC_R_SUCCESS); @@ -4383,7 +4403,7 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) { node = qpdbiter->node; qpdbiter->node = NULL; - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); switch (qpdbiter->nsec3mode) { @@ -4407,7 +4427,7 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS && QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } @@ -4433,7 +4453,7 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS) { INSIST(QPDBITER_NSEC_ORIGIN_NODE(qpdb, qpdbiter)); /* skip the NSEC origin node */ - result = dns_qpiter_prev(&qpdbiter->iter, NULL, + result = dns_qpiter_prev(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } else { @@ -4442,7 +4462,7 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) { * should point to its predecessor, which is the node we * want. */ - result = dns_qpiter_current(&qpdbiter->iter, NULL, + result = dns_qpiter_current(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); INSIST(result == ISC_R_SUCCESS); @@ -4496,7 +4516,7 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) { node = qpdbiter->node; qpdbiter->node = NULL; - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); switch (qpdbiter->nsec3mode) { @@ -4514,7 +4534,7 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) { if (result == ISC_R_SUCCESS && QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } @@ -4540,7 +4560,7 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) { QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) { /* skip the NSEC3 origin node (or its predecessor). */ - result = dns_qpiter_next(&qpdbiter->iter, NULL, + result = dns_qpiter_next(&qpdbiter->iter, (void **)&qpdbiter->node, NULL); } diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 3cf30b36088..676917d517f 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -2511,7 +2511,7 @@ dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, case DNS_R_PARTIALMATCH: i = dns_qpchain_length(&chain); while (i-- > 0) { - dns_qpchain_node(&chain, i, NULL, (void **)&data, NULL); + dns_qpchain_node(&chain, i, (void **)&data, NULL); INSIST(data != NULL); if (rpz_type == DNS_RPZ_TYPE_QNAME) { found_zbits |= data->wild.qname; diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 758a4eae64d..831fb7190fa 100644 --- a/lib/dns/zt.c +++ b/lib/dns/zt.c @@ -190,7 +190,7 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, /* get pval from the previous chain link */ int len = dns_qpchain_length(&chain); if (len >= 2) { - dns_qpchain_node(&chain, len - 2, NULL, &pval, + dns_qpchain_node(&chain, len - 2, &pval, NULL); result = DNS_R_PARTIALMATCH; } else { @@ -527,7 +527,7 @@ dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub, dns_qpmulti_query(zt->multi, &qpr); dns_qpiter_init(&qpr, &qpi); - while (dns_qpiter_next(&qpi, NULL, &zone, NULL) == ISC_R_SUCCESS) { + while (dns_qpiter_next(&qpi, &zone, NULL) == ISC_R_SUCCESS) { result = action(zone, uap); if (tresult == ISC_R_SUCCESS) { tresult = result; diff --git a/tests/bench/qplookups.c b/tests/bench/qplookups.c index f83b1846d35..4617c1b84d4 100644 --- a/tests/bench/qplookups.c +++ b/tests/bench/qplookups.c @@ -212,10 +212,13 @@ main(int argc, char **argv) { start = isc_time_monotonic(); for (i = 0;; i++) { + void *pval = NULL; + uint32_t ival = 0; name = dns_fixedname_initname(&items[i]); - if (dns_qpiter_next(&it, name, NULL, NULL) != ISC_R_SUCCESS) { + if (dns_qpiter_next(&it, &pval, &ival) != ISC_R_SUCCESS) { break; } + name_from_smallname(name, pval, ival); } stop = isc_time_monotonic(); diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index 6bb9d91ce45..03c65be43e9 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -46,6 +46,70 @@ bool verbose = false; +/* + * Test-only functions for dns_qpiter_*_with_name functionality + */ +static void +maybe_set_name(dns_qpreader_t *qp, void *pval, uint32_t ival, + dns_name_t *name) { + if (name != NULL) { + dns_qpkey_t key; + size_t len; + dns_qpnode_t node = make_leaf(pval, ival); + dns_name_reset(name); + len = leaf_qpkey(qp, &node, key); + dns_qpkey_toname(key, len, name, NULL); + } +} + +static isc_result_t +qpiter_next_with_name(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, + uint32_t *ival_r) { + isc_result_t result; + void *pval = NULL; + uint32_t ival = 0; + + result = iterate(true, qpi, &pval, &ival); + if (result == ISC_R_SUCCESS) { + SET_IF_NOT_NULL(pval_r, pval); + SET_IF_NOT_NULL(ival_r, ival); + maybe_set_name(qpi->qp, pval, ival, name); + } + return result; +} + +static isc_result_t +qpiter_prev_with_name(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, + uint32_t *ival_r) { + isc_result_t result; + void *pval = NULL; + uint32_t ival = 0; + + result = iterate(false, qpi, &pval, &ival); + if (result == ISC_R_SUCCESS) { + SET_IF_NOT_NULL(pval_r, pval); + SET_IF_NOT_NULL(ival_r, ival); + maybe_set_name(qpi->qp, pval, ival, name); + } + return result; +} + +static isc_result_t +qpiter_current_with_name(dns_qpiter_t *qpi, dns_name_t *name, void **pval_r, + uint32_t *ival_r) { + isc_result_t result; + void *pval = NULL; + uint32_t ival = 0; + + result = dns_qpiter_current(qpi, &pval, &ival); + if (result == ISC_R_SUCCESS) { + SET_IF_NOT_NULL(pval_r, pval); + SET_IF_NOT_NULL(ival_r, ival); + maybe_set_name(qpi->qp, pval, ival, name); + } + return result; +} + ISC_RUN_TEST_IMPL(qpkey_name) { struct { const char *namestr; @@ -288,7 +352,7 @@ ISC_RUN_TEST_IMPL(qpiter) { /* check that we see only valid items in the correct order */ uint32_t prev = 0; dns_qpiter_init(qp, &qpi); - while (dns_qpiter_next(&qpi, NULL, &pval, &ival) == + while (dns_qpiter_next(&qpi, &pval, &ival) == ISC_R_SUCCESS) { assert_in_range(ival, prev + 1, ITER_ITEMS - 1); @@ -309,7 +373,7 @@ ISC_RUN_TEST_IMPL(qpiter) { /* now iterate backward and check correctness */ n = inserted; - while (dns_qpiter_prev(&qpi, NULL, NULL, &ival) == + while (dns_qpiter_prev(&qpi, NULL, &ival) == ISC_R_SUCCESS) { --n; @@ -317,7 +381,7 @@ ISC_RUN_TEST_IMPL(qpiter) { assert_int_equal(ival, order[n]); /* and check current iterator value as well */ - result = dns_qpiter_current(&qpi, NULL, NULL, &ival); + result = dns_qpiter_current(&qpi, NULL, &ival); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(ival, order[n]); } @@ -325,13 +389,13 @@ ISC_RUN_TEST_IMPL(qpiter) { assert_int_equal(n, 0); /* ...and forward again */ - while (dns_qpiter_next(&qpi, NULL, NULL, &ival) == + while (dns_qpiter_next(&qpi, NULL, &ival) == ISC_R_SUCCESS) { assert_int_equal(ival, order[n]); /* and check current iterator value as well */ - result = dns_qpiter_current(&qpi, NULL, NULL, &ival); + result = dns_qpiter_current(&qpi, NULL, &ival); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(ival, order[n]); @@ -347,32 +411,32 @@ ISC_RUN_TEST_IMPL(qpiter) { */ if (inserted > 3) { assert_int_equal( - dns_qpiter_next(&qpi, NULL, NULL, &ival), + dns_qpiter_next(&qpi, NULL, &ival), ISC_R_SUCCESS); assert_int_equal(ival, order[0]); assert_int_equal( - dns_qpiter_next(&qpi, NULL, NULL, &ival), + dns_qpiter_next(&qpi, NULL, &ival), ISC_R_SUCCESS); assert_int_equal(ival, order[1]); assert_int_equal( - dns_qpiter_prev(&qpi, NULL, NULL, &ival), + dns_qpiter_prev(&qpi, NULL, &ival), ISC_R_SUCCESS); assert_int_equal(ival, order[0]); assert_int_equal( - dns_qpiter_next(&qpi, NULL, NULL, &ival), + dns_qpiter_next(&qpi, NULL, &ival), ISC_R_SUCCESS); assert_int_equal(ival, order[1]); assert_int_equal( - dns_qpiter_prev(&qpi, NULL, NULL, &ival), + dns_qpiter_prev(&qpi, NULL, &ival), ISC_R_SUCCESS); assert_int_equal(ival, order[0]); assert_int_equal( - dns_qpiter_prev(&qpi, NULL, NULL, &ival), + dns_qpiter_prev(&qpi, NULL, &ival), ISC_R_NOMORE); } } @@ -426,12 +490,12 @@ check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[], result = dns_qp_lookup(qp, name, space, NULL, &chain, &pval, NULL); - /* Extract the found name using maybe_set_name if we found something */ - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - if (chain.len > 0) { - dns_qpnode_t *node = chain.chain[chain.len - 1].node; - maybe_set_name((dns_qpreader_t *)qp, node, foundname); - } + /* Extract the found name if we found something */ + if ((result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) && + pval != NULL) + { + maybe_set_name((dns_qpreader_t *)qp, pval, space, + foundname); } #if 0 @@ -612,7 +676,10 @@ check_qpchainiter(dns_qp_t *qp, struct check_qpchain check[], dns_name_t *expected = dns_fixedname_initname(&fn2); dns_name_t *found = dns_fixedname_initname(&fn3); dns_test_namefromstring(check[i].names[j], &fn2); - dns_qpchain_node(&chain, j, found, NULL, NULL); + void *pval = NULL; + uint32_t ival = 0; + dns_qpchain_node(&chain, j, &pval, &ival); + maybe_set_name(chain.qp, pval, ival, found); #if 0 char nb[DNS_NAME_FORMATSIZE]; dns_name_format(found, nb, sizeof(nb)); @@ -839,9 +906,9 @@ check_predecessors_withchain(dns_qp_t *qp, struct check_predecessors check[], * we found an exact match; iterate to find * the predecessor. */ - result = dns_qpiter_prev(&it, pred, NULL, &ival); + result = qpiter_prev_with_name(&it, pred, NULL, &ival); if (result == ISC_R_NOMORE) { - result = dns_qpiter_prev(&it, pred, NULL, + result = qpiter_prev_with_name(&it, pred, NULL, &ival); } } else { @@ -849,7 +916,7 @@ check_predecessors_withchain(dns_qp_t *qp, struct check_predecessors check[], * we didn't find a match, so the iterator should * already be pointed at the predecessor node. */ - result = dns_qpiter_current(&it, pred, NULL, &ival); + result = qpiter_current_with_name(&it, pred, NULL, &ival); } assert_int_equal(result, ISC_R_SUCCESS); @@ -869,7 +936,7 @@ check_predecessors_withchain(dns_qp_t *qp, struct check_predecessors check[], isc_mem_free(isc_g_mctx, predstr); int j = 0; - while (dns_qpiter_next(&it, name, NULL, NULL) == ISC_R_SUCCESS) + while (qpiter_next_with_name(&it, name, NULL, NULL) == ISC_R_SUCCESS) { #if 0 result = dns_name_tostring(name, &namestr, isc_g_mctx);