From: Yuri Schaeffer Date: Tue, 24 Sep 2013 19:11:20 +0000 (+0000) Subject: extending vandergaast unittest, found a bug or two. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=07ee7a375c7cda523ec564674afac9ae18a8b6bb;p=thirdparty%2Funbound.git extending vandergaast unittest, found a bug or two. git-svn-id: file:///svn/unbound/branches/edns-subnet@2969 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/edns-subnet/addrtree.c b/edns-subnet/addrtree.c index d6a6a71f7..103c00814 100644 --- a/edns-subnet/addrtree.c +++ b/edns-subnet/addrtree.c @@ -29,7 +29,7 @@ edge_create(struct addrnode* node, const addrkey_t* addr, addrlen_t addrlen) return NULL; edge->node = node; edge->len = addrlen; - n = (size_t)((addrlen / KEYWIDTH) + ((addrlen % KEYWIDTH)!=0)?1:0); /*ceil()*/ + n = (size_t)((addrlen / KEYWIDTH) + ((addrlen % KEYWIDTH != 0)?1:0)); /*ceil()*/ edge->str = (addrkey_t*)calloc(n, sizeof(addrkey_t)); if (!edge->str) { free(edge); @@ -113,7 +113,7 @@ size_t addrtree_size(const struct addrtree* tree) void addrtree_clean_node(const struct addrtree* tree, struct addrnode* node) { if (node->elem) { - reply_info_delete(node->elem, NULL); + reply_info_delete(node->elem, NULL); //YBS niet NULL: regel 244 node->elem = NULL; } } @@ -143,6 +143,10 @@ freenode_recursive(struct addrtree* tree, struct addrnode* node) free(node); } +/** + * Free complete addrtree structure + * @param tree: Tree to free + */ void addrtree_delete(struct addrtree* tree) { if (tree) { @@ -152,7 +156,12 @@ void addrtree_delete(struct addrtree* tree) } } -/** Get N'th bit from address */ +/** Get N'th bit from address + * @param addr: address to inspect + * @param addrlen: length of addr in bits + * @param n: index of bit to test. Must be in range [0, addrlen) + * @return 0 or 1 + */ static int getbit(const addrkey_t* addr, addrlen_t addrlen, addrlen_t n) { @@ -212,6 +221,14 @@ void addrtree_insert(struct addrtree* tree, const addrkey_t* addr, addrlen_t sourcemask, addrlen_t scope, struct reply_info* elem) { + /*TODO: + * problem: code might delete elem so effectively owns elem. + * but fail is silent and elem may leak. + * plan return NULL or elem on insert to let caller decide. + * Also try again to make tree agnostic of elem type. We need a + * elem_size callback and clean_tree might have to return a list + * of elem or require delete callback */ + struct addrnode* newnode, *node; struct addredge* edge, *newedge; uint8_t index; diff --git a/testcode/unitvandergaast.c b/testcode/unitvandergaast.c index 4881fe097..aca2271d1 100644 --- a/testcode/unitvandergaast.c +++ b/testcode/unitvandergaast.c @@ -44,9 +44,122 @@ #ifdef CLIENT_SUBNET #include "util/log.h" +#include "util/module.h" #include "testcode/unitmain.h" #include "edns-subnet/addrtree.h" +/* + void printkey(addrkey_t *k, addrlen_t bits) + { + int byte; + int bytes = bits/8 + ((bits%8)>0); + for (byte = 0; byte < bytes; byte++) { + printf("%02x ", k[byte]); + } + } + + void print_tree(struct addrnode* node, int indent) + { + struct addredge* edge; + int i, s, byte; + if (indent == 0) printf("-----Tree-----"); + printf("[node elem:%d]\n", node->elem != NULL); + for (i = 0; i<2; i++) { + if (node->edge[i]) { + for (s = 0; s < indent; s++) printf(" "); + printkey(node->edge[i]->str, node->edge[i]->len); + printf("(len %d bits, %d bytes) ", node->edge[i]->len, + node->edge[i]->len/8 + ((node->edge[i]->len%8)>0)); + print_tree(node->edge[i]->node, indent+1); + } + } + if (indent == 0) printf("-----Tree-----"); + } +*/ + +/* what should we check? + * X - is it balanced? (a node with 1 child shoudl not have + * a node with 1 child MUST have elem + * child must be sub of parent + * edge must be longer than parent edge + * */ +static int addrtree_inconsistent_subtree(struct addredge* parent_edge) +{ + struct addredge* edge; + struct addrnode* node = parent_edge->node; + int childcount, i, r; + + childcount = (node->edge[0] != NULL) + (node->edge[1] != NULL); + /* Only nodes with 2 children should possibly have no element. */ + if (childcount < 2 && !node->elem) return 10; + for (i = 0; i<2; i++) { + edge = node->edge[i]; + if (!edge) continue; + if (!edge->node) return 11; + if (!edge->str) return 12; + if (edge->len <= parent_edge->len) return 13; + if (!unittest_wrapper_addrtree_issub(parent_edge->str, + parent_edge->len, edge->str, edge->len, 0)) + return 14; + if ((r = addrtree_inconsistent_subtree(edge)) != 0) + return 15+r; + } + return 0; +} + +static int addrtree_inconsistent(struct addrtree* tree) +{ + struct addredge* edge; + int i, r; + + if (!tree) return 0; + if (!tree->root) return 1; + + for (i = 0; i<2; i++) { + edge = tree->root->edge[i]; + if (!edge) continue; + if (!edge->node) return 3; + if (!edge->str) return 4; + if ((r = addrtree_inconsistent_subtree(edge)) != 0) + return r; + } + return 0; +} + +static int randomkey(addrkey_t **k, int maxlen) +{ + int byte; + int bits = rand() % maxlen; + int bytes = bits/8 + (bits%8>0); /*ceil*/ + *k = (addrkey_t *) malloc(bytes * sizeof(addrkey_t)); + for (byte = 0; byte < bytes; byte++) { + (*k)[byte] = (addrkey_t)(rand() & 0xFF); + } + return bits; +} + +static void consistency_test(void) +{ + int i, l, r; + addrkey_t *k; + struct addrtree* t; + struct module_env env; + struct reply_info *elem; + unit_show_func("edns-subnet/addrtree.h", "Tree consistency check"); + srand(9195); /* just some value for reproducibility */ + + env.alloc = NULL; + t = addrtree_create(100, &env); + + for (i = 0; i < 1000; i++) { + l = randomkey(&k, 128); + elem = (struct reply_info *) calloc(1, sizeof(struct reply_info)); + addrtree_insert(t, k, l, 64, elem); + free(k); + unit_assert( !addrtree_inconsistent(t) ); + } +} + static void issub_test(void) { unit_show_func("edns-subnet/addrtree.h", "issub"); @@ -116,6 +229,7 @@ void vandergaast_test(void) bits_common_test(); getbit_test(); issub_test(); + consistency_test(); } #endif /* CLIENT_SUBNET */