sharkd: clean up the JSON marshalling.

Add routines to open and close an object, and use them.  The open
routine takes a member name as an argument, sets it, and begins an
object; the close routine ends the object.

Have sharkd_json_response_close() end the object, just as
sharkd_json_response_open() begins it.

Have sharkd_session_process_tap_stats_node_cb() take a key and use that
when opening the array.

Have sharkd_session_process_frame_cb_tree() take a key and use that when
opening the array.

This makes the structure of the code better mirror the structure of the
JSON objects it marshals.

If there's a key for a string value, but there's no string value or no
format for a string value, crash with a null-pointer dereference rather
than putting out the key and then, on the next operation, getting a
"json_dumper_bad(): Bad json_dumper state: illegal transition" error as
in, for example, issue #18886. This way, it will be a bit more obvious
what the true error is.

If thre's no key for a base-64 value, crash rather than not setting the
key, for the same reason.

1 files changed, 47 insertions, 51 deletions

diff --git a/sharkd_session.c b/sharkd_session.c
index d63a1f2ec1..6c7468cabf 100644
--- a/sharkd_session.c
+++ b/sharkd_session.c
@@ -116,12 +116,10 @@ sharkd_json_value_anyf(const char *key, const char *format, ...)
     if (key)
         json_dumper_set_member_name(&dumper, key);
 
-    if (format) {
-        va_list ap;
-        va_start(ap, format);
-        json_dumper_value_va_list(&dumper, format, ap);
-        va_end(ap);
-    }
+    va_list ap;
+    va_start(ap, format);
+    json_dumper_value_va_list(&dumper, format, ap);
+    va_end(ap);
 }
 
 static void
@@ -129,15 +127,13 @@ sharkd_json_value_string(const char *key, const char *str)
 {
     if (key)
         json_dumper_set_member_name(&dumper, key);
-    if (str)
-        json_dumper_value_string(&dumper, str);
+    json_dumper_value_string(&dumper, str);
 }
 
 static void
 sharkd_json_value_base64(const char *key, const guint8 *data, size_t len)
 {
-    if (key)
-        json_dumper_set_member_name(&dumper, key);
+    json_dumper_set_member_name(&dumper, key);
     json_print_base64(data, len);
 }
 
@@ -147,14 +143,12 @@ sharkd_json_value_stringf(const char *key, const char *format, ...)
     if (key)
         json_dumper_set_member_name(&dumper, key);
 
-    if (format) {
-        va_list ap;
-        va_start(ap, format);
-        char* sformat = ws_strdup_printf("\"%s\"", format);
-        json_dumper_value_va_list(&dumper, sformat, ap);
-        g_free(sformat);
-        va_end(ap);
-    }
+    va_list ap;
+    va_start(ap, format);
+    char* sformat = ws_strdup_printf("\"%s\"", format);
+    json_dumper_value_va_list(&dumper, sformat, ap);
+    g_free(sformat);
+    va_end(ap);
 }
 
 static void
@@ -172,6 +166,19 @@ sharkd_json_array_close(void)
 }
 
 static void
