[spp] [PATCH 04/13] shared/sec: change order of args of JSON fmtters

[yasufum.o@gmail.com]

From: Yasufumi Ogawa <yasufum.o@gmail.com>

Formatter functions defined in `json_helper.h` takes three args, a pair
of name and value of JSON and a string buffer as a placeholder. But the
order of args `func(name, buf, value)` is not well. This update is to
move `buf` at first as similar to other C functions such as sprintf().

Signed-off-by: Yasufumi Ogawa <yasufum.o@gmail.com>
---
 .../secondary/spp_worker_th/cmd_runner.c      | 61 +++++++------
 .../secondary/spp_worker_th/json_helper.c     | 61 +++++++------
 .../secondary/spp_worker_th/json_helper.h     | 85 ++++++++++++++++---
 3 files changed, 138 insertions(+), 69 deletions(-)

diff --git a/src/shared/secondary/spp_worker_th/cmd_runner.c b/src/shared/secondary/spp_worker_th/cmd_runner.c
index 423774b..e41fd85 100644
--- a/src/shared/secondary/spp_worker_th/cmd_runner.c
+++ b/src/shared/secondary/spp_worker_th/cmd_runner.c
@@ -21,6 +21,7 @@
 #define RTE_LOGTYPE_WK_CMD_RUNNER RTE_LOGTYPE_USER1
 
 /* request message initial size */
+#define CMD_TAG_APPEND_SIZE 16
 #define CMD_ERR_MSG_LEN 128
 #define CMD_REQ_BUF_INIT_SIZE 2048
 #define CMD_RES_BUF_INIT_SIZE 2048
@@ -741,7 +742,7 @@ static int
 append_result_value(const char *name, char **output, void *tmp)
 {
 	const struct cmd_result *result = tmp;
-	return append_json_str_value(name, output, result->result);
+	return append_json_str_value(output, name, result->result);
 }
 
 /* append error details for JSON format */
