[Asterisk-code-review] func_json: Enhance parsing capabilities of JSON_DECODE (asterisk[master])

[N A]

Attention is currently required from: Kevin Harwell.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18012 )

Change subject: func_json: Enhance parsing capabilities of JSON_DECODE
......................................................................


Patch Set 4:

(1 comment)

File funcs/func_json.c:

https://gerrit.asterisk.org/c/asterisk/+/18012/comment/cd877ee7_bc21f377 
PS4, Line 90: static int parse_node(char **key, char *currentkey, char *nestchar, int count, struct ast_json *json, char *buf, size_t len)
            : {
            : 	const char *result = NULL;
            : 	char *previouskey;
            : 	struct ast_json *jsonval = json;
            : 
            : 	snprintf(buf, len, "%s", ""); /* clear the buffer from previous round if necessary */
            : 	if (!json) { /* no error or warning should be thrown */
            : 		ast_debug(1, "Could not find key '%s' in parsed JSON\n", currentkey);
            : 		return -1;
            : 	}
            : 
            : 	switch(ast_json_typeof(jsonval)) {
            : 		unsigned long int size;
            : 		int r;
            : 		char *result2;
            : 
            : 		case AST_JSON_STRING:
            : 			ast_debug(1, "Got JSON string\n");
            : 			result = ast_json_string_get(jsonval);
            : 			snprintf(buf, len, "%s", result);
            : 			break;
            : 		case AST_JSON_INTEGER:
            : 			ast_debug(1, "Got JSON integer\n");
            : 			r = ast_json_integer_get(jsonval);
            : 			snprintf(buf, len, "%d", r); /* the snprintf below is mutually exclusive with this one */
            : 			break;
            : 		case AST_JSON_ARRAY:
            : 			ast_debug(1, "Got JSON array\n");
            : 			previouskey = currentkey;
            : 			currentkey = strsep(key, nestchar); /* retrieve the desired index */
            : 			size = ast_json_array_size(jsonval);
            : 			ast_debug(1, "Parsed JSON array of size %lu, key: %s\n", size, currentkey);
            : 			if (!currentkey) { /* this is the end, so just dump the array */
            : 				if (count) {
            : 					ast_debug(1, "No key on which to index in the array, so returning count: %lu\n", size);
            : 					snprintf(buf, len, "%lu", size);
            : 					return 0;
            : 				} else {
            : 					char *jsonarray;
            : 					ast_debug(1, "No key on which to index in the array, so dumping '%s' array\n", previouskey);
            : 					jsonarray = ast_json_dump_string(jsonval);
            : 					ast_copy_string(buf, jsonarray, len);
            : 					ast_json_free(jsonarray);
            : 				}
            : 			} else if (ast_str_to_int(currentkey, &r) || r < 0) {
            : 				ast_debug(1, "Requested index '%s' is not numeric or is invalid\n", currentkey);
            : 			} else if (r >= size) {
            : 				ast_debug(1, "Requested index '%d' does not exist in parsed array\n", r);
            : 			} else {
            : 				struct ast_json *json2 = ast_json_array_get(jsonval, r);
            : 				if (!json2) {
            : 					ast_debug(1, "Array index %d contains empty item\n", r);
            : 					return -1;
            : 				}
            : 				previouskey = currentkey;
            : 				currentkey = strsep(key, nestchar); /* get the next subkey */
            : 				ast_debug(1, "Recursing on index %d in array (key was '%s' and is now '%s')\n", r, previouskey, currentkey);
            : 				/* json2 is a borrowed ref. That's fine, since json won't get freed until recursing is over */
            : 				/* If there are keys remaining, then parse the next object we can get. Otherwise, just dump the child */
            : 				if (parse_node(key, currentkey, nestchar, count, currentkey ? ast_json_object_get(json2, currentkey) : json2, buf, len)) { /* recurse on this node */
            : 					return -1;
            : 				}
            : 			}
            : 			break;
            : 		default:
            : 			ast_debug(1, "Got generic JSON object\n");
            : 			result2 = ast_json_dump_string(jsonval);
            : 			snprintf(buf, len, "%s", result2);
            : 			ast_json_free(result2);
            : 	}
            : 	return 0;
            : }
> I think this function can be simplified, made more generic, and then moved into json.c. […]
Sorry it took so long to get back to this.
I'm wondering, is there a reason that AST_JSON_INTEGER is not handled in your ast_json_find_by_key example? Right now that case is handled in parse_node. It seems like here, it would fall through to the generic ast_json_dump_string, which wouldn't work on an integer value. Or have I missed something here?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18012
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I603940b216a3911b498fc6583b18934011ef5d5b
Gerrit-Change-Number: 18012
Gerrit-PatchSet: 4
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <default.enum at gmail.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Kevin Harwell <default.enum at gmail.com>
Gerrit-Comment-Date: Sat, 06 Aug 2022 21:01:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <default.enum at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220806/a1c9b9e4/attachment.html>