<p> Attention is currently required from: Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16634">View Change</a></p><p>8 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File funcs/func_json.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/2dea1e02_b036861b">Patch Set #2, Line 50:</a> <code style="font-family:monospace,monospace"> <para>The JSON_DECODE function parses a JSON string and returns a value by key.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This is not documented enough. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/51d91852_65ccba1a">Patch Set #2, Line 63:</a> <code style="font-family:monospace,monospace"> RAII_VAR(struct ast_json *, json, NULL, ast_json_free);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">RAII_VAR isn't really needed because you can just call ast_json_free after use below, there isn't an […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/9c572a91_06f4e2bd">Patch Set #2, Line 64:</a> <code style="font-family:monospace,monospace"> RAII_VAR(struct ast_json *, jsonresult, NULL, ast_json_free);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This doesn't appear to be used anywhere</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/5dd6f1db_cdfaef02">Patch Set #2, Line 74:</a> <code style="font-family:monospace,monospace"> AST_STANDARD_APP_ARGS(args, data);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This doesn't handle if varname or key are empty.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/c2dd99b1_f71b6b54">Patch Set #2, Line 76:</a> <code style="font-family:monospace,monospace"> varsubst = ast_alloca(strlen(args.varname) + 4);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Document why the 4 with a comment, and this could fail.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/4889b71f_d877c046">Patch Set #2, Line 77:</a> <code style="font-family:monospace,monospace"> sprintf(varsubst, "${%s}", args.varname);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Comment that this is safe because of the above allocation so if someone runs an automatic tool there […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/2c06b943_71dd4fd5">Patch Set #2, Line 80:</a> <code style="font-family:monospace,monospace"> ast_debug(1, "String was empty\n");</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This should be more descriptive if at all possible - such as giving the channel name and the variabl […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16634/comment/a45a5960_1ef093d7">Patch Set #2, Line 83:</a> <code style="font-family:monospace,monospace"> jsonstring = ast_str_buffer(str);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why is there a need for jsonstring at all if you can just use ast_str_buffer?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16634">change 16634</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/16634"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iea60c49a7358dfdc2db60803cdc9a742f808ba2c </div>
<div style="display:none"> Gerrit-Change-Number: 16634 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 16 Nov 2021 12:31:19 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>