@@ -764,14 +765,13 @@ append_error_details_value(const char *name, char **output, void *tmp)
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_str_value("message", &tmp_buff,
-			result->err_msg);
+	ret = append_json_str_value(&tmp_buff, "message", result->err_msg);
 	if (unlikely(ret < 0)) {
 		spp_strbuf_free(tmp_buff);
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -781,7 +781,7 @@ static int
 add_client_id(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_int_value(name, output, sppwk_get_client_id());
+	return append_json_int_value(output, name, sppwk_get_client_id());
 }
 
 /* append a list of interface numbers */
@@ -816,7 +816,7 @@ static int
 append_process_type_value(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
-	return append_json_str_value(name, output,
+	return append_json_str_value(output, name,
 			SPPWK_PROC_TYPE_LIST[sppwk_get_proc_type()]);
 }
 
@@ -849,7 +849,7 @@ add_interface(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff);
+	ret = append_json_array_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -859,16 +859,16 @@ static int
 append_vlan_value(char **output, const int ope, const int vid, const int pcp)
 {
 	int ret = SPP_RET_OK;
-	ret = append_json_str_value("operation", output,
+	ret = append_json_str_value(output, "operation",
 			PORT_ABILITY_STAT_LIST[ope]);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
-	ret = append_json_int_value("id", output, vid);
+	ret = append_json_int_value(output, "id", vid);
 	if (unlikely(ret < 0))
 		return SPP_RET_NG;
 
-	ret = append_json_int_value("pcp", output, pcp);
+	ret = append_json_int_value(output, "pcp", pcp);
 	if (unlikely(ret < 0))
 		return SPP_RET_NG;
 
@@ -922,7 +922,7 @@ append_vlan_block(const char *name, char **output,
 			return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -943,7 +943,7 @@ append_port_block(char **output, const struct sppwk_port_idx *port,
 	}
 
 	spp_format_port_string(port_str, port->iface_type, port->iface_no);
-	ret = append_json_str_value("port", &tmp_buff, port_str);
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
@@ -954,7 +954,7 @@ append_port_block(char **output, const struct sppwk_port_idx *port,
 	if (unlikely(ret < SPP_RET_OK))
 		return SPP_RET_NG;
 
-	ret = append_json_block_brackets("", output, tmp_buff);
+	ret = append_json_block_brackets(output, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -982,7 +982,7 @@ append_port_array(const char *name, char **output, const int num,
 			return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff);
+	ret = append_json_array_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -1020,17 +1020,17 @@ append_core_element_value(
 	 * TODO(yasufum) change ambiguous "core" to more specific one such as
 	 * "worker-lcores" or "slave-lcores".
 	 */
-	ret = append_json_uint_value("core", &tmp_buff, lcore_id);
+	ret = append_json_uint_value(&tmp_buff, "core", lcore_id);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
 	if (unuse_flg) {
-		ret = append_json_str_value("name", &tmp_buff, name);
+		ret = append_json_str_value(&tmp_buff, "name", name);
 		if (unlikely(ret < 0))
 			return ret;
 	}
 
-	ret = append_json_str_value("type", &tmp_buff, type);
+	ret = append_json_str_value(&tmp_buff, "type", type);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
@@ -1046,7 +1046,7 @@ append_core_element_value(
 			return ret;
 	}
 
-	ret = append_json_block_brackets("", &buff, tmp_buff);
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	params->output = buff;
 	return ret;
@@ -1058,7 +1058,7 @@ add_master_lcore(const char *name, char **output,
 		void *tmp __attribute__ ((unused)))
 {
 	int ret = SPP_RET_NG;
-	ret = append_json_int_value(name, output, rte_get_master_lcore());
+	ret = append_json_int_value(output, name, rte_get_master_lcore());
 	return ret;
 }
 
@@ -1087,7 +1087,7 @@ add_core(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, itr_params.output);
+	ret = append_json_array_brackets(output, name, itr_params.output);
 	spp_strbuf_free(itr_params.output);
 	return ret;
 }
@@ -1119,8 +1119,7 @@ append_classifier_element_value(
 
 	spp_format_port_string(port_str, port->iface_type, port->iface_no);
 
-	ret = append_json_str_value("type", &tmp_buff,
-			CLS_TYPE_A_LIST[type]);
+	ret = append_json_str_value(&tmp_buff, "type", CLS_TYPE_A_LIST[type]);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
@@ -1137,15 +1136,15 @@ append_classifier_element_value(
 		break;
 	}
 
-	ret = append_json_str_value("value", &tmp_buff, value_str);
+	ret = append_json_str_value(&tmp_buff, "value", value_str);
 	if (unlikely(ret < 0))
 		return ret;
 
-	ret = append_json_str_value("port", &tmp_buff, port_str);
+	ret = append_json_str_value(&tmp_buff, "port", port_str);
 	if (unlikely(ret < SPP_RET_OK))
 		return ret;
 
-	ret = append_json_block_brackets("", &buff, tmp_buff);
+	ret = append_json_block_brackets(&buff, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	params->output = buff;
 	return ret;
@@ -1185,7 +1184,7 @@ add_classifier_table(const char *name, char **output,
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_array_brackets(name, output, itr_params.output);
+	ret = append_json_array_brackets(output, name, itr_params.output);
 	spp_strbuf_free(itr_params.output);
 	return ret;
 }
@@ -1319,7 +1318,7 @@ append_command_results_value(const char *name, char **output,
 			return SPP_RET_NG;
 		}
 
-		ret = append_json_block_brackets("", &tmp_buff2, tmp_buff1);
+		ret = append_json_block_brackets(&tmp_buff2, "", tmp_buff1);
 		if (unlikely(ret < 0)) {
 			spp_strbuf_free(tmp_buff1);
 			spp_strbuf_free(tmp_buff2);
@@ -1328,7 +1327,7 @@ append_command_results_value(const char *name, char **output,
 
 	}
 
-	ret = append_json_array_brackets(name, output, tmp_buff2);
+	ret = append_json_array_brackets(output, name, tmp_buff2);
 	spp_strbuf_free(tmp_buff1);
 	spp_strbuf_free(tmp_buff2);
 	return ret;
@@ -1355,7 +1354,7 @@ append_info_value(const char *name, char **output)
 		return SPP_RET_NG;
 	}
 
-	ret = append_json_block_brackets(name, output, tmp_buff);
+	ret = append_json_block_brackets(output, name, tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	return ret;
 }
@@ -1394,7 +1393,7 @@ send_decode_error_response(int *sock,
 				"(name = decode_error_response)\n");
 		return;
 	}
-	ret = append_json_block_brackets("", &msg, tmp_buff);
+	ret = append_json_block_brackets(&msg, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	if (unlikely(ret < SPP_RET_OK)) {
 		spp_strbuf_free(msg);
@@ -1477,7 +1476,7 @@ send_result_spp_ctl(int *sock,
 				"allocate error. (name = result_response)\n");
 		return;
 	}
-	ret = append_json_block_brackets("", &msg, tmp_buff);
+	ret = append_json_block_brackets(&msg, "", tmp_buff);
 	spp_strbuf_free(tmp_buff);
 	if (unlikely(ret < SPP_RET_OK)) {
 		spp_strbuf_free(msg);
diff --git a/src/shared/secondary/spp_worker_th/json_helper.c b/src/shared/secondary/spp_worker_th/json_helper.c
index 4c1baa3..20badb0 100644
--- a/src/shared/secondary/spp_worker_th/json_helper.c
+++ b/src/shared/secondary/spp_worker_th/json_helper.c
@@ -7,27 +7,27 @@
 
 #define RTE_LOGTYPE_WK_JSON_HELPER RTE_LOGTYPE_USER1
 
+/* Add a comma to given JSON string, or nothing if the end of the msg. */
 int
 append_json_comma(char **output)
 {
 	*output = spp_strbuf_append(*output, ", ", strlen(", "));
 	if (unlikely(*output == NULL)) {
-		RTE_LOG(ERR, WK_JSON_HELPER,
-				"JSON's comma failed to add.\n");
+		RTE_LOG(ERR, WK_JSON_HELPER, "Failed to add comma.\n");
 		return SPP_RET_NG;
 	}
 
 	return SPP_RET_OK;
 }
 
-/* append data of unsigned integral type for JSON format */
+/* Add a uint value to given JSON string. */
 int
-append_json_uint_value(const char *name, char **output, unsigned int value)
+append_json_uint_value(char **output, const char *name, unsigned int value)
 {
 	int len = strlen(*output);
-	/* extend the buffer */
+
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+			strlen(name) + JSON_APPEND_LEN*2);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's numeric format failed to add. "
@@ -40,14 +40,14 @@ append_json_uint_value(const char *name, char **output, unsigned int value)
 	return SPP_RET_OK;
 }
 
-/* append data of integral type for JSON format */
+/* Add an int value to given JSON string. */
 int
-append_json_int_value(const char *name, char **output, int value)
+append_json_int_value(char **output, const char *name, int value)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + CMD_TAG_APPEND_SIZE*2);
+			strlen(name) + JSON_APPEND_LEN*2);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's numeric format failed to add. "
@@ -60,66 +60,77 @@ append_json_int_value(const char *name, char **output, int value)
 	return SPP_RET_OK;
 }
 
-/* append data of string type for JSON format */
+/* Add a string value to given JSON string. */
 int
-append_json_str_value(const char *name, char **output, const char *str)
+append_json_str_value(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's string format failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	sprintf(&(*output)[len], JSON_APPEND_VALUE("\"%s\""),
-			JSON_APPEND_COMMA(len), name, str);
+			JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
 
-/* append brackets of the array for JSON format */
+/**
+ * Add an entry of array by surrounding given value with '[' and ']' to make
+ * it an array entry. The added entry `"key": [ val ]"` is defined as macro
+ * `JSON_APPEND_ARRAY`.
+ */
 int
-append_json_array_brackets(const char *name, char **output, const char *str)
+append_json_array_brackets(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's square bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	sprintf(&(*output)[len], JSON_APPEND_ARRAY,
-			JSON_APPEND_COMMA(len), name, str);
+			JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
 
-/* append brackets of the blocks for JSON format */
+/**
+ * Add an entry of hash by surrounding given value with '{' and '}' to make
+ * it a hash entry. The added entry `"key": { val }"` is defined as macro
+ * `JSON_APPEND_BLOCK`.
+ *
+ * This function is also used to make a block without key `{ val }` if given
+ * key is `""`.
+ */
 int
-append_json_block_brackets(const char *name, char **output, const char *str)
+append_json_block_brackets(char **output, const char *name, const char *val)
 {
 	int len = strlen(*output);
 	/* extend the buffer */
 	*output = spp_strbuf_append(*output, "",
-			strlen(name) + strlen(str) + CMD_TAG_APPEND_SIZE);
+			strlen(name) + strlen(val) + JSON_APPEND_LEN);
 	if (unlikely(*output == NULL)) {
 		RTE_LOG(ERR, WK_JSON_HELPER,
 				"JSON's curly bracket failed to add. "
-				"(name = %s, str = %s)\n", name, str);
+				"(name = %s, val= %s)\n", name, val);
 		return SPP_RET_NG;
 	}
 
 	if (name[0] == '\0')
 		sprintf(&(*output)[len], JSON_APPEND_BLOCK_NONAME,
-				JSON_APPEND_COMMA(len), name, str);
+				JSON_APPEND_COMMA(len), name, val);
 	else
 		sprintf(&(*output)[len], JSON_APPEND_BLOCK,
-				JSON_APPEND_COMMA(len), name, str);
+				JSON_APPEND_COMMA(len), name, val);
 	return SPP_RET_OK;
 }
diff --git a/src/shared/secondary/spp_worker_th/json_helper.h b/src/shared/secondary/spp_worker_th/json_helper.h
index f286404..0bfe0bf 100644
--- a/src/shared/secondary/spp_worker_th/json_helper.h
+++ b/src/shared/secondary/spp_worker_th/json_helper.h
@@ -7,32 +7,91 @@
 
 #include "cmd_utils.h"
 
-#define CMD_TAG_APPEND_SIZE 16
+/* TODO(yasufum) revise name considering the usage. */
+#define JSON_APPEND_LEN 16
 
+/* Add comma at the end of JSON statement, or do nothing. */
 #define JSON_APPEND_COMMA(flg)    ((flg)?", ":"")
 
+/**
+ * Defines of macro for JSON formatter. There are two ro three strings, the
+ * first one is placehodler for JSON msg, second one is key and third one is
+ * its value. Key and value are appended at the end of the placeholder.
+ */
 #define JSON_APPEND_VALUE(format) "%s\"%s\": "format
-
 #define JSON_APPEND_ARRAY         "%s\"%s\": [ %s ]"
-
 #define JSON_APPEND_BLOCK_NONAME  "%s%s{ %s }"
 #define JSON_APPEND_BLOCK         "%s\"%s\": { %s }"
 
+/**
+ * Add a comma to given JSON string, or nothing if the end of the statement.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ */
 int append_json_comma(char **output);
 
-int append_json_uint_value(const char *name, char **output,
-		unsigned int value);
+/**
+ * Add a uint value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val Uint value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_uint_value(char **output, const char *name, unsigned int val);
 
-int append_json_int_value(const char *name, char **output,
-		int value);
+/**
+ * Add an int value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val Int value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_int_value(char **output, const char *name, int val);
 
-int append_json_str_value(const char *name, char **output,
-		const char *str);
+/**
+ * Add a string value to given JSON string.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_str_value(char **output, const char *name, const char *val);
 
-int append_json_array_brackets(const char *name, char **output,
-		const char *str);
+/**
+ * Add an entry of array by surrounding given value with '[' and ']' to make
+ * it an array entry. The added entry `"key": [ val ]"` is defined as macro
+ * `JSON_APPEND_ARRAY`.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_array_brackets(char **output, const char *name,
+		const char *val);
 
-int append_json_block_brackets(const char *name, char **output,
-		const char *str);
+/**
+ * Add an entry of hash by surrounding given value with '{' and '}' to make
+ * it a hash entry. The added entry `"key": { val }"` is defined as macro
+ * `JSON_APPEND_BLOCK`.
+ *
+ * This function is also used to make a block without key `{ val }` if given
+ * key is `""`.
+ *
+ * @param[in,out] output Placeholder of JSON msg.
+ * @param[in] name Name as a key.
+ * @param[in] val String value of the key.
+ * @retval SPP_RET_OK if succeeded.
+ * @retval SPP_RET_NG if failed.
+ */
+int append_json_block_brackets(char **output, const char *name,
+		const char *val);
 
 #endif
-- 
2.17.1