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.

diff --git a/src/curl_json.c b/src/curl_json.c
index a589cd6..6b50b45 100644 (file)
--- 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 1e4d3b3..4b98466 100644 (file)
--- 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;
 }