From 307bf74a8cef765fcd5d788f6aad5b2e9bdf670b Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Tue, 16 May 2017 22:44:26 +0200 Subject: [PATCH] curl_json plugin: Refactor the way trees/keys are stored. Previously, keys had a "magic" as their first member which was used to differentiate between the two types when they were returned from the binary search tree. This patch creates a new struct, cj_tree_entry_t, which includes an enum identifying which union member is valid. --- src/curl_json.c | 148 ++++++++++++++++++++++++++++----------------------- src/curl_json_test.c | 7 ++- 2 files changed, 85 insertions(+), 70 deletions(-) diff --git a/src/curl_json.c b/src/curl_json.c index a589cd65..6b50b454 100644 --- a/src/curl_json.c +++ b/src/curl_json.c @@ -44,8 +44,6 @@ #endif #define CJ_DEFAULT_HOST "localhost" -#define CJ_KEY_MAGIC 0x43484b59UL /* CHKY */ -#define CJ_IS_KEY(key) ((key)->magic == CJ_KEY_MAGIC) #define CJ_ANY "*" #define COUCH_MIN(x, y) ((x) < (y) ? (x) : (y)) @@ -53,13 +51,34 @@ struct cj_key_s; typedef struct cj_key_s cj_key_t; struct cj_key_s /* {{{ */ { - unsigned long magic; char *path; char *type; char *instance; }; /* }}} */ +/* cj_tree_entry_t is a union of either a metric configuration ("key") or a tree + * mapping array indexes / map keys to a descendant cj_tree_entry_t*. */ +typedef struct { + enum { KEY, TREE } type; + union { + c_avl_tree_t *tree; + cj_key_t *key; + }; +} cj_tree_entry_t; + +/* cj_state_t is a stack providing the configuration relevant for the context + * that is currently being parsed. If entry->type == KEY, the parser should + * expect a metric (a numeric value). If entry->type == TREE, the parser should + * expect an array of map to descent into. If entry == NULL, no configuration + * exists for this part of the JSON structure. */ +typedef struct { + cj_tree_entry_t *entry; + _Bool in_array; + int index; + char name[DATA_MAX_NAME_LEN]; +} cj_state_t; + struct cj_s /* {{{ */ { char *instance; @@ -87,15 +106,7 @@ struct cj_s /* {{{ */ yajl_handle yajl; c_avl_tree_t *tree; int depth; - struct { - union { - c_avl_tree_t *tree; - cj_key_t *key; - }; - _Bool in_array; - int index; - char name[DATA_MAX_NAME_LEN]; - } state[YAJL_MAX_DEPTH]; + cj_state_t state[YAJL_MAX_DEPTH]; }; typedef struct cj_s cj_t; /* }}} */ @@ -144,12 +155,10 @@ static size_t cj_curl_callback(void *buf, /* {{{ */ } /* }}} size_t cj_curl_callback */ static int cj_get_type(cj_key_t *key) { - const data_set_t *ds; - - if ((key == NULL) || !CJ_IS_KEY(key)) + if (key == NULL) return -EINVAL; - ds = plugin_get_ds(key->type); + const data_set_t *ds = plugin_get_ds(key->type); if (ds == NULL) { static char type[DATA_MAX_NAME_LEN] = "!!!invalid!!!"; @@ -182,31 +191,20 @@ static int cj_load_key(cj_t *db, char const *key) { sstrncpy(db->state[db->depth].name, key, sizeof(db->state[db->depth].name)); - c_avl_tree_t *tree = db->state[db->depth - 1].tree; - if (tree == NULL) { + if (db->state[db->depth - 1].entry == NULL || + db->state[db->depth - 1].entry->type != TREE) { return 0; } - /* the parent has a key, so the tree pointer is invalid. */ - if (CJ_IS_KEY(db->state[db->depth - 1].key)) { - return 0; - } + c_avl_tree_t *tree = db->state[db->depth - 1].entry->tree; + cj_tree_entry_t *e = NULL; - void *value = NULL; - if (c_avl_get(tree, key, (void *)&value) == 0) { - if (CJ_IS_KEY((cj_key_t *)value)) { - db->state[db->depth].key = value; - } else { - db->state[db->depth].tree = value; - } - } else if (c_avl_get(tree, CJ_ANY, (void *)&value) == 0) { - if (CJ_IS_KEY((cj_key_t *)value)) { - db->state[db->depth].key = value; - } else { - db->state[db->depth].tree = value; - } + if (c_avl_get(tree, key, (void *)&e) == 0) { + db->state[db->depth].entry = e; + } else if (c_avl_get(tree, CJ_ANY, (void *)&e) == 0) { + db->state[db->depth].entry = e; } else { - db->state[db->depth].key = NULL; + db->state[db->depth].entry = NULL; } return 0; @@ -238,30 +236,26 @@ static int cj_cb_null(void *ctx) { } static int cj_cb_number(void *ctx, const char *number, yajl_len_t number_len) { - char buffer[number_len + 1]; - cj_t *db = (cj_t *)ctx; - cj_key_t *key = db->state[db->depth].key; /* Create a null-terminated version of the string. */ + char buffer[number_len + 1]; memcpy(buffer, number, number_len); buffer[sizeof(buffer) - 1] = 0; - - - if (key == NULL) { - /* no config for this element. */ - cj_advance_array(ctx); - return CJ_CB_CONTINUE; - } else if (!CJ_IS_KEY(key)) { - /* the config expects a map or an array. */ - NOTICE( - "curl_json plugin: Found \"%s\", but the configuration expects a map.", - buffer); + if (db->state[db->depth].entry == NULL || + db->state[db->depth].entry->type != KEY) { + if (db->state[db->depth].entry != NULL) { + NOTICE("curl_json plugin: Found \"%s\", but the configuration expects a " + "map.", + buffer); + } cj_advance_array(ctx); return CJ_CB_CONTINUE; } + cj_key_t *key = db->state[db->depth].entry->key; + int type = cj_get_type(key); value_t vt; int status = parse_value(buffer, &vt, type); @@ -299,7 +293,7 @@ static int cj_cb_string(void *ctx, const unsigned char *val, yajl_len_t len) { static int cj_cb_end(void *ctx) { cj_t *db = (cj_t *)ctx; - db->state[db->depth].tree = NULL; + memset(&db->state[db->depth], 0, sizeof(db->state[db->depth])); db->depth--; cj_advance_array(ctx); return (CJ_CB_CONTINUE); @@ -367,17 +361,16 @@ static void cj_key_free(cj_key_t *key) /* {{{ */ static void cj_tree_free(c_avl_tree_t *tree) /* {{{ */ { char *name; - void *value; + cj_tree_entry_t *e; - while (c_avl_pick(tree, (void *)&name, (void *)&value) == 0) { - cj_key_t *key = (cj_key_t *)value; + while (c_avl_pick(tree, (void *)&name, (void *)&e) == 0) { + sfree(name); - if (CJ_IS_KEY(key)) - cj_key_free(key); + if (e->type == KEY) + cj_key_free(e->key); else - cj_tree_free((c_avl_tree_t *)value); - - sfree(name); + cj_tree_free(e->tree); + sfree(e); } c_avl_destroy(tree); @@ -471,13 +464,21 @@ static int cj_append_key(cj_t *db, cj_key_t *key) { /* {{{ */ len = COUCH_MIN(len, sizeof(name) - 1); sstrncpy(name, start, len + 1); - c_avl_tree_t *value; - if (c_avl_get(tree, name, (void *)&value) != 0) { - value = cj_avl_create(); - c_avl_insert(tree, strdup(name), value); + cj_tree_entry_t *e; + if (c_avl_get(tree, name, (void *)&e) != 0) { + e = calloc(1, sizeof(*e)); + if (e == NULL) + return ENOMEM; + e->type = TREE; + e->tree = cj_avl_create(); + + c_avl_insert(tree, strdup(name), e); } - tree = value; + if (e->type != TREE) + return EINVAL; + + tree = e->tree; start = end + 1; } @@ -486,7 +487,13 @@ static int cj_append_key(cj_t *db, cj_key_t *key) { /* {{{ */ return -1; } - c_avl_insert(tree, strdup(start), key); + cj_tree_entry_t *e = calloc(1, sizeof(*e)); + if (e == NULL) + return ENOMEM; + e->type = KEY; + e->key = key; + + c_avl_insert(tree, strdup(start), e); return 0; } /* }}} int cj_append_key */ @@ -506,7 +513,6 @@ static int cj_config_add_key(cj_t *db, /* {{{ */ ERROR("curl_json plugin: calloc failed."); return (-1); } - key->magic = CJ_KEY_MAGIC; if (strcasecmp("Key", ci->key) == 0) { status = cf_util_get_string(ci, &key->path); @@ -940,9 +946,15 @@ static int cj_read(user_data_t *ud) /* {{{ */ db->depth = 0; memset(&db->state, 0, sizeof(db->state)); - db->state[db->depth].tree = db->tree; + db->state[0].entry = &(cj_tree_entry_t){ + .type = TREE, .tree = db->tree, + }; + + int status = cj_perform(db); + + db->state[0].entry = NULL; - return cj_perform(db); + return status; } /* }}} int cj_read */ static int cj_init(void) /* {{{ */ diff --git a/src/curl_json_test.c b/src/curl_json_test.c index 1e4d3b3b..4b984660 100644 --- a/src/curl_json_test.c +++ b/src/curl_json_test.c @@ -47,13 +47,14 @@ static cj_t *test_setup(char *json, char *key_path) { db->curl = (void *)cj_avl_create(); cj_key_t *key = calloc(1, sizeof(*key)); - key->magic = CJ_KEY_MAGIC; key->path = strdup(key_path); key->type = strdup("MAGIC"); assert(cj_append_key(db, key) == 0); - db->state[0].tree = db->tree; + db->state[0].entry = &(cj_tree_entry_t){ + .type = TREE, .tree = db->tree, + }; cj_curl_callback(json, strlen(json), 1, db); #if HAVE_YAJL_V2 @@ -62,6 +63,8 @@ static cj_t *test_setup(char *json, char *key_path) { yajl_parse_complete(db->yajl); #endif + db->state[0].entry = NULL; + return db; } -- 2.11.0