diff --git a/fuzz/dns_qp.c b/fuzz/dns_qp.c index 045c8acbf2..416fd2c849 100644 --- a/fuzz/dns_qp.c +++ b/fuzz/dns_qp.c @@ -179,7 +179,8 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { UNREACHABLE(); } } else { - result = dns_qp_deletekey(qp, item[i].key, item[i].len); + result = dns_qp_deletekey(qp, item[i].key, item[i].len, + NULL, NULL); TRACE("count %zu del %s %zu >%s<", count, isc_result_toid(result), i, item[i].ascii); if (result == ISC_R_SUCCESS) { diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h index d4bf9b3650..786c8f6064 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -437,12 +437,11 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key, /*%< * Find a leaf in a qp-trie that matches the given search key * - * The leaf values are assigned to `*pval_r` and `*ival_r` + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOTFOUND. * * Requires: * \li `qpr` is a pointer to a readable qp-trie - * \li `pval_r != NULL` - * \li `ival_r != NULL` * \li `search_keylen < sizeof(dns_qpkey_t)` * * Returns: @@ -456,13 +455,12 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, /*%< * Find a leaf in a qp-trie that matches the given DNS name * - * The leaf values are assigned to `*pval_r` and `*ival_r` + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOTFOUND. * * Requires: * \li `qpr` is a pointer to a readable qp-trie * \li `name` is a pointer to a valid `dns_name_t` - * \li `pval_r != NULL` - * \li `ival_r != NULL` * * Returns: * \li ISC_R_NOTFOUND if the trie has no leaf with a matching key @@ -479,13 +477,12 @@ dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, * If the DNS_QPFIND_NOEXACT option is set, find a strict parent * domain not equal to the search name. * - * The leaf values are assigned to `*pval_r` and `*ival_r` + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOTFOUND. * * Requires: * \li `qpr` is a pointer to a readable qp-trie * \li `name` is a pointer to a valid `dns_name_t` - * \li `pval_r != NULL` - * \li `ival_r != NULL` * * Returns: * \li ISC_R_SUCCESS if an exact match was found @@ -509,10 +506,14 @@ dns_qp_insert(dns_qp_t *qp, void *pval, uint32_t ival); */ isc_result_t -dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t key, size_t keylen); +dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t key, size_t keylen, + void **pval_r, uint32_t *ival_r); /*%< * Delete a leaf from a qp-trie that matches the given key * + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOTFOUND. + * * Requires: * \li `qp` is a pointer to a valid qp-trie * \li `keylen < sizeof(dns_qpkey_t)` @@ -523,10 +524,14 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t key, size_t keylen); */ isc_result_t -dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name); +dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name, void **pval_r, + uint32_t *ival_r); /*%< * Delete a leaf from a qp-trie that matches the given DNS name * + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOTFOUND. + * * Requires: * \li `qp` is a pointer to a valid qp-trie * \li `name` is a pointer to a valid qp-trie @@ -556,6 +561,9 @@ dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); /*%< * Get the next leaf object of a trie in lexicographic order of its keys. * + * The leaf values are assigned to whichever of `*pval_r` and `*ival_r` + * are not null, unless the return value is ISC_R_NOMORE. + * * NOTE: see the safety note under `dns_qpiter_init()`. * * For example, @@ -570,8 +578,6 @@ dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); * * Requires: * \li `qpi` is a pointer to a valid qp iterator - * \li `pval_r != NULL` - * \li `ival_r != NULL` * * Returns: * \li ISC_R_SUCCESS if a leaf was found and pval_r and ival_r were set diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 29259bdb2c..6b56310273 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -1613,7 +1613,7 @@ growbranch: isc_result_t dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, - size_t search_keylen) { + size_t search_keylen, void **pval_r, uint32_t *ival_r) { REQUIRE(QP_VALID(qp)); REQUIRE(search_keylen < sizeof(dns_qpkey_t)); @@ -1643,6 +1643,8 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, return (ISC_R_NOTFOUND); } + SET_IF_NOT_NULL(pval_r, leaf_pval(n)); + SET_IF_NOT_NULL(ival_r, leaf_ival(n)); detach_leaf(qp, n); qp->leaf_count--; @@ -1685,10 +1687,11 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, } isc_result_t -dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name) { +dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name, void **pval_r, + uint32_t *ival_r) { dns_qpkey_t key; size_t keylen = dns_qpkey_fromname(key, name); - return (dns_qp_deletekey(qp, key, keylen)); + return (dns_qp_deletekey(qp, key, keylen, pval_r, ival_r)); } /*********************************************************************** @@ -1716,8 +1719,6 @@ isc_result_t dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { REQUIRE(QPITER_VALID(qpi)); REQUIRE(QP_VALID(qpi->qp)); - REQUIRE(pval_r != NULL); - REQUIRE(ival_r != NULL); dns_qpreader_t *qp = qpi->qp; @@ -1731,8 +1732,8 @@ dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { for (;;) { qp_node_t *n = ref_ptr(qp, qpi->stack[qpi->sp].ref); if (node_tag(n) == LEAF_TAG) { - *pval_r = leaf_pval(n); - *ival_r = leaf_ival(n); + SET_IF_NOT_NULL(pval_r, leaf_pval(n)); + SET_IF_NOT_NULL(ival_r, leaf_ival(n)); break; } qpi->sp++; @@ -1772,8 +1773,6 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key, qp_node_t *n = NULL; REQUIRE(QP_VALID(qp)); - REQUIRE(pval_r != NULL); - REQUIRE(ival_r != NULL); REQUIRE(search_keylen < sizeof(dns_qpkey_t)); n = get_root(qp); @@ -1797,8 +1796,8 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key, return (ISC_R_NOTFOUND); } - *pval_r = leaf_pval(n); - *ival_r = leaf_ival(n); + SET_IF_NOT_NULL(pval_r, leaf_pval(n)); + SET_IF_NOT_NULL(ival_r, leaf_ival(n)); return (ISC_R_SUCCESS); } @@ -1827,8 +1826,6 @@ dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, } label[DNS_NAME_MAXLABELS]; REQUIRE(QP_VALID(qp)); - REQUIRE(pval_r != NULL); - REQUIRE(ival_r != NULL); searchlen = dns_qpkey_fromname(search, name); if ((options & DNS_QPFIND_NOEXACT) != 0) { @@ -1888,8 +1885,8 @@ dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, offset = qpkey_compare(search, searchlen, found, foundlen); if (offset == QPKEY_EQUAL || offset == foundlen) { - *pval_r = leaf_pval(n); - *ival_r = leaf_ival(n); + SET_IF_NOT_NULL(pval_r, leaf_pval(n)); + SET_IF_NOT_NULL(ival_r, leaf_ival(n)); if (offset == QPKEY_EQUAL) { return (result); } else { @@ -1899,8 +1896,8 @@ dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, while (labels-- > 0) { if (offset > label[labels].off) { n = ref_ptr(qp, label[labels].ref); - *pval_r = leaf_pval(n); - *ival_r = leaf_ival(n); + SET_IF_NOT_NULL(pval_r, leaf_pval(n)); + SET_IF_NOT_NULL(ival_r, leaf_ival(n)); return (DNS_R_PARTIALMATCH); } } diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 76eea010c0..648c49436b 100644 --- a/lib/dns/zt.c +++ b/lib/dns/zt.c @@ -156,7 +156,7 @@ dns_zt_unmount(dns_zt_t *zt, dns_zone_t *zone) { REQUIRE(VALID_ZT(zt)); dns_qpmulti_write(zt->multi, &qp); - result = dns_qp_deletename(qp, dns_zone_getorigin(zone)); + result = dns_qp_deletename(qp, dns_zone_getorigin(zone), NULL, NULL); dns_qp_compact(qp, DNS_QPGC_MAYBE); dns_qpmulti_commit(zt->multi, &qp); @@ -169,7 +169,6 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, isc_result_t result; dns_qpread_t qpr; void *pval = NULL; - uint32_t ival; dns_ztfind_t exactmask = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_EXACT; dns_ztfind_t exactopts = options & exactmask; @@ -179,12 +178,12 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, dns_qpmulti_query(zt->multi, &qpr); if (exactopts == DNS_ZTFIND_EXACT) { - result = dns_qp_getname(&qpr, name, &pval, &ival); + result = dns_qp_getname(&qpr, name, &pval, NULL); } else if (exactopts == DNS_ZTFIND_NOEXACT) { result = dns_qp_findname_parent(&qpr, name, DNS_QPFIND_NOEXACT, - &pval, &ival); + &pval, NULL); } else { - result = dns_qp_findname_parent(&qpr, name, 0, &pval, &ival); + result = dns_qp_findname_parent(&qpr, name, 0, &pval, NULL); } dns_qpread_destroy(zt->multi, &qpr); @@ -508,7 +507,6 @@ dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub, dns_qpiter_t qpi; dns_qpread_t qpr; void *zone = NULL; - uint32_t ival; REQUIRE(VALID_ZT(zt)); REQUIRE(action != NULL); @@ -516,7 +514,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, &zone, &ival) == 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/load-names.c b/tests/bench/load-names.c index e56bea87e4..2a26b03752 100644 --- a/tests/bench/load-names.c +++ b/tests/bench/load-names.c @@ -373,10 +373,7 @@ sqz_qp(void *qp) { static isc_result_t get_qp(void *qp, size_t count, void **pval) { - uint32_t ival = 0; - isc_result_t result = dns_qp_getname(qp, &item[count].fixed.name, pval, - &ival); - return (result); + return (dns_qp_getname(qp, &item[count].fixed.name, pval, NULL)); } static void * diff --git a/tests/bench/qpmulti.c b/tests/bench/qpmulti.c index 03dd866264..f45e382577 100644 --- a/tests/bench/qpmulti.c +++ b/tests/bench/qpmulti.c @@ -311,7 +311,8 @@ mutate_transactions(uv_idle_t *idle) { uint32_t i = isc_random_uniform(args->max_item); if (item[i].present) { isc_result_t result = dns_qp_deletekey( - qp, item[i].key, item[i].len); + qp, item[i].key, item[i].len, NULL, + NULL); INSIST(result == ISC_R_SUCCESS); item[i].present = false; args->present++; diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index 7508291720..e2bc50bd9c 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -189,7 +189,11 @@ ISC_RUN_TEST_IMPL(qpiter) { dns_qpkey_t key; size_t len = qpiter_makekey(key, item, pval, ival); if (dns_qp_insert(qp, pval, ival) == ISC_R_EXISTS) { - dns_qp_deletekey(qp, key, len); + void *pvald = NULL; + uint32_t ivald = 0; + dns_qp_deletekey(qp, key, len, &pvald, &ivald); + assert_ptr_equal(pval, pvald); + assert_int_equal(ival, ivald); item[ival] = 0; } @@ -257,7 +261,6 @@ check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[]) { dns_fixedname_t fixed; dns_name_t *name = dns_fixedname_name(&fixed); void *pval = NULL; - uint32_t ival; #if 0 fprintf(stderr, "%s %u %s %s\n", check[i].query, @@ -266,7 +269,7 @@ check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[]) { #endif dns_test_namefromstring(check[i].query, &fixed); result = dns_qp_findname_parent(qp, name, check[i].options, - &pval, &ival); + &pval, NULL); assert_int_equal(result, check[i].result); if (check[i].found == NULL) { assert_null(pval); @@ -341,7 +344,7 @@ ISC_RUN_TEST_IMPL(partialmatch) { /* what if entries in the trie are relative to the zone apex? */ dns_qpkey_t rootkey = { SHIFT_NOBYTE }; - result = dns_qp_deletekey(qp, rootkey, 1); + result = dns_qp_deletekey(qp, rootkey, 1, NULL, NULL); assert_int_equal(result, ISC_R_SUCCESS); INSIST(insert[i][0] == '\0'); insert_str(qp, insert[i++]); diff --git a/tests/dns/qpmulti_test.c b/tests/dns/qpmulti_test.c index 89440984d6..30ec498f56 100644 --- a/tests/dns/qpmulti_test.c +++ b/tests/dns/qpmulti_test.c @@ -159,7 +159,6 @@ random_byte(void) { static void setup_items(void) { void *pval = NULL; - uint32_t ival = ~0U; dns_qp_t *qp = NULL; dns_qp_create(mctx, &test_methods, NULL, &qp); for (size_t i = 0; i < ARRAY_SIZE(item); i++) { @@ -172,7 +171,7 @@ setup_items(void) { memmove(item[i].ascii, item[i].key, len); qp_test_keytoascii(item[i].ascii, len); } while (dns_qp_getkey(qp, item[i].key, item[i].len, &pval, - &ival) == ISC_R_SUCCESS); + NULL) == ISC_R_SUCCESS); assert_int_equal(dns_qp_insert(qp, &item[i], i), ISC_R_SUCCESS); } dns_qp_destroy(&qp); @@ -283,9 +282,13 @@ one_transaction(dns_qpmulti_t *qpm) { if (item[i].in_rw) { /* TRACE("delete %zu %.*s", i, item[i].len, item[i].ascii); */ - result = dns_qp_deletekey(qpw, item[i].key, - item[i].len); + void *pvald = NULL; + uint32_t ivald = 0; + result = dns_qp_deletekey(qpw, item[i].key, item[i].len, + &pvald, &ivald); ASSERT(result == ISC_R_SUCCESS); + ASSERT(pvald == &item[i]); + ASSERT(ivald == i); item[i].in_rw = false; } else { /* TRACE("insert %zu %.*s", i,