<p>Patch set 7:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11083">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/11083/7/main/rtp_engine.c">File main/rtp_engine.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/11083/7/main/rtp_engine.c@3767">Patch Set #7, Line 3767:</a> <code style="font-family:monospace,monospace">   SET_AST_JSON_OBJ(j_res, "txcount", ast_json_integer_create(stats->txcount));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think jj_res should just not be created if any of these fail. This is because you've marked some as required in the JSON API definition, but with this logic some that are required may not actually be present if they can't be allocated. In developer mode this will cause a failure, outside of developer mode it goes against the JSON API spec.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/11083/7/res/ari/resource_channels.c">File res/ari/resource_channels.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/11083/7/res/ari/resource_channels.c@1969">Patch Set #7, Line 1969:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Channel is not locked during this period of time and should be, otherwise you have no guarantee that it won't go away.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/11083/7/res/ari/resource_channels.c@1984">Patch Set #7, Line 1984:</a> <code style="font-family:monospace,monospace">       ret = glue->get_rtp_info(chan, &rtp);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You need to ensure ao2_cleanup is called on rtp or else the RTP instance will leak.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/11083/7/res/ari/resource_channels.c@1985">Patch Set #7, Line 1985:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">        if (ret == AST_RTP_GLUE_RESULT_FORBID) {<br>              ast_ari_response_error(response, 404, "Not Found",<br>                  "RTP info not found");<br>              return;<br>       }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Even if forbid there are cases where rtp will be present, so check for that instead.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/11083">change 11083</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/+/11083"/><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: I4343eec070438cec13f2a4f22e7fd9e574381376 </div>
<div style="display:none"> Gerrit-Change-Number: 11083 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 07 Mar 2019 11:25:42 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>