<p>Patch set 2:<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/10937">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10937/2/res/res_stasis.c">File res/res_stasis.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/10937/2/res/res_stasis.c@1332">Patch Set #2, Line 1332:</a> <code style="font-family:monospace,monospace">                   RAII_VAR(struct stasis_app *, next_app, NULL, ao2_cleanup);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Needless RAII_VAR usage :P</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10937/2/res/res_stasis.c@1384">Patch Set #2, Line 1384:</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 (next_app) {<br>                                       ast_log(LOG_ERROR, "Could not move to Stasis app '%s' - not registered\n",<br>                                          control_next_app(control));<br>                           } else {<br>                                      ast_log(LOG_ERROR, "Could not move to Stasis app '%s' - not active\n",<br>                                              control_next_app(control));<br>                           }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm wondering about this from a usage perspective - as a user using the API I think there needs to be a message or something that indicates it couldn't be moved, so that the application can then react accordingly. If there's no event then it could be complex in the application to handle it.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Ideally the HTTP response would indicate this, but I don't know if this is possible.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10937/2/res/stasis/control.h">File res/stasis/control.h:</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/10937/2/res/stasis/control.h@41">Patch Set #2, Line 41:</a> <code style="font-family:monospace,monospace"> * \note This function inherits app's ref rather than bumping app!</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about in an error case? It seems as though it's possible to leak the app in some cases.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/10937">change 10937</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/10937"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I43d12b10045a98a8d42541889b85695be26f288a </div>
<div style="display:none"> Gerrit-Change-Number: 10937 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: Joshua C. Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 31 Jan 2019 17:15:03 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>