From 7111a0959f572f3541290c5871901fe56ab3666e Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 30 Oct 2013 00:29:30 +0100 Subject: [PATCH 1/5] A dict based on rbtree --- src/dict.c | 341 ++++++++++++++++------------------------------------- src/dict.h | 18 +-- 2 files changed, 112 insertions(+), 247 deletions(-) diff --git a/src/dict.c b/src/dict.c index bda4ef37..60a011ad 100644 --- a/src/dict.c +++ b/src/dict.c @@ -30,45 +30,14 @@ */ #include -#include #include #include "dict.h" -/* TODO: change this to make the walk safe for reentrant/multi-thread calls */ -struct getdns_list *walkresultlist; -struct getdns_dict *walkresultdict; -char *walkresultchar; -int walkresultcharlen; - /*---------------------------------------- getdns_dict_cmp */ -/** - * private function used by the t*() functions for managing binary trees - * behaves similar to strcmp() - * @param item1 pointer to pointer to getdns_dict_item to compare - * @param item2 pointer to pointer to getdns_dict_item to compare - * @return results of lexicographic comparison between item1->key and item2->key - */ int getdns_dict_cmp(const void *item1, const void *item2) { - int retval = 0; - - if(item1 == NULL) - { - if(item2 == NULL) - retval = 0; - else - retval = -1; - } - else if(item2 == NULL) - retval = 1; - else - { - retval = strcmp(((struct getdns_dict_item *) item1)->key - , ((struct getdns_dict_item *) item2)->key); - } - - return retval; + return strcmp((const char *)item1, (const char *)item2); } /* getdns_dict_comp */ /*---------------------------------------- getdns_dict_find */ @@ -83,100 +52,50 @@ getdns_dict_cmp(const void *item1, const void *item2) struct getdns_dict_item * getdns_dict_find(struct getdns_dict *dict, char *key, bool addifnotfnd) { - struct getdns_dict_item keyitem; - struct getdns_dict_item **item; - struct getdns_dict_item *newitem; - struct getdns_dict_item *ret = NULL; + struct getdns_dict_item *item = NULL; if(dict != NULL && key != NULL) { - /* we try to find it first, if we do then clear the existing data */ - keyitem.key = key; - keyitem.dtype = t_invalid; - keyitem.data.n = 0; - item = tfind(&keyitem, &(dict->rootp), getdns_dict_cmp); - if(addifnotfnd == true && (item == NULL || *item == NULL)) + item = (struct getdns_dict_item *)ldns_rbtree_search(&(dict->root), key); + if(addifnotfnd == true && item == NULL) { /* tsearch will add a node automatically for us */ - newitem = (struct getdns_dict_item *) malloc(sizeof(struct getdns_dict_item)); - newitem->key = strdup(key); - newitem->dtype = t_invalid; - newitem->data.n = 0; - item = tsearch(newitem, &(dict->rootp), getdns_dict_cmp); + item = (struct getdns_dict_item *) malloc(sizeof(struct getdns_dict_item)); + item->key = strdup(key); + item->node.key = item->key; + item->dtype = t_invalid; + item->data.n = 0; + ldns_rbtree_insert(&(dict->root), (ldns_rbnode_t *)item); } - if(item != NULL) - ret = *item; } - - return ret; + return item; } /* getdns_dict_find */ -/*---------------------------------------- getdns_dict_visit */ -/** - * private function called by the tree walk function invoked by getdns_dict_get_names - * it is called as each node is visited. twalk() calls 3x for each node and passes order - * to tell us whether it is a pre/in/post order - */ -void -getdns_dict_visit(const void *node, VISIT order, int level) -{ - struct getdns_dict_item *item; - size_t index; - - item = *(struct getdns_dict_item **) node; - /* postorder is mis-named - it results in in-order traversal */ - if(order == postorder || order == leaf) - { - if(getdns_list_add_item(walkresultlist, &index) == GETDNS_RETURN_GOOD) - { - switch(item->dtype) - { - case t_bindata: - getdns_list_set_bindata(walkresultlist, index, item->data.bindata); - break; - - case t_dict: - getdns_list_set_dict(walkresultlist, index, item->data.dict); - break; - - case t_int: - getdns_list_set_int(walkresultlist, index, item->data.n); - break; - - case t_list: - getdns_list_set_list(walkresultlist, index, item->data.list); - break; - - case t_invalid: - default: - // TODO: this is a fault of some kind, for now ignore it - break; - } - } - } - - return; -} /* getdns_dict_visit */ - /*---------------------------------------- getdns_dict_get_names - TODO: this needs to be made thread safe by creating a thread specific list - the binary search tree implementation in the */ getdns_return_t getdns_dict_get_names(struct getdns_dict *dict, struct getdns_list **answer) { getdns_return_t retval = GETDNS_RETURN_NO_SUCH_DICT_NAME; + struct getdns_dict_item *item; + size_t index; if(dict != NULL && answer != NULL) { *answer = getdns_list_create(); - walkresultlist = *answer; - - twalk(dict->rootp, getdns_dict_visit); + LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) + { + if(getdns_list_add_item(*answer, &index) == GETDNS_RETURN_GOOD) + { + struct getdns_bindata bindata; + bindata.size = strlen(item->key); + bindata.data = (void *)item->key; + getdns_list_set_bindata(*answer, index, &bindata); + } + } retval = GETDNS_RETURN_GOOD; } - return retval; } /* getdns_dict_get_names */ @@ -307,55 +226,11 @@ getdns_dict_create() struct getdns_dict *dict; dict = (struct getdns_dict *) malloc(sizeof(struct getdns_dict)); - dict->rootp = NULL; - + //ldns_rbtree_init(&(dict->root), getdns_dict_cmp); + ldns_rbtree_init(&(dict->root), (int (*)(const void *, const void *))strcmp); return dict; } /* getdns_dict_create */ -/*---------------------------------------- getdns_dict_visit_copyitem */ -/** - * private function called by getdns_dict_copy() through the tree walk function - * is called as each node is visited. twalk() calls 3x for each node and passes order - * to tell us whether it is a pre/in/post order. We use this to copy the dictionary one item at - * a time - this could be sped up - */ -void -getdns_dict_visit_copyitem(const void *node, VISIT order, int level) -{ - struct getdns_dict_item *item; - - item = *(struct getdns_dict_item **) node; - /* postorder is mis-named - it results in in-order traversal */ - if(order == postorder || order == leaf) - { - switch(item->dtype) - { - case t_bindata: - getdns_dict_set_bindata(walkresultdict, item->key, item->data.bindata); - break; - - case t_dict: - getdns_dict_set_dict(walkresultdict, item->key, item->data.dict); - break; - - case t_int: - getdns_dict_set_int(walkresultdict, item->key, item->data.n); - break; - - case t_list: - getdns_dict_set_list(walkresultdict, item->key, item->data.list); - break; - - case t_invalid: - default: - // TODO: this is a fault of some kind, for now ignore it - break; - } - } - - return; -} /* getdns_dict_visit_copyitem */ - /*---------------------------------------- getdns_dict_copy */ /** * private function used to make a copy of a dict structure, the caller is responsible @@ -370,14 +245,38 @@ getdns_return_t getdns_dict_copy(struct getdns_dict *srcdict, struct getdns_dict **dstdict) { getdns_return_t retval = GETDNS_RETURN_NO_SUCH_DICT_NAME; + struct getdns_dict_item *item; if(srcdict != NULL && dstdict != NULL) { *dstdict = getdns_dict_create(); - walkresultdict = *dstdict; - twalk(srcdict->rootp, getdns_dict_visit_copyitem); + LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(srcdict->root)) + { + switch(item->dtype) + { + case t_bindata: + getdns_dict_set_bindata(*dstdict, item->key, item->data.bindata); + break; + case t_dict: + getdns_dict_set_dict(*dstdict, item->key, item->data.dict); + break; + + case t_int: + getdns_dict_set_int(*dstdict, item->key, item->data.n); + break; + + case t_list: + getdns_dict_set_list(*dstdict, item->key, item->data.list); + break; + + case t_invalid: + default: + // TODO: this is a fault of some kind, for now ignore it + break; + } + } retval = GETDNS_RETURN_GOOD; } @@ -391,8 +290,10 @@ getdns_dict_copy(struct getdns_dict *srcdict, struct getdns_dict **dstdict) * @return void */ void -getdns_dict_item_free(struct getdns_dict_item *item) +getdns_dict_item_free(ldns_rbnode_t *node, void *arg) { + (void) arg; + struct getdns_dict_item *item = (struct getdns_dict_item *)node; if(item != NULL) { if(item->dtype == t_bindata) @@ -420,19 +321,9 @@ getdns_dict_item_free(struct getdns_dict_item *item) void getdns_dict_destroy(struct getdns_dict *dict) { - struct getdns_dict_item keyitem; - struct getdns_dict_item *item; - - if(dict != NULL && dict->rootp != NULL) + if(dict != NULL) { - while(dict->rootp != NULL) - { - item = *((struct getdns_dict_item **) dict->rootp); - keyitem.key = item->key; - tdelete(&keyitem, &(dict->rootp), getdns_dict_cmp); - getdns_dict_item_free(item); - } - + ldns_traverse_postorder(&(dict->root), getdns_dict_item_free, NULL); free(dict); } @@ -544,92 +435,62 @@ getdns_dict_set_int(struct getdns_dict *dict, char *name, uint32_t child_uint32) return retval; } /* getdns_dict_set_int */ -/*---------------------------------------- getdns_dict_visit_print */ -/** - * private function called by the tree walk function invoked by getdns_pretty_print_dict - * it is called as each node is visited. twalk() calls 3x for each node and passes order - * to tell us whether it is a pre/in/post order - * TODO: need to handle nested non-trivial data types - */ -void -getdns_dict_visit_print(const void *node, VISIT order, int level) -{ - struct getdns_dict_item *item; - int newlen; - char *dtypestr = NULL; - char *valstr = NULL; - char *itemstr = NULL; - - item = *(struct getdns_dict_item **) node; - /* postorder is mis-named - it results in in-order traversal */ - if(order == postorder || order == leaf) - { - switch(item->dtype) - { - case t_bindata: - dtypestr = "bindata"; - valstr = strdup("NOT IMPLEMENTED"); - break; - - case t_dict: - dtypestr = "dict"; - valstr = strdup("NOT IMPLEMENTED"); - break; - - case t_int: - dtypestr = "int"; - asprintf(&valstr, "%d", item->data.n); - break; - - case t_list: - dtypestr = "list"; - valstr = strdup("NOT IMPLEMENTED"); - break; - - case t_invalid: - default: - dtypestr = "invalid"; - valstr = strdup(""); - break; - } - - newlen = asprintf(&itemstr, "key=\"%s\", type=\"%s\", value=\"%s\"\n", item->key, dtypestr, valstr); - if(newlen != -1) - { - walkresultchar = (char *) realloc(walkresultchar, walkresultcharlen + newlen + 1); - memcpy(walkresultchar + walkresultcharlen, itemstr, newlen); - walkresultcharlen += newlen; - walkresultchar[walkresultcharlen] = '\0'; - } - // else - // TODO: this is a fault - do something - - free(valstr); - } - - return; -} /* getdns_dict_visit_print */ - /*---------------------------------------- getdns_pretty_print_dict */ char * getdns_pretty_print_dict(struct getdns_dict *dict) { - char *retval = NULL; + struct getdns_dict_item *item; + char buf[8192]; + size_t i; + char* tmp; - walkresultcharlen = 0; - walkresultchar = NULL; + buf[0] = 0; - if(dict != NULL && dict->rootp != NULL) + if(dict != NULL) { - twalk(dict->rootp, getdns_dict_visit_print); - if(walkresultcharlen > 0) + i = 0; + strcat(buf, "{"); + LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) { - retval = strdup(walkresultchar); - free(walkresultchar); + if (i) + strcat(buf, ", \""); + else + strcat(buf, " \""); + strcat(buf, item->node.key); + strcat(buf, "\": "); + switch(item->dtype) + { + case t_bindata: + sprintf(buf + strlen(buf), "", (int)item->data.bindata->size); + break; + + case t_dict: + tmp = getdns_pretty_print_dict(item->data.dict); + strcat(buf, tmp); + free(tmp); + break; + + case t_int: + sprintf(buf + strlen(buf), "%d", item->data.n); + break; + + case t_list: + strcat(buf, "[]"); + break; + + case t_invalid: + default: + strcat(buf, ""); + break; + } + i++; } + if(i) + strcat(buf, " "); + strcat(buf, "}"); } - return retval; + return strdup(buf); } /* getdns_pretty_print_dict */ /* getdns_dict.c */ diff --git a/src/dict.h b/src/dict.h index 8a684e8f..28820554 100644 --- a/src/dict.h +++ b/src/dict.h @@ -31,19 +31,23 @@ #define _GETDNS_DICT_H_ #include +#include + +union getdns_item { + struct getdns_list *list; + struct getdns_dict *dict; + int n; + struct getdns_bindata *bindata; +}; /** * this structure represents a single item in a dictionary type */ struct getdns_dict_item { + ldns_rbnode_t node; char *key; getdns_data_type dtype; - union { - struct getdns_list *list; - struct getdns_dict *dict; - int n; - struct getdns_bindata *bindata; - } data; + union getdns_item data; }; /** @@ -54,7 +58,7 @@ struct getdns_dict_item { * application should stick to the helper functions. */ struct getdns_dict { - void *rootp; + ldns_rbtree_t root; }; From 6bc33c50ab3683dad2541d619dae984fbcf3e032 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 30 Oct 2013 17:05:49 +0100 Subject: [PATCH 2/5] Fix stack buffer overflow in _pretty_print_dict --- src/dict.c | 187 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 138 insertions(+), 49 deletions(-) diff --git a/src/dict.c b/src/dict.c index 60a011ad..2b1de402 100644 --- a/src/dict.c +++ b/src/dict.c @@ -30,6 +30,7 @@ */ #include +#include #include #include "dict.h" @@ -233,9 +234,8 @@ getdns_dict_create() /*---------------------------------------- getdns_dict_copy */ /** - * private function used to make a copy of a dict structure, the caller is responsible - * for freeing storage allocated to returned value - * NOTE: not thread safe - this needs to be fixed to be thread safe + * private function used to make a copy of a dict structure, + * the caller is responsible * for freeing storage allocated to returned value * @param srcdict the dictionary structure to copy * @param dstdict the copy destination * @return the address of the copy of the dictionary structure on success @@ -435,62 +435,151 @@ getdns_dict_set_int(struct getdns_dict *dict, char *name, uint32_t child_uint32) return retval; } /* getdns_dict_set_int */ +const char * +getdns_indent(size_t indent) +{ + static const char *spaces = " " + " "; + return spaces + 80 - (indent < 80 ? indent : 0); +} + +int +getdns_snprintf(char **str, size_t *size, int* total, const char *format, ...) +{ + va_list ap; + int written; + + va_start(ap, format); + written = vsnprintf(*str, *size, format, ap); + va_end(ap); + + if(written < 0) { + *total = written; + return written; + } + *total += written; + if(written <= *size) { + *str += written; + *size -= written; + } else { + *str += *size; + *size = 0; + } + return 0; +} + +/*---------------------------------------- getdns_snprint_dict */ +/** + * private function to pretty print dict to a buffer, moving the buffer pointer + * forward in the process. + * @param str destination character buffer. + * str will be increased with the number of characters written + * @param size bytes available in destination character buffer. + * the number of characters written will be substracted from size + * @param total the number of characters written will be added to toal + * @param indent number of spaces to append after newline + * @param dict the dict to print + * @return zero on successi + * if an output error is encountered, a negative value is returned + */ +int +getdns_snprint_dict(char **str, size_t *size, int *total, size_t indent, struct getdns_dict *dict) +{ + struct getdns_dict_item *item; + size_t i; + + if(dict == NULL) + return *total; + + if(getdns_snprintf(str, size, total, "{")) + return *total; + + i = 0; + indent += 2; + LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) + { + /* + fprintf(stderr, "%s\n%s\"%s\":", (i ? "," : ""), + getdns_indent(indent), item->node.key); + */ + if(getdns_snprintf(str, size, total, + "%s\n%s\"%s\":", (i ? "," : ""), + getdns_indent(indent), item->node.key)) + return *total; + + switch(item->dtype) + { + case t_bindata: + if(getdns_snprintf(str, size, total, " ", + (int)item->data.bindata->size)) + return *total; + break; + + case t_dict: + if(getdns_snprintf(str, size, total, + "\n%s", getdns_indent(indent))) + return *total; + + if(getdns_snprint_dict(str, size, total, indent, item->data.dict)) + return *total; + + case t_int: + if(getdns_snprintf(str, size, total, " %d", (int)item->data.n)) + return *total; + break; + + case t_list: + if(getdns_snprintf(str, size, total, " []")) + return *total; + break; + + case t_invalid: + default: + if(getdns_snprintf(str, size, total, " ")) + return *total; + break; + } + i++; + } + indent -= 2; + if (getdns_snprintf(str, size, total, i ? "\n%s}" : "}", + getdns_indent(indent))) + return *total; + + return 0; +} /* getdns_snprint_dict */ + /*---------------------------------------- getdns_pretty_print_dict */ char * getdns_pretty_print_dict(struct getdns_dict *dict) { - struct getdns_dict_item *item; - char buf[8192]; - size_t i; - char* tmp; + size_t size = 1024; + char buf[1024]; + char *str, *newstr; + int total = 0; - buf[0] = 0; + str = buf; + if(getdns_snprint_dict(&str, &size, &total, 0, dict)) + return NULL; - if(dict != NULL) - { - i = 0; - strcat(buf, "{"); - LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) - { - if (i) - strcat(buf, ", \""); - else - strcat(buf, " \""); - strcat(buf, item->node.key); - strcat(buf, "\": "); - switch(item->dtype) - { - case t_bindata: - sprintf(buf + strlen(buf), "", (int)item->data.bindata->size); - break; + if(total == 0) + return strdup(""); - case t_dict: - tmp = getdns_pretty_print_dict(item->data.dict); - strcat(buf, tmp); - free(tmp); - break; + if(total <= size) + return strdup(buf); - case t_int: - sprintf(buf + strlen(buf), "%d", item->data.n); - break; + total += 1; + str = newstr = (char *)malloc((size_t) total * sizeof(char)); + if(!str) + return NULL; - case t_list: - strcat(buf, "[]"); - break; - - case t_invalid: - default: - strcat(buf, ""); - break; - } - i++; - } - if(i) - strcat(buf, " "); - strcat(buf, "}"); + size = total; + total = 0; + if (getdns_snprint_dict(&str, &size, &total, 0, dict)) { + free(newstr); + return NULL; } - - return strdup(buf); + return newstr; } /* getdns_pretty_print_dict */ /* getdns_dict.c */ From 684337652148129cb09045608e26a1368f5f112d Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 30 Oct 2013 23:23:53 +0100 Subject: [PATCH 3/5] Pretty print safe & complete... but ugly And it needs a bit of documentational comments --- src/dict.c | 165 +++++++++++++++++++++++++++++++++++++++++------------ src/dict.h | 2 +- 2 files changed, 129 insertions(+), 38 deletions(-) diff --git a/src/dict.c b/src/dict.c index 2b1de402..e0304bea 100644 --- a/src/dict.c +++ b/src/dict.c @@ -31,6 +31,7 @@ #include #include +#include #include #include "dict.h" @@ -435,7 +436,7 @@ getdns_dict_set_int(struct getdns_dict *dict, char *name, uint32_t child_uint32) return retval; } /* getdns_dict_set_int */ -const char * +static const char * getdns_indent(size_t indent) { static const char *spaces = " " @@ -443,7 +444,7 @@ getdns_indent(size_t indent) return spaces + 80 - (indent < 80 ? indent : 0); } -int +static int getdns_snprintf(char **str, size_t *size, int* total, const char *format, ...) { va_list ap; @@ -468,6 +469,9 @@ getdns_snprintf(char **str, size_t *size, int* total, const char *format, ...) return 0; } +static int +getdns_snprint_item(char **str, size_t *size, int *total, size_t indent, getdns_data_type dtype, union getdns_item item); + /*---------------------------------------- getdns_snprint_dict */ /** * private function to pretty print dict to a buffer, moving the buffer pointer @@ -482,14 +486,14 @@ getdns_snprintf(char **str, size_t *size, int* total, const char *format, ...) * @return zero on successi * if an output error is encountered, a negative value is returned */ -int +static int getdns_snprint_dict(char **str, size_t *size, int *total, size_t indent, struct getdns_dict *dict) { struct getdns_dict_item *item; - size_t i; + size_t i, length; if(dict == NULL) - return *total; + return 0; if(getdns_snprintf(str, size, total, "{")) return *total; @@ -498,47 +502,29 @@ getdns_snprint_dict(char **str, size_t *size, int *total, size_t indent, struct indent += 2; LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) { - /* - fprintf(stderr, "%s\n%s\"%s\":", (i ? "," : ""), - getdns_indent(indent), item->node.key); - */ if(getdns_snprintf(str, size, total, "%s\n%s\"%s\":", (i ? "," : ""), getdns_indent(indent), item->node.key)) return *total; - switch(item->dtype) - { - case t_bindata: - if(getdns_snprintf(str, size, total, " ", - (int)item->data.bindata->size)) - return *total; - break; - - case t_dict: - if(getdns_snprintf(str, size, total, - "\n%s", getdns_indent(indent))) + do { + if(item->dtype == t_list) { + if(getdns_list_get_length(item->data.list, &length) != GETDNS_RETURN_GOOD) + return -1; + if(length == 0) { + if(getdns_snprintf(str, size, total, " []")) + return *total; + break; + } + } + if(item->dtype == t_dict || item->dtype == t_list) + if(getdns_snprintf(str, size, total, "\n%s", getdns_indent(indent))) return *total; - if(getdns_snprint_dict(str, size, total, indent, item->data.dict)) - return *total; + getdns_snprint_item(str, size, total, indent, item->dtype, item->data); - case t_int: - if(getdns_snprintf(str, size, total, " %d", (int)item->data.n)) - return *total; - break; + } while(false); - case t_list: - if(getdns_snprintf(str, size, total, " []")) - return *total; - break; - - case t_invalid: - default: - if(getdns_snprintf(str, size, total, " ")) - return *total; - break; - } i++; } indent -= 2; @@ -549,6 +535,111 @@ getdns_snprint_dict(char **str, size_t *size, int *total, size_t indent, struct return 0; } /* getdns_snprint_dict */ +static int +getdns_snprint_list(char **str, size_t *size, int *total, size_t indent, struct getdns_list *list) +{ + getdns_data_type dtype; + union getdns_item data; + size_t i, length; + + if(list == NULL) + return 0; + + if(getdns_snprintf(str, size, total, "[")) + return *total; + + if (getdns_list_get_length(list, &length) != GETDNS_RETURN_GOOD) + return -1; + + indent += 2; + for(i = 0; i < length; i++) + { + if(getdns_snprintf(str, size, total, + "%s\n%s", (i ? "," : ""), getdns_indent(indent))) + return *total; + + if(getdns_list_get_data_type(list, i, &dtype) != GETDNS_RETURN_GOOD) + return -1; + + if((dtype == t_bindata && + getdns_list_get_bindata(list, i, &data.bindata) != GETDNS_RETURN_GOOD) || + (dtype == t_dict && + getdns_list_get_dict(list, i, &data.dict) != GETDNS_RETURN_GOOD) || + (dtype == t_int && + getdns_list_get_int(list, i, &data.n) != GETDNS_RETURN_GOOD) || + (dtype == t_list && + getdns_list_get_list(list, i, &data.list) != GETDNS_RETURN_GOOD)) + return -1; + + if(getdns_snprint_item(str, size, total, indent, dtype, data)) + return *total; + i++; + } + indent -= 2; + if (getdns_snprintf(str, size, total, i ? "\n%s]" : "]", + getdns_indent(indent))) + return *total; + + return 0; +} /* getdns_snprint_dict */ + + +static int +getdns_snprint_item(char **str, size_t *size, int *total, size_t indent, getdns_data_type dtype, union getdns_item item) +{ + size_t i; + + switch(dtype) + { + case t_bindata: + if(getdns_snprintf(str, size, total, " size)) + return *total; + i = 0; + if(item.bindata->size && item.bindata->data[item.bindata->size - 1] == 0) + while(i < item.bindata->size - 1 && isprint(item.bindata->data[i])) + i++; + if(i >= item.bindata->size - 1) { + if(getdns_snprintf(str, size, total, "\"%s\">", item.bindata->data)) + return *total; + break; + } + for(i = 0; i < item.bindata->size; i++) { + if(i >= 16) { + if(getdns_snprintf(str, size, total, "...")) + return *total; + break; + } + if(getdns_snprintf(str, size, total, "%.2X", (int)item.bindata->data[i])) + return *total; + } + if(getdns_snprintf(str, size, total, ">")) + return *total; + break; + + case t_dict: + if(getdns_snprint_dict(str, size, total, indent, item.dict)) + return *total; + break; + + case t_int: + if(getdns_snprintf(str, size, total, " %d", (int)item.n)) + return *total; + break; + + case t_list: + if(getdns_snprint_list(str, size, total, indent, item.list)) + return *total; + break; + + case t_invalid: + default: + if(getdns_snprintf(str, size, total, " ")) + return *total; + break; + } + return 0; +} + /*---------------------------------------- getdns_pretty_print_dict */ char * getdns_pretty_print_dict(struct getdns_dict *dict) diff --git a/src/dict.h b/src/dict.h index 28820554..8a483b44 100644 --- a/src/dict.h +++ b/src/dict.h @@ -36,7 +36,7 @@ union getdns_item { struct getdns_list *list; struct getdns_dict *dict; - int n; + uint32_t n; struct getdns_bindata *bindata; }; From c1ba94c08a984ceac20dea7fcbf42f69646853a9 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 30 Oct 2013 23:33:29 +0100 Subject: [PATCH 4/5] Remove key attribute from getdns_dict_item It is in the node attribute already --- src/dict.c | 29 +++++++++++------------------ src/dict.h | 1 - 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/dict.c b/src/dict.c index e0304bea..b5b2b0f8 100644 --- a/src/dict.c +++ b/src/dict.c @@ -35,13 +35,6 @@ #include #include "dict.h" -/*---------------------------------------- getdns_dict_cmp */ -int -getdns_dict_cmp(const void *item1, const void *item2) -{ - return strcmp((const char *)item1, (const char *)item2); -} /* getdns_dict_comp */ - /*---------------------------------------- getdns_dict_find */ /** * private function used to locate a key in a dictionary @@ -63,8 +56,7 @@ getdns_dict_find(struct getdns_dict *dict, char *key, bool addifnotfnd) { /* tsearch will add a node automatically for us */ item = (struct getdns_dict_item *) malloc(sizeof(struct getdns_dict_item)); - item->key = strdup(key); - item->node.key = item->key; + item->node.key = strdup(key); item->dtype = t_invalid; item->data.n = 0; ldns_rbtree_insert(&(dict->root), (ldns_rbnode_t *)item); @@ -91,8 +83,8 @@ getdns_dict_get_names(struct getdns_dict *dict, struct getdns_list **answer) if(getdns_list_add_item(*answer, &index) == GETDNS_RETURN_GOOD) { struct getdns_bindata bindata; - bindata.size = strlen(item->key); - bindata.data = (void *)item->key; + bindata.size = strlen(item->node.key); + bindata.data = (void *)item->node.key; getdns_list_set_bindata(*answer, index, &bindata); } } @@ -228,7 +220,6 @@ getdns_dict_create() struct getdns_dict *dict; dict = (struct getdns_dict *) malloc(sizeof(struct getdns_dict)); - //ldns_rbtree_init(&(dict->root), getdns_dict_cmp); ldns_rbtree_init(&(dict->root), (int (*)(const void *, const void *))strcmp); return dict; } /* getdns_dict_create */ @@ -247,6 +238,7 @@ getdns_dict_copy(struct getdns_dict *srcdict, struct getdns_dict **dstdict) { getdns_return_t retval = GETDNS_RETURN_NO_SUCH_DICT_NAME; struct getdns_dict_item *item; + char *key; if(srcdict != NULL && dstdict != NULL) { @@ -254,22 +246,23 @@ getdns_dict_copy(struct getdns_dict *srcdict, struct getdns_dict **dstdict) LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(srcdict->root)) { + key = (const *)item->node.key; switch(item->dtype) { case t_bindata: - getdns_dict_set_bindata(*dstdict, item->key, item->data.bindata); + getdns_dict_set_bindata(*dstdict, key, item->data.bindata); break; case t_dict: - getdns_dict_set_dict(*dstdict, item->key, item->data.dict); + getdns_dict_set_dict(*dstdict, key, item->data.dict); break; case t_int: - getdns_dict_set_int(*dstdict, item->key, item->data.n); + getdns_dict_set_int(*dstdict, key, item->data.n); break; case t_list: - getdns_dict_set_list(*dstdict, item->key, item->data.list); + getdns_dict_set_list(*dstdict, key, item->data.list); break; case t_invalid: @@ -312,8 +305,8 @@ getdns_dict_item_free(ldns_rbnode_t *node, void *arg) getdns_list_destroy(item->data.list); } - if(item->key != NULL) - free(item->key); + if(item->node.key != NULL) + free((char *)item->node.key); free(item); } } /* getdns_dict_item_free */ diff --git a/src/dict.h b/src/dict.h index 8a483b44..d184cba2 100644 --- a/src/dict.h +++ b/src/dict.h @@ -45,7 +45,6 @@ union getdns_item { */ struct getdns_dict_item { ldns_rbnode_t node; - char *key; getdns_data_type dtype; union getdns_item data; }; From e0c28a6346a700f14b3da5f182e5293bb6d5e280 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Fri, 1 Nov 2013 18:07:03 +0100 Subject: [PATCH 5/5] Pretty printing to ldns_buffer This makes it much more readable --- src/dict.c | 357 +++++++++++++++++++++++++++-------------------------- 1 file changed, 182 insertions(+), 175 deletions(-) diff --git a/src/dict.c b/src/dict.c index b5b2b0f8..2d78e5f8 100644 --- a/src/dict.c +++ b/src/dict.c @@ -29,10 +29,8 @@ * THE SOFTWARE. */ -#include -#include #include -#include +#include #include "dict.h" /*---------------------------------------- getdns_dict_find */ @@ -246,7 +244,7 @@ getdns_dict_copy(struct getdns_dict *srcdict, struct getdns_dict **dstdict) LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(srcdict->root)) { - key = (const *)item->node.key; + key = (char *)item->node.key; switch(item->dtype) { case t_bindata: @@ -429,117 +427,93 @@ getdns_dict_set_int(struct getdns_dict *dict, char *name, uint32_t child_uint32) return retval; } /* getdns_dict_set_int */ +/*---------------------------------------- getdns_pp_dict */ +/** + * private function to help with indenting. + * @param indent number of spaces to return + * @return a character string containing min(80, indent) spaces + */ static const char * getdns_indent(size_t indent) { static const char *spaces = " " " "; return spaces + 80 - (indent < 80 ? indent : 0); -} +} /* getdns_indent */ -static int -getdns_snprintf(char **str, size_t *size, int* total, const char *format, ...) -{ - va_list ap; - int written; - - va_start(ap, format); - written = vsnprintf(*str, *size, format, ap); - va_end(ap); - - if(written < 0) { - *total = written; - return written; - } - *total += written; - if(written <= *size) { - *str += written; - *size -= written; - } else { - *str += *size; - *size = 0; - } - return 0; -} - -static int -getdns_snprint_item(char **str, size_t *size, int *total, size_t indent, getdns_data_type dtype, union getdns_item item); - -/*---------------------------------------- getdns_snprint_dict */ +/*---------------------------------------- getdns_pp_bindata */ /** - * private function to pretty print dict to a buffer, moving the buffer pointer - * forward in the process. - * @param str destination character buffer. - * str will be increased with the number of characters written - * @param size bytes available in destination character buffer. - * the number of characters written will be substracted from size - * @param total the number of characters written will be added to toal - * @param indent number of spaces to append after newline - * @param dict the dict to print - * @return zero on successi - * if an output error is encountered, a negative value is returned + * private function to pretty print bindata to a ldns_buffer + * @param buf buffer to write to + * @param indent number of spaces to append after newline + * @param bindata the bindata to print + * @return on success the number of written characters + * if an output error is encountered, a negative value */ static int -getdns_snprint_dict(char **str, size_t *size, int *total, size_t indent, struct getdns_dict *dict) +getdns_pp_bindata(ldns_buffer *buf, size_t indent, struct getdns_bindata *bindata) { - struct getdns_dict_item *item; - size_t i, length; + size_t i, p = ldns_buffer_position(buf); + uint8_t *dptr; - if(dict == NULL) - return 0; - - if(getdns_snprintf(str, size, total, "{")) - return *total; + if(ldns_buffer_printf(buf, " root)) - { - if(getdns_snprintf(str, size, total, - "%s\n%s\"%s\":", (i ? "," : ""), - getdns_indent(indent), item->node.key)) - return *total; + if(bindata->size && bindata->data[bindata->size - 1] == 0) + while(i < bindata->size - 1 && isprint(bindata->data[i])) + i++; - do { - if(item->dtype == t_list) { - if(getdns_list_get_length(item->data.list, &length) != GETDNS_RETURN_GOOD) + if(i >= bindata->size - 1) { /* all chars were printable */ + if(ldns_buffer_printf(buf, "for \"%s\">", bindata->data) < 0) + return -1; + } else { + if(ldns_buffer_printf(buf, "of 0x") < 0) + return -1; + for(dptr = bindata->data; dptr < bindata->data + bindata->size; dptr++) { + if(dptr - bindata->data >= 16) { + if(ldns_buffer_printf(buf, "...") < 0) return -1; - if(length == 0) { - if(getdns_snprintf(str, size, total, " []")) - return *total; - break; - } + break; } - if(item->dtype == t_dict || item->dtype == t_list) - if(getdns_snprintf(str, size, total, "\n%s", getdns_indent(indent))) - return *total; - - getdns_snprint_item(str, size, total, indent, item->dtype, item->data); - - } while(false); - - i++; + if(ldns_buffer_printf(buf, "%.2x", *dptr) < 0) + return -1; + } + if(ldns_buffer_printf(buf, ">") < 0) + return -1; } - indent -= 2; - if (getdns_snprintf(str, size, total, i ? "\n%s}" : "}", - getdns_indent(indent))) - return *total; + return ldns_buffer_position(buf) - p; +} /* getdns_pp_bindata */ - return 0; -} /* getdns_snprint_dict */ static int -getdns_snprint_list(char **str, size_t *size, int *total, size_t indent, struct getdns_list *list) +getdns_pp_dict(ldns_buffer *buf, size_t indent, struct getdns_dict *dict); + +/*---------------------------------------- getdns_pp_list */ +/** + * private function to pretty print list to a ldns_buffer + * @param buf buffer to write to + * @param indent number of spaces to append after newline + * @param list the to list print + * @return on success the number of written characters + * if an output error is encountered, a negative value + */ +static int +getdns_pp_list(ldns_buffer *buf, size_t indent, struct getdns_list *list) { + size_t i, length, p = ldns_buffer_position(buf); getdns_data_type dtype; - union getdns_item data; - size_t i, length; + struct getdns_dict *dict_item; + struct getdns_list *list_item; + struct getdns_bindata *bindata_item; + uint32_t int_item; if(list == NULL) return 0; - if(getdns_snprintf(str, size, total, "[")) - return *total; + if(ldns_buffer_printf(buf, "[") < 0) + return -1; if (getdns_list_get_length(list, &length) != GETDNS_RETURN_GOOD) return -1; @@ -547,123 +521,156 @@ getdns_snprint_list(char **str, size_t *size, int *total, size_t indent, struct indent += 2; for(i = 0; i < length; i++) { - if(getdns_snprintf(str, size, total, - "%s\n%s", (i ? "," : ""), getdns_indent(indent))) - return *total; + if(ldns_buffer_printf(buf, "%s\n%s", (i ? "," : ""), + getdns_indent(indent)) < 0) + return -1; if(getdns_list_get_data_type(list, i, &dtype) != GETDNS_RETURN_GOOD) return -1; - if((dtype == t_bindata && - getdns_list_get_bindata(list, i, &data.bindata) != GETDNS_RETURN_GOOD) || - (dtype == t_dict && - getdns_list_get_dict(list, i, &data.dict) != GETDNS_RETURN_GOOD) || - (dtype == t_int && - getdns_list_get_int(list, i, &data.n) != GETDNS_RETURN_GOOD) || - (dtype == t_list && - getdns_list_get_list(list, i, &data.list) != GETDNS_RETURN_GOOD)) - return -1; + switch(dtype) { + case t_int : if (getdns_list_get_int(list, i, &int_item) != + GETDNS_RETURN_GOOD) + if(ldns_buffer_printf(buf, " %d", (int)int_item) < 0) + return -1; + break; - if(getdns_snprint_item(str, size, total, indent, dtype, data)) - return *total; + case t_bindata: if (getdns_list_get_bindata(list, i, &bindata_item) != + GETDNS_RETURN_GOOD) + return -1; + if (getdns_pp_bindata(buf, indent, bindata_item) < 0) + return -1; + break; + + case t_list : if (getdns_list_get_list(list, i, &list_item) != + GETDNS_RETURN_GOOD) + return -1; + if (getdns_pp_list(buf, indent, list_item) < 0) + return -1; + break; + + case t_dict : if (getdns_list_get_dict(list, i, &dict_item) != + GETDNS_RETURN_GOOD) + return -1; + if (getdns_pp_dict(buf, indent, dict_item) < 0) + return -1; + break; + + case t_invalid: + default : if(ldns_buffer_printf(buf, " ") < 0) + return -1; + } i++; } indent -= 2; - if (getdns_snprintf(str, size, total, i ? "\n%s]" : "]", - getdns_indent(indent))) - return *total; + if (ldns_buffer_printf(buf, i ? "\n%s]" : "]", getdns_indent(indent)) < 0) + return -1; - return 0; -} /* getdns_snprint_dict */ + return ldns_buffer_position(buf) - p; +} /* getdns_pp_list */ +/*---------------------------------------- getdns_pp_dict */ +/** + * private function to pretty print dict to a ldns_buffer + * @param buf buffer to write to + * @param indent number of spaces to append after newline + * @param dict the dict to print + * @return on success the number of written characters + * if an output error is encountered, a negative value + */ static int -getdns_snprint_item(char **str, size_t *size, int *total, size_t indent, getdns_data_type dtype, union getdns_item item) +getdns_pp_dict(ldns_buffer *buf, size_t indent, struct getdns_dict *dict) { - size_t i; + size_t i, length, p = ldns_buffer_position(buf); + struct getdns_dict_item *item; - switch(dtype) + if(dict == NULL) + return 0; + + if(ldns_buffer_printf(buf, "{") < 0) + return -1; + + i = 0; + indent += 2; + LDNS_RBTREE_FOR(item, struct getdns_dict_item *, &(dict->root)) { - case t_bindata: - if(getdns_snprintf(str, size, total, " size)) - return *total; - i = 0; - if(item.bindata->size && item.bindata->data[item.bindata->size - 1] == 0) - while(i < item.bindata->size - 1 && isprint(item.bindata->data[i])) - i++; - if(i >= item.bindata->size - 1) { - if(getdns_snprintf(str, size, total, "\"%s\">", item.bindata->data)) - return *total; - break; - } - for(i = 0; i < item.bindata->size; i++) { - if(i >= 16) { - if(getdns_snprintf(str, size, total, "...")) - return *total; - break; - } - if(getdns_snprintf(str, size, total, "%.2X", (int)item.bindata->data[i])) - return *total; - } - if(getdns_snprintf(str, size, total, ">")) - return *total; - break; + if(ldns_buffer_printf(buf, "%s\n%s\"%s\":", (i ? "," : "") + , getdns_indent(indent) + , item->node.key) < 0) + return -1; - case t_dict: - if(getdns_snprint_dict(str, size, total, indent, item.dict)) - return *total; - break; + switch(item->dtype) { + case t_int : if(ldns_buffer_printf(buf, " %d", item->data.n) < 0) + return -1; + break; - case t_int: - if(getdns_snprintf(str, size, total, " %d", (int)item.n)) - return *total; - break; + case t_bindata: if (getdns_pp_bindata(buf, indent, + item->data.bindata) < 0) + return -1; + break; - case t_list: - if(getdns_snprint_list(str, size, total, indent, item.list)) - return *total; - break; + case t_list : /* Don't put empty lists on a new line */ - case t_invalid: - default: - if(getdns_snprintf(str, size, total, " ")) - return *total; - break; + if(getdns_list_get_length(item->data.list, + &length) != GETDNS_RETURN_GOOD) + return -1; + if(length == 0) { + if(ldns_buffer_printf(buf, " []") < 0) + return -1; + break; + } + if(ldns_buffer_printf(buf, "\n%s", getdns_indent(indent)) < 0) + return -1; + if(getdns_pp_list(buf, indent, item->data.list) < 0) + return -1; + break; + + case t_dict : if(ldns_buffer_printf(buf, "\n%s", getdns_indent(indent)) < 0) + return -1; + if(getdns_pp_dict(buf, indent, item->data.dict) < 0) + return -1; + break; + + case t_invalid: + default : if(ldns_buffer_printf(buf, " ") < 0) + return -1; + } + i++; } - return 0; -} + indent -= 2; + if (ldns_buffer_printf(buf, i ? "\n%s}" : "}", getdns_indent(indent)) < 0) + return -1; + + return ldns_buffer_position(buf) - p; +} /* getdns_pp_dict */ + /*---------------------------------------- getdns_pretty_print_dict */ +/** + * Return a character string containing a "human readable" representation + * of dict. + * @param dict the dict to pretty print + * @return the "human readable" representation of dict + * or NULL on error + */ char * getdns_pretty_print_dict(struct getdns_dict *dict) { - size_t size = 1024; - char buf[1024]; - char *str, *newstr; - int total = 0; + ldns_buffer *buf; + char *ret; - str = buf; - if(getdns_snprint_dict(&str, &size, &total, 0, dict)) + buf = ldns_buffer_new(100); + if (! buf) return NULL; - if(total == 0) - return strdup(""); - - if(total <= size) - return strdup(buf); - - total += 1; - str = newstr = (char *)malloc((size_t) total * sizeof(char)); - if(!str) - return NULL; - - size = total; - total = 0; - if (getdns_snprint_dict(&str, &size, &total, 0, dict)) { - free(newstr); + if (getdns_pp_dict(buf, 0, dict) < 0) { + ldns_buffer_free(buf); return NULL; } - return newstr; + ret = (char *)ldns_buffer_export(buf); + ldns_buffer_free(buf); + return ret; } /* getdns_pretty_print_dict */ /* getdns_dict.c */