+sharkd_json_object_open(const char *key)
+{
+    json_dumper_set_member_name(&dumper, key);
+    json_dumper_begin_object(&dumper);
+}
+
+static void
+sharkd_json_object_close(void)
+{
+    json_dumper_end_object(&dumper);
+}
+
+static void
 sharkd_json_response_open(guint32 id)
 {
     json_dumper_begin_object(&dumper);  // start the message
@@ -182,6 +189,8 @@ sharkd_json_response_open(guint32 id)
 static void
 sharkd_json_response_close(void)
 {
+    json_dumper_end_object(&dumper);  // end the message
+
     json_dumper_finish(&dumper);
 
     /*
@@ -205,15 +214,13 @@ static void
 sharkd_json_result_prologue(guint32 id)
 {
     sharkd_json_response_open(id);
-    sharkd_json_value_anyf("result", NULL);
-    json_dumper_begin_object(&dumper);  // start the result object
+    sharkd_json_object_open("result");  // start the result object
 }
 
 static void
 sharkd_json_result_epilogue(void)
 {
     json_dumper_end_object(&dumper);  // end the result object
-    json_dumper_end_object(&dumper);  // end the message
     sharkd_json_response_close();
 }
 
@@ -228,7 +235,6 @@ static void
 sharkd_json_result_array_epilogue(void)
 {
     sharkd_json_array_close();        // end of result array
-    json_dumper_end_object(&dumper);  // end the message
     sharkd_json_response_close();
 }
 
@@ -253,8 +259,7 @@ static void G_GNUC_PRINTF(4, 5)
 sharkd_json_error(guint32 id, int code, char* data, char* format, ...)
 {
     sharkd_json_response_open(id);
-    sharkd_json_value_anyf("error", NULL);
-    json_dumper_begin_object(&dumper);
+    sharkd_json_object_open("error");
     sharkd_json_value_anyf("code", "%d", code);
 
     if (format)
@@ -271,12 +276,11 @@ sharkd_json_error(guint32 id, int code, char* data, char* format, ...)
         g_free(error_msg);
     }
 
-    json_dumper_end_object(&dumper);
+    sharkd_json_object_close();
 
     if (data)
         sharkd_json_value_string("data", data);
 
-    json_dumper_end_object(&dumper);
     sharkd_json_response_close();
 }
 
@@ -1562,11 +1566,11 @@ sharkd_session_process_frames(const char *buf, const jsmntok_t *tokens, int coun
 }
 
 static void
-sharkd_session_process_tap_stats_node_cb(const stat_node *n)
+sharkd_session_process_tap_stats_node_cb(const char *key, const stat_node *n)
 {
     stat_node *node;
 
-    sharkd_json_array_open(NULL);
+    sharkd_json_array_open(key);
     for (node = n->children; node; node = node->next)
     {
         json_dumper_begin_object(&dumper);
@@ -1611,8 +1615,7 @@ sharkd_session_process_tap_stats_node_cb(const stat_node *n)
 
         if (node->children)
         {
-            sharkd_json_value_anyf("sub", NULL);
-            sharkd_session_process_tap_stats_node_cb(node);
+            sharkd_session_process_tap_stats_node_cb("sub", node);
         }
         json_dumper_end_object(&dumper);
     }
@@ -1651,8 +1654,7 @@ sharkd_session_process_tap_stats_cb(void *psp)
     sharkd_json_value_string("type", "stats");
     sharkd_json_value_string("name", st->cfg->name);
 
-    sharkd_json_value_anyf("stats", NULL);
-    sharkd_session_process_tap_stats_node_cb(&st->root);
+    sharkd_session_process_tap_stats_node_cb("stats", &st->root);
 
     json_dumper_end_object(&dumper);
 }
@@ -3286,11 +3288,11 @@ sharkd_session_process_follow(char *buf, const jsmntok_t *tokens, int count)
 }
 
 static void
-sharkd_session_process_frame_cb_tree(epan_dissect_t *edt, proto_tree *tree, tvbuff_t **tvbs, gboolean display_hidden)
+sharkd_session_process_frame_cb_tree(const char *key, epan_dissect_t *edt, proto_tree *tree, tvbuff_t **tvbs, gboolean display_hidden)
 {
     proto_node *node;
 
-    sharkd_json_array_open(NULL);
+    sharkd_json_array_open(key);
     for (node = tree->first_child; node; node = node->next)
     {
         field_info *finfo = PNODE_FINFO(node);
@@ -3387,8 +3389,7 @@ sharkd_session_process_frame_cb_tree(epan_dissect_t *edt, proto_tree *tree, tvbu
             if (finfo->tree_type != -1)
                 sharkd_json_value_anyf("e", "%d", finfo->tree_type);
 
-            sharkd_json_value_anyf("n", NULL);
-            sharkd_session_process_frame_cb_tree(edt, (proto_tree *) node, tvbs, display_hidden);
+            sharkd_session_process_frame_cb_tree("n", edt, (proto_tree *) node, tvbs, display_hidden);
         }
 
         json_dumper_end_object(&dumper);
@@ -3486,8 +3487,7 @@ sharkd_session_process_frame_cb(epan_dissect_t *edt, proto_tree *tree, struct ep
             tvbs[count] = NULL;
         }
 
-        sharkd_json_value_anyf("tree", NULL);
-        sharkd_session_process_frame_cb_tree(edt, tree, tvbs, display_hidden);
+        sharkd_session_process_frame_cb_tree("tree", edt, tree, tvbs, display_hidden);
 
         g_free(tvbs);
     }
@@ -4450,8 +4450,7 @@ sharkd_session_process_dumpconf_cb(pref_t *pref, gpointer d)
     char json_pref_key[512];
 
     snprintf(json_pref_key, sizeof(json_pref_key), "%s.%s", data->module->name, pref_name);
-    json_dumper_set_member_name(&dumper, json_pref_key);
-    json_dumper_begin_object(&dumper);
+    sharkd_json_object_open(json_pref_key);
 
     switch (prefs_get_type(pref))
     {
@@ -4544,7 +4543,7 @@ sharkd_session_process_dumpconf_cb(pref_t *pref, gpointer d)
     sharkd_json_value_string("t", prefs_get_title(pref));
 #endif
 
-    json_dumper_end_object(&dumper);
+    sharkd_json_object_close();
 
     return 0; /* continue */
 }
@@ -4594,10 +4593,9 @@ sharkd_session_process_dumpconf(char *buf, const jsmntok_t *tokens, int count)
 
         sharkd_json_result_prologue(rpcid);
 
-        sharkd_json_value_anyf("prefs", NULL);
-        json_dumper_begin_object(&dumper);
+        sharkd_json_object_open("prefs");
         prefs_modules_foreach(sharkd_session_process_dumpconf_mod_cb, &data);
-        json_dumper_end_object(&dumper);
+        sharkd_json_object_close();
 
         sharkd_json_result_epilogue();
         return;
@@ -4621,10 +4619,9 @@ sharkd_session_process_dumpconf(char *buf, const jsmntok_t *tokens, int count)
 
             sharkd_json_result_prologue(rpcid);
 
-            sharkd_json_value_anyf("prefs", NULL);
-            json_dumper_begin_object(&dumper);
+            sharkd_json_object_open("prefs");
             sharkd_session_process_dumpconf_cb(pref, &data);
-            json_dumper_end_object(&dumper);
+            sharkd_json_object_close();
 
             sharkd_json_result_epilogue();
             return;
@@ -4649,10 +4646,9 @@ sharkd_session_process_dumpconf(char *buf, const jsmntok_t *tokens, int count)
 
         sharkd_json_result_prologue(rpcid);
 
-        sharkd_json_value_anyf("prefs", NULL);
-        json_dumper_begin_object(&dumper);
+        sharkd_json_object_open("prefs");
         prefs_pref_foreach(pref_mod, sharkd_session_process_dumpconf_cb, &data);
-        json_dumper_end_object(&dumper);
+        sharkd_json_object_close();
 
         sharkd_json_result_epilogue();
     }
@@ -4959,7 +4955,7 @@ sharkd_session_process_download(char *buf, const jsmntok_t *tokens, int count)
             sharkd_json_value_string("file", filename);
             sharkd_json_value_string("mime", mime);
 
-            sharkd_json_value_anyf("data", NULL);
+            json_dumper_set_member_name(&dumper, "data");
             json_dumper_begin_base64(&dumper);
             sharkd_rtp_download_decode(&rtp_req);
             json_dumper_end_base64(&dumper);