[Asterisk-code-review] func_json: Adds JSON_DECODE function (asterisk[master])

[N A]

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

Change subject: func_json: Adds JSON_DECODE function
......................................................................


Patch Set 5:

(8 comments)

File funcs/func_json.c:

https://gerrit.asterisk.org/c/asterisk/+/16634/comment/2dea1e02_b036861b 
PS2, Line 50: 			<para>The JSON_DECODE function parses a JSON string and returns a value by key.
> This is not documented enough. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/51d91852_65ccba1a 
PS2, Line 63: 	RAII_VAR(struct ast_json *, json, NULL, ast_json_free);
> RAII_VAR isn't really needed because you can just call ast_json_free after use below, there isn't an […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/9c572a91_06f4e2bd 
PS2, Line 64: 	RAII_VAR(struct ast_json *, jsonresult, NULL, ast_json_free);
> This doesn't appear to be used anywhere
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/5dd6f1db_cdfaef02 
PS2, Line 74: 	AST_STANDARD_APP_ARGS(args, data);
> This doesn't handle if varname or key are empty.
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/c2dd99b1_f71b6b54 
PS2, Line 76: 	varsubst = ast_alloca(strlen(args.varname) + 4);
> Document why the 4 with a comment, and this could fail.
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/4889b71f_d877c046 
PS2, Line 77: 	sprintf(varsubst, "${%s}", args.varname);
> Comment that this is safe because of the above allocation so if someone runs an automatic tool there […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/2c06b943_71dd4fd5 
PS2, Line 80: 		ast_debug(1, "String was empty\n");
> This should be more descriptive if at all possible - such as giving the channel name and the variabl […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16634/comment/a45a5960_1ef093d7 
PS2, Line 83: 	jsonstring = ast_str_buffer(str);
> Why is there a need for jsonstring at all if you can just use ast_str_buffer?
It's used thrice, so it seemed like it made sense. Should I call ast_str_buffer 3 times instead? Wasn't sure how much overhead that would add.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Iea60c49a7358dfdc2db60803cdc9a742f808ba2c
Gerrit-Change-Number: 16634
Gerrit-PatchSet: 5
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Tue, 16 Nov 2021 12:31:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211116/f3ce8b6c/attachment-0001.html>