From ac9bd03a0d092e4de01ecf70683b4a128ae4cc0b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 4 Oct 2023 18:14:55 -0700 Subject: [PATCH] clean up dns_rbt - create_node() in rbt.c cannot fail - the dns_rbt_*name() functions, which are wrappers around dns_rbt_[add|find|delete]node(), were never used except in tests. this change isn't really necessary since RBT is likely to go away eventually anyway. but keeping the API as simple as possible while it persists is a good thing, and may reduce confusion while QPDB is being developed from RBTDB code. --- lib/dns/include/dns/rbt.h | 155 ++--------------------------- lib/dns/rbt.c | 163 +++++------------------------- lib/dns/rbtdb.c | 2 +- tests/bench/load-names.c | 23 ++++- tests/dns/rbt_test.c | 203 ++++++++++++-------------------------- 5 files changed, 113 insertions(+), 433 deletions(-) diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h index c37a3a98b3..766166467a 100644 --- a/lib/dns/include/dns/rbt.h +++ b/lib/dns/include/dns/rbt.h @@ -30,7 +30,7 @@ ISC_LANG_BEGINDECLS /*@{*/ /*% - * Option values for dns_rbt_findnode() and dns_rbt_findname(). + * Option values for dns_rbt_findnode(). * These are used to form a bitmask. */ #define DNS_RBTFIND_NOOPTIONS 0x00 @@ -144,13 +144,6 @@ typedef isc_result_t (*dns_rbtfindcallback_t)(dns_rbtnode_t *node, dns_name_t *name, void *callback_arg DNS__DB_FLARG); -typedef isc_result_t (*dns_rbtdatawriter_t)(FILE *file, unsigned char *data, - void *arg, uint64_t *crc); - -typedef isc_result_t (*dns_rbtdatafixer_t)(dns_rbtnode_t *rbtnode, void *base, - size_t offset, void *arg, - uint64_t *crc); - typedef void (*dns_rbtdeleter_t)(void *, void *); /***** @@ -276,54 +269,15 @@ dns_rbt_create(isc_mem_t *mctx, dns_rbtdeleter_t deleter, void *deleter_arg, * * Returns: *\li #ISC_R_SUCCESS Success - *\li #ISC_R_NOMEMORY Resource limit: Out of Memory - */ - -isc_result_t -dns_rbt_addname(dns_rbt_t *rbt, const dns_name_t *name, void *data); -/*%< - * Add 'name' to the tree of trees, associated with 'data'. - * - * Notes: - *\li 'data' is never required to be non-NULL, but specifying it - * when the name is added is faster than searching for 'name' - * again and then setting the data pointer. The lack of a data pointer - * for a node also has other ramifications regarding whether - * dns_rbt_findname considers a node to exist, or dns_rbt_deletename - * joins nodes. - * - * Requires: - *\li rbt is a valid rbt manager. - *\li dns_name_isabsolute(name) == TRUE - * - * Ensures: - *\li 'name' is not altered in any way. - * - *\li Any external references to nodes in the tree are unaffected by - * node splits that are necessary to insert the new name. - * - *\li If result is #ISC_R_SUCCESS: - * 'name' is findable in the red/black tree of trees in O(log N). - * The data pointer of the node for 'name' is set to 'data'. - * - *\li If result is #ISC_R_EXISTS or #ISC_R_NOSPACE: - * The tree of trees is unaltered. - * - *\li If result is #ISC_R_NOMEMORY: - * No guarantees. - * - * Returns: - *\li #ISC_R_SUCCESS Success - *\li #ISC_R_EXISTS The name already exists with associated data. - *\li #ISC_R_NOSPACE The name had more logical labels than are allowed. - *\li #ISC_R_NOMEMORY Resource Limit: Out of Memory */ isc_result_t dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep); /*%< - * Just like dns_rbt_addname, but returns the address of the node. + * Add 'name' to the tree of trees. On success, return the address of + * the newly added node. If 'name' already existed, return ISC_R_EXISTS + * and the address of the pre-existing node. * * Requires: *\li rbt is a valid rbt structure. @@ -344,61 +298,16 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep); * The tree of trees is unaltered. * *nodep is the existing node for 'name'. * - *\li If result is ISC_R_NOMEMORY: - * No guarantees. - * * Returns: *\li #ISC_R_SUCCESS Success *\li #ISC_R_EXISTS The name already exists, possibly without data. - *\li #ISC_R_NOMEMORY Resource Limit: Out of Memory - */ - -#define dns_rbt_findname(rbt, name, options, foundname, data) \ - dns__rbt_findname(rbt, name, options, foundname, data DNS__DB_FILELINE) -isc_result_t -dns__rbt_findname(dns_rbt_t *rbt, const dns_name_t *name, unsigned int options, - dns_name_t *foundname, void **data DNS__DB_FLARG); -/*%< - * Get the data pointer associated with 'name'. - * - * Notes: - *\li When #DNS_RBTFIND_NOEXACT is set, the closest matching superdomain is - * returned (also subject to #DNS_RBTFIND_EMPTYDATA), even when there is - * an exact match in the tree. - * - *\li A node that has no data is considered not to exist for this function, - * unless the #DNS_RBTFIND_EMPTYDATA option is set. - * - * Requires: - *\li rbt is a valid rbt manager. - *\li dns_name_isabsolute(name) == TRUE - *\li data != NULL && *data == NULL - * - * Ensures: - *\li 'name' and the tree are not altered in any way. - * - *\li If result is ISC_R_SUCCESS: - * *data is the data associated with 'name'. - * - *\li If result is DNS_R_PARTIALMATCH: - * *data is the data associated with the deepest superdomain - * of 'name' which has data. - * - *\li If result is ISC_R_NOTFOUND: - * Neither the name nor a superdomain was found with data. - * - * Returns: - *\li #ISC_R_SUCCESS Success - *\li #DNS_R_PARTIALMATCH Superdomain found with data - *\li #ISC_R_NOTFOUND No match - *\li #ISC_R_NOSPACE Concatenating nodes to form foundname failed + *\li #ISC_R_NOSPACE The name had more logical labels than are allowed. */ #define dns_rbt_findnode(rbt, name, foundname, node, chain, options, callback, \ callback_arg) \ dns__rbt_findnode(rbt, name, foundname, node, chain, options, \ callback, callback_arg DNS__DB_FILELINE) - isc_result_t dns__rbt_findnode(dns_rbt_t *rbt, const dns_name_t *name, dns_name_t *foundname, dns_rbtnode_t **node, dns_rbtnodechain_t *chain, @@ -504,52 +413,6 @@ dns__rbt_findnode(dns_rbt_t *rbt, const dns_name_t *name, dns_name_t *foundname, *\li #ISC_R_NOSPACE Concatenating nodes to form foundname failed */ -#define dns_rbt_deletename(rbt, name, recurse) \ - dns__rbt_deletename(rbt, name, recurse DNS__DB_FILELINE) -isc_result_t -dns__rbt_deletename(dns_rbt_t *rbt, const dns_name_t *name, - bool recurse DNS__DB_FLARG); -/*%< - * Delete 'name' from the tree of trees. - * - * Notes: - *\li When 'name' is removed, if recurse is true then all of its - * subnames are removed too. - * - * Requires: - *\li rbt is a valid rbt manager. - *\li dns_name_isabsolute(name) == TRUE - * - * Ensures: - *\li 'name' is not altered in any way. - * - *\li Does NOT ensure that any external references to nodes in the tree - * are unaffected by node joins. - * - *\li If result is ISC_R_SUCCESS: - * 'name' does not appear in the tree with data; however, - * the node for the name might still exist which can be - * found with dns_rbt_findnode (but not dns_rbt_findname). - * - *\li If result is ISC_R_NOTFOUND: - * 'name' does not appear in the tree with data, because - * it did not appear in the tree before the function was called. - * - *\li If result is something else: - * See result codes for dns_rbt_findnode (if it fails, the - * node is not deleted) or dns_rbt_deletenode (if it fails, - * the node is deleted, but the tree is not optimized when - * it could have been). - * - * Returns: - *\li #ISC_R_SUCCESS Success - *\li #ISC_R_NOTFOUND No match - *\li something_else Any return code from dns_rbt_findnode except - * DNS_R_PARTIALMATCH (which causes ISC_R_NOTFOUND - * to be returned instead), and any code from - * dns_rbt_deletenode. - */ - isc_result_t dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, bool recurse); /*%< @@ -574,13 +437,12 @@ dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, bool recurse); * the node could can be found with dns_rbt_findnode when * that function's empty_data_ok parameter is true. * - *\li If result is ISC_R_NOMEMORY or ISC_R_NOSPACE: + *\li If result is ISC_R_NOSPACE: * The node was deleted, but the tree structure was not * optimized. * * Returns: *\li #ISC_R_SUCCESS Success - *\li #ISC_R_NOMEMORY Resource Limit: Out of Memory when joining nodes. *\li #ISC_R_NOSPACE dns_name_concatenate failed when joining nodes. */ @@ -664,10 +526,8 @@ dns_rbt_hashsize(dns_rbt_t *rbt); * \li rbt is a valid rbt manager. */ -void -dns_rbt_destroy(dns_rbt_t **rbtp); isc_result_t -dns_rbt_destroy2(dns_rbt_t **rbtp, unsigned int quantum); +dns_rbt_destroy(dns_rbt_t **rbtp, unsigned int quantum); /*%< * Stop working with a red-black tree of trees. * If 'quantum' is zero then the entire tree will be destroyed. @@ -902,7 +762,6 @@ dns_rbtnodechain_last(dns_rbtnodechain_t *chain, dns_rbt_t *rbt, * * Returns: *\li #DNS_R_NEWORIGIN The name & origin were successfully set. - *\li #ISC_R_NOMEMORY Resource Limit: Out of Memory building chain. *\li <something_else> Any error result from dns_name_concatenate. */ diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index cfcda98f95..e4b5b5d6ef 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -178,8 +178,8 @@ dns__rbtnode_getdistance(dns_rbtnode_t *node) { /* * Forward declarations. */ -static isc_result_t -create_node(isc_mem_t *mctx, const dns_name_t *name, dns_rbtnode_t **nodep); +static dns_rbtnode_t * +rbtnode_new(isc_mem_t *mctx, const dns_name_t *name); static void hashtable_new(dns_rbt_t *rbt, uint8_t index, uint8_t bits); @@ -291,13 +291,8 @@ dns_rbt_create(isc_mem_t *mctx, dns_rbtdeleter_t deleter, void *deleter_arg, /* * Deallocate a red/black tree of trees. */ -void -dns_rbt_destroy(dns_rbt_t **rbtp) { - RUNTIME_CHECK(dns_rbt_destroy2(rbtp, 0) == ISC_R_SUCCESS); -} - isc_result_t -dns_rbt_destroy2(dns_rbt_t **rbtp, unsigned int quantum) { +dns_rbt_destroy(dns_rbt_t **rbtp, unsigned int quantum) { dns_rbt_t *rbt; REQUIRE(rbtp != NULL && VALID_RBT(*rbtp)); @@ -459,18 +454,14 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep) { dns_name_clone(name, add_name); if (rbt->root == NULL) { - result = create_node(rbt->mctx, add_name, &new_current); - if (result == ISC_R_SUCCESS) { - rbt->nodecount++; - new_current->is_root = 1; - - new_current->uppernode = NULL; - - rbt->root = new_current; - *nodep = new_current; - hash_node(rbt, new_current, name); - } - return (result); + new_current = rbtnode_new(rbt->mctx, add_name); + rbt->nodecount++; + new_current->is_root = 1; + new_current->uppernode = NULL; + rbt->root = new_current; + *nodep = new_current; + hash_node(rbt, new_current, name); + return (ISC_R_SUCCESS); } level_count = 0; @@ -583,12 +574,7 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep) { */ dns_name_split(¤t_name, common_labels, prefix, suffix); - result = create_node(rbt->mctx, suffix, - &new_current); - - if (result != ISC_R_SUCCESS) { - break; - } + new_current = rbtnode_new(rbt->mctx, suffix); /* * Reproduce the tree attributes of the @@ -672,20 +658,16 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep) { * The current node has no data, * because it is just a placeholder. * Its data pointer is already NULL - * from create_node()), so there's + * from rbtnode_new()), so there's * nothing more to do to it. - */ - - /* + * * The not-in-common parts of the new * name will be inserted into the new - * level following this loop (unless - * result != ISC_R_SUCCESS, which - * is tested after the loop ends). + * level following this loop. */ dns_name_split(add_name, common_labels, add_name, NULL); - + result = ISC_R_SUCCESS; break; } } @@ -693,7 +675,7 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep) { } while (child != NULL); if (result == ISC_R_SUCCESS) { - result = create_node(rbt->mctx, add_name, &new_current); + new_current = rbtnode_new(rbt->mctx, add_name); } if (result == ISC_R_SUCCESS) { @@ -712,37 +694,6 @@ dns_rbt_addnode(dns_rbt_t *rbt, const dns_name_t *name, dns_rbtnode_t **nodep) { return (result); } -/* - * Add a name to the tree of trees, associating it with some data. - */ -isc_result_t -dns_rbt_addname(dns_rbt_t *rbt, const dns_name_t *name, void *data) { - isc_result_t result; - dns_rbtnode_t *node; - - REQUIRE(VALID_RBT(rbt)); - REQUIRE(dns_name_isabsolute(name)); - - node = NULL; - - result = dns_rbt_addnode(rbt, name, &node); - - /* - * dns_rbt_addnode will report the node exists even when - * it does not have data associated with it, but the - * dns_rbt_*name functions all behave depending on whether - * there is data associated with a node. - */ - if (result == ISC_R_SUCCESS || - (result == ISC_R_EXISTS && node != NULL && node->data == NULL)) - { - node->data = data; - result = ISC_R_SUCCESS; - } - - return (result); -} - /* * Find the node for "name" in the tree of trees. */ @@ -1274,72 +1225,6 @@ dns__rbt_findnode(dns_rbt_t *rbt, const dns_name_t *name, dns_name_t *foundname, return (result); } -/* - * Get the data pointer associated with 'name'. - */ -isc_result_t -dns__rbt_findname(dns_rbt_t *rbt, const dns_name_t *name, unsigned int options, - dns_name_t *foundname, void **data DNS__DB_FLARG) { - dns_rbtnode_t *node = NULL; - isc_result_t result; - - REQUIRE(data != NULL && *data == NULL); - - result = dns__rbt_findnode(rbt, name, foundname, &node, NULL, options, - NULL, NULL DNS__DB_FLARG_PASS); - - if (node != NULL && WANTEMPTYDATA_OR_DATA(options, node)) { - *data = node->data; - } else { - result = ISC_R_NOTFOUND; - } - - return (result); -} - -/* - * Delete a name from the tree of trees. - */ -isc_result_t -dns__rbt_deletename(dns_rbt_t *rbt, const dns_name_t *name, - bool recurse DNS__DB_FLARG) { - dns_rbtnode_t *node = NULL; - isc_result_t result; - - REQUIRE(VALID_RBT(rbt)); - REQUIRE(dns_name_isabsolute(name)); - - /* - * First, find the node. - * - * When searching, the name might not have an exact match: - * consider a.b.a.com, b.b.a.com and c.b.a.com as the only - * elements of a tree, which would make layer 1 a single - * node tree of "b.a.com" and layer 2 a three node tree of - * a, b, and c. Deleting a.com would find only a partial depth - * match in the first layer. Should it be a requirement that - * that the name to be deleted have data? For now, it is. - * - * ->dirty, ->locknum and ->references are ignored; they are - * solely the province of rbtdb.c. - */ - result = dns__rbt_findnode(rbt, name, NULL, &node, NULL, - DNS_RBTFIND_NOOPTIONS, NULL, - NULL DNS__DB_FLARG_PASS); - - if (result == ISC_R_SUCCESS) { - if (node->data != NULL) { - result = dns_rbt_deletenode(rbt, node, recurse); - } else { - result = ISC_R_NOTFOUND; - } - } else if (result == DNS_R_PARTIALMATCH) { - result = ISC_R_NOTFOUND; - } - - return (result); -} - /* * Remove a node from the tree of trees. * @@ -1497,9 +1382,9 @@ dns_rbt_formatnodename(dns_rbtnode_t *node, char *printname, return (printname); } -static isc_result_t -create_node(isc_mem_t *mctx, const dns_name_t *name, dns_rbtnode_t **nodep) { - dns_rbtnode_t *node; +static dns_rbtnode_t * +rbtnode_new(isc_mem_t *mctx, const dns_name_t *name) { + dns_rbtnode_t *node = NULL; isc_region_t region; unsigned int labels; size_t nodelen; @@ -1548,9 +1433,7 @@ create_node(isc_mem_t *mctx, const dns_name_t *name, dns_rbtnode_t **nodep) { #if DNS_RBT_USEMAGIC node->magic = DNS_RBTNODE_MAGIC; #endif /* if DNS_RBT_USEMAGIC */ - *nodep = node; - - return (ISC_R_SUCCESS); + return (node); } /* @@ -2565,9 +2448,7 @@ dns_rbtnodechain_current(dns_rbtnodechain_t *chain, dns_name_t *name, REQUIRE(VALID_CHAIN(chain)); - if (node != NULL) { - *node = chain->end; - } + SET_IF_NOT_NULL(node, chain->end); if (chain->end == NULL) { return (ISC_R_NOTFOUND); diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 06ac203308..405d6bff5a 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -516,7 +516,7 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log) { } start = isc_time_now(); - result = dns_rbt_destroy2(treep, rbtdb->quantum); + result = dns_rbt_destroy(treep, rbtdb->quantum); if (result == ISC_R_QUOTA) { INSIST(rbtdb->loop != NULL); if (rbtdb->quantum != 0) { diff --git a/tests/bench/load-names.c b/tests/bench/load-names.c index bf41222311..70d7d07443 100644 --- a/tests/bench/load-names.c +++ b/tests/bench/load-names.c @@ -311,15 +311,30 @@ new_rbt(isc_mem_t *mem) { static isc_result_t add_rbt(void *rbt, size_t count) { - isc_result_t result = dns_rbt_addname(rbt, &item[count].fixed.name, - &item[count]); + isc_result_t result; + dns_rbtnode_t *node = NULL; + + result = dns_rbt_addnode(rbt, &item[count].fixed.name, &node); + if (result == ISC_R_SUCCESS || + (result == ISC_R_EXISTS && node->data == NULL)) + { + node->data = &item[count]; + result = ISC_R_SUCCESS; + } + return (result); } static isc_result_t get_rbt(void *rbt, size_t count, void **pval) { - isc_result_t result = dns_rbt_findname(rbt, &item[count].fixed.name, 0, - NULL, pval); + isc_result_t result; + dns_rbtnode_t *node = NULL; + + result = dns_rbt_findnode(rbt, &item[count].fixed.name, NULL, &node, + NULL, 0, NULL, NULL); + if (result == ISC_R_SUCCESS) { + *pval = node->data; + } return (result); } diff --git a/tests/dns/rbt_test.c b/tests/dns/rbt_test.c index f3c10e5cc4..57f45926f6 100644 --- a/tests/dns/rbt_test.c +++ b/tests/dns/rbt_test.c @@ -150,7 +150,8 @@ test_context_setup(void) { for (i = 0; i < domain_names_count; i++) { size_t *n; dns_fixedname_t fname; - dns_name_t *name; + dns_name_t *name = NULL; + dns_rbtnode_t *node = NULL; dns_test_namefromstring(domain_names[i], &fname); @@ -159,13 +160,16 @@ test_context_setup(void) { n = isc_mem_get(mctx, sizeof(size_t)); assert_non_null(n); *n = i + 1; - result = dns_rbt_addname(ctx->rbt, name, n); + result = dns_rbt_addnode(ctx->rbt, name, &node); assert_int_equal(result, ISC_R_SUCCESS); + node->data = n; + node = NULL; n = isc_mem_get(mctx, sizeof(size_t)); assert_non_null(n); *n = node_distances[i]; - result = dns_rbt_addname(ctx->rbt_distances, name, n); + result = dns_rbt_addnode(ctx->rbt_distances, name, &node); + node->data = n; assert_int_equal(result, ISC_R_SUCCESS); } @@ -174,8 +178,8 @@ test_context_setup(void) { static void test_context_teardown(test_context_t *ctx) { - dns_rbt_destroy(&ctx->rbt); - dns_rbt_destroy(&ctx->rbt_distances); + dns_rbt_destroy(&ctx->rbt, 0); + dns_rbt_destroy(&ctx->rbt_distances, 0); isc_mem_put(mctx, ctx, sizeof(*ctx)); } @@ -194,15 +198,18 @@ check_test_data(dns_rbt_t *rbt) { for (i = 0; i < domain_names_count; i++) { dns_fixedname_t fname; - dns_name_t *name; - size_t *n; + dns_rbtnode_t *node = NULL; + dns_name_t *name = NULL; + size_t *n = NULL; dns_test_namefromstring(domain_names[i], &fname); name = dns_fixedname_name(&fname); n = NULL; - result = dns_rbt_findname(rbt, name, 0, foundname, (void *)&n); + result = dns_rbt_findnode(rbt, name, foundname, &node, NULL, 0, + NULL, NULL); assert_int_equal(result, ISC_R_SUCCESS); + n = node->data; assert_int_equal(*n, i + 1); } } @@ -311,7 +318,7 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_random) { * yueojmhyffslpvfmgyfwioxegfhepnqq. */ for (i = 0; i < (1 << log_num_nodes); i++) { - size_t *n; + size_t *n = NULL; char namebuf[34]; n = isc_mem_get(mctx, sizeof(size_t)); @@ -321,7 +328,8 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_random) { while (1) { int j; dns_fixedname_t fname; - dns_name_t *name; + dns_rbtnode_t *node = NULL; + dns_name_t *name = NULL; for (j = 0; j < 32; j++) { uint32_t v = isc_random_uniform(26); @@ -333,7 +341,8 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_random) { dns_test_namefromstring(namebuf, &fname); name = dns_fixedname_name(&fname); - result = dns_rbt_addname(mytree, name, n); + result = dns_rbt_addnode(mytree, name, &node); + node->data = n; if (result == ISC_R_SUCCESS) { break; } @@ -352,7 +361,7 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_random) { tree_ok = dns__rbt_checkproperties(mytree); assert_true(tree_ok); - dns_rbt_destroy(&mytree); + dns_rbt_destroy(&mytree, 0); } /* @@ -389,7 +398,8 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_ordered) { size_t *n; char namebuf[14]; dns_fixedname_t fname; - dns_name_t *name; + dns_name_t *name = NULL; + dns_rbtnode_t *node = NULL; n = isc_mem_get(mctx, sizeof(size_t)); assert_non_null(n); @@ -399,8 +409,9 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_ordered) { dns_test_namefromstring(namebuf, &fname); name = dns_fixedname_name(&fname); - result = dns_rbt_addname(mytree, name, n); + result = dns_rbt_addnode(mytree, name, &node); assert_int_equal(result, ISC_R_SUCCESS); + node->data = n; } /* 1 (root . node) + (1 << log_num_nodes) */ @@ -415,7 +426,7 @@ ISC_RUN_TEST_IMPL(rbt_check_distance_ordered) { tree_ok = dns__rbt_checkproperties(mytree); assert_true(tree_ok); - dns_rbt_destroy(&mytree); + dns_rbt_destroy(&mytree, 0); } static isc_result_t @@ -610,6 +621,26 @@ ISC_RUN_TEST_IMPL(rbt_insert) { * as a red-black tree. This test checks node deletion when upper nodes * have data. */ +static isc_result_t +deletename(dns_rbt_t *mytree, const dns_name_t *name) { + isc_result_t result; + dns_rbtnode_t *node = NULL; + + result = dns_rbt_findnode(mytree, name, NULL, &node, NULL, 0, NULL, + NULL); + if (result == ISC_R_SUCCESS) { + if (node->data != NULL) { + result = dns_rbt_deletenode(mytree, node, false); + } else { + result = ISC_R_NOTFOUND; + } + } else if (result == DNS_R_PARTIALMATCH) { + result = ISC_R_NOTFOUND; + } + + return (result); +} + ISC_RUN_TEST_IMPL(rbt_remove) { isc_result_t result; size_t j; @@ -673,7 +704,7 @@ ISC_RUN_TEST_IMPL(rbt_remove) { name = dns_fixedname_name(&fname); - result = dns_rbt_deletename(mytree, name, false); + result = deletename(mytree, name); assert_int_equal(result, ISC_R_SUCCESS); } @@ -783,7 +814,7 @@ ISC_RUN_TEST_IMPL(rbt_remove) { /* We should have reached the end of the tree. */ assert_null(node); - dns_rbt_destroy(&mytree); + dns_rbt_destroy(&mytree, 0); } } @@ -840,22 +871,22 @@ remove_nodes(dns_rbt_t *mytree, char **names, size_t *names_count, UNUSED(mytree); for (i = 0; i < num_names; i++) { - uint32_t node; - dns_fixedname_t fname; - dns_name_t *name; isc_result_t result; + dns_fixedname_t fname; + dns_name_t *name = NULL; + uint32_t num; - node = isc_random_uniform(*names_count); + num = isc_random_uniform(*names_count); - dns_test_namefromstring(names[node], &fname); + dns_test_namefromstring(names[num], &fname); name = dns_fixedname_name(&fname); - result = dns_rbt_deletename(mytree, name, false); + result = deletename(mytree, name); assert_int_equal(result, ISC_R_SUCCESS); - isc_mem_free(mctx, names[node]); + isc_mem_free(mctx, names[num]); if (*names_count > 0) { - names[node] = names[*names_count - 1]; + names[num] = names[*names_count - 1]; names[*names_count - 1] = NULL; *names_count -= 1; } @@ -901,7 +932,8 @@ check_tree(dns_rbt_t *mytree, char **names, size_t names_count) { ISC_RUN_TEST_IMPL(rbt_insert_and_remove) { isc_result_t result; dns_rbt_t *mytree = NULL; - size_t *n; + size_t *n = NULL; + dns_rbtnode_t *node = NULL; char *names[1024]; size_t names_count; int i; @@ -913,8 +945,9 @@ ISC_RUN_TEST_IMPL(rbt_insert_and_remove) { n = isc_mem_get(mctx, sizeof(size_t)); assert_non_null(n); - result = dns_rbt_addname(mytree, dns_rootname, n); + result = dns_rbt_addnode(mytree, dns_rootname, &node); assert_int_equal(result, ISC_R_SUCCESS); + node->data = n; memset(names, 0, sizeof(names)); names_count = 0; @@ -954,116 +987,11 @@ ISC_RUN_TEST_IMPL(rbt_insert_and_remove) { } } - result = dns_rbt_deletename(mytree, dns_rootname, false); + result = deletename(mytree, dns_rootname); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(dns_rbt_nodecount(mytree), 0); - dns_rbt_destroy(&mytree); -} - -/* Test findname return values */ -ISC_RUN_TEST_IMPL(rbt_findname) { - isc_result_t result; - test_context_t *ctx = NULL; - dns_fixedname_t fname, found; - dns_name_t *name = NULL, *foundname = NULL; - size_t *n = NULL; - - isc_mem_debugging = ISC_MEM_DEBUGRECORD; - - ctx = test_context_setup(); - - /* Try to find a name that exists. */ - dns_test_namefromstring("d.e.f.", &fname); - name = dns_fixedname_name(&fname); - - foundname = dns_fixedname_initname(&found); - - result = dns_rbt_findname(ctx->rbt, name, DNS_RBTFIND_EMPTYDATA, - foundname, (void *)&n); - assert_true(dns_name_equal(foundname, name)); - assert_int_equal(result, ISC_R_SUCCESS); - - /* Now without EMPTYDATA */ - result = dns_rbt_findname(ctx->rbt, name, 0, foundname, (void *)&n); - assert_int_equal(result, ISC_R_NOTFOUND); - - /* Now one that partially matches */ - dns_test_namefromstring("d.e.f.g.h.i.j.", &fname); - name = dns_fixedname_name(&fname); - result = dns_rbt_findname(ctx->rbt, name, DNS_RBTFIND_EMPTYDATA, - foundname, (void *)&n); - assert_int_equal(result, DNS_R_PARTIALMATCH); - - /* Now one that doesn't match */ - dns_test_namefromstring("1.2.", &fname); - name = dns_fixedname_name(&fname); - result = dns_rbt_findname(ctx->rbt, name, DNS_RBTFIND_EMPTYDATA, - foundname, (void *)&n); - assert_int_equal(result, DNS_R_PARTIALMATCH); - assert_true(dns_name_equal(foundname, dns_rootname)); - - test_context_teardown(ctx); -} - -/* Test addname return values */ -ISC_RUN_TEST_IMPL(rbt_addname) { - isc_result_t result; - test_context_t *ctx = NULL; - dns_fixedname_t fname; - dns_name_t *name = NULL; - size_t *n; - - isc_mem_debugging = ISC_MEM_DEBUGRECORD; - - ctx = test_context_setup(); - - n = isc_mem_get(mctx, sizeof(size_t)); - assert_non_null(n); - *n = 1; - - dns_test_namefromstring("d.e.f.g.h.i.j.k.", &fname); - name = dns_fixedname_name(&fname); - - /* Add a name that doesn't exist */ - result = dns_rbt_addname(ctx->rbt, name, n); - assert_int_equal(result, ISC_R_SUCCESS); - - /* Now add again, should get ISC_R_EXISTS */ - n = isc_mem_get(mctx, sizeof(size_t)); - assert_non_null(n); - *n = 2; - result = dns_rbt_addname(ctx->rbt, name, n); - assert_int_equal(result, ISC_R_EXISTS); - isc_mem_put(mctx, n, sizeof(size_t)); - - test_context_teardown(ctx); -} - -/* Test deletename return values */ -ISC_RUN_TEST_IMPL(rbt_deletename) { - isc_result_t result; - test_context_t *ctx = NULL; - dns_fixedname_t fname; - dns_name_t *name = NULL; - - isc_mem_debugging = ISC_MEM_DEBUGRECORD; - - ctx = test_context_setup(); - - /* Delete a name that doesn't exist */ - dns_test_namefromstring("z.x.y.w.", &fname); - name = dns_fixedname_name(&fname); - result = dns_rbt_deletename(ctx->rbt, name, false); - assert_int_equal(result, ISC_R_NOTFOUND); - - /* Now one that does */ - dns_test_namefromstring("d.e.f.", &fname); - name = dns_fixedname_name(&fname); - result = dns_rbt_deletename(ctx->rbt, name, false); - assert_int_equal(result, ISC_R_NOTFOUND); - - test_context_teardown(ctx); + dns_rbt_destroy(&mytree, 0); } /* Test nodechain */ @@ -1124,7 +1052,7 @@ ISC_RUN_TEST_IMPL(rbt_nodechain) { test_context_teardown(ctx); } -/* Test addname return values */ +/* Test name lengths */ ISC_RUN_TEST_IMPL(rbtnode_namelen) { isc_result_t result; test_context_t *ctx = NULL; @@ -1270,7 +1198,7 @@ ISC_RUN_TEST_IMPL(benchmark) { free(names); free(fnames); - dns_rbt_destroy(&mytree); + dns_rbt_destroy(&mytree, 0); } #endif /* defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__) */ @@ -1283,9 +1211,6 @@ ISC_TEST_ENTRY(rbt_check_distance_ordered) ISC_TEST_ENTRY(rbt_insert) ISC_TEST_ENTRY(rbt_remove) ISC_TEST_ENTRY(rbt_insert_and_remove) -ISC_TEST_ENTRY(rbt_findname) -ISC_TEST_ENTRY(rbt_addname) -ISC_TEST_ENTRY(rbt_deletename) ISC_TEST_ENTRY(rbt_nodechain) ISC_TEST_ENTRY(rbtnode_namelen) #if defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__)