Improve qp-trie leaf return values

Make the `pval_r` and `ival_r` out arguments optional.

Add `pval_r` and `ival_r` out arguments to `dns_qp_deletekey()`
and `dns_qp_deletename()`, to return the deleted leaf.

diff --git a/fuzz/dns_qp.c b/fuzz/dns_qp.c
index 045c8acbf24f41b77e16a69ba00ce5794406e37b..416fd2c849b01a30d46e54c21f8cec6b9375ae4b 100644 (file)
--- 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 d4bf9b365093d08874a51fcf406ade330b0cbb91..786c8f606459d43704a6a247cee50b540c09f5f2 100644 (file)
--- 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 29259bdb2caace716f30cc8eb227de0949b0555b..6b56310273201a6044ade903120ae8def03dd1db 100644 (file)
--- 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 76eea010c0541c4c8e8b331a24f6246da5b3a522..648c49436b7c200de4529e1e2feab1001303811f 100644 (file)
--- 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 e56bea87e45f3acef7508058be27a5c3dc9ee33a..2a26b0375266a45fd63d5db931c98718f51d4176 100644 (file)
--- 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 03dd866264134b03f91ed3f84f53f9e7b5ce20e1..f45e38257728ee136b02f9f3355f5c8e3c0ff660 100644 (file)
--- 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 750829172005869a715fed8029bba68709e7fb37..e2bc50bd9c3774ce21e457061e665b05e724eed8 100644 (file)
--- 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 89440984d6717459a1c5bc857564dad5c638b504..30ec498f56f34d68222af1b6ac7f857205307595 100644 (file)
--- 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,