<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3672/">https://reviewboard.asterisk.org/r/3672/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3672/diff/1/?file=60736#file60736line1461" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/bridge_channel.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void bridge_channel_feature(struct ast_bridge_channel *bridge_channel, const char *starting_dtmf)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1449</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">features</span><span class="o">-></span><span class="n">dtmf_passthrough</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1461</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">ast_channel_lock</span><span class="p">(</span><span class="n">bridge_channel</span><span class="o">-></span><span class="n">chan</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1450</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="cm">/* Stream any collected DTMF to the other channels. */</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1462</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">res</span> <span class="o">></span> <span class="mi">0</span> <span class="o">&&</span> <span class="p">(</span><span class="n">reinterpret_feature</span> <span class="o">=</span> <span class="n">pbx_builtin_getvar_helper</span><span class="p">(</span><span class="n">bridge_channel</span><span class="o">-></span><span class="n">chan</span><span class="p">,</span> <span class="s">"REINTERPRET_FEATURE"</span><span class="p">)))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There is no need to lock the channel if res == 0.
Also assignment inside the if test should be avoided if possible. Pulling the res test outside of the channel lock area allows avoiding the assignment in the if test.
if !res
break
lock(chan)
reinterpret = getvar("REINTERPRET")
if !strlen_zero(reinterpret)
ast_debug("requested reinterpret to")
hook = ao2_find()
if !hook
ast_debug()
unlock(chan)</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On June 25th, 2014, 2:14 p.m. CDT, opticron wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By opticron.</div>
<p style="color: grey;"><i>Updated June 25, 2014, 2:14 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This change allows DTMF hooks to request that an alternative DTMF hook be executed. This behavior is triggered by a new return value for DTMF hooks while the hook to be executed is specified in the REINTERPRET_FEATURE channel variable.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Manual testing as well as the attached unittest. Note that the DTMF and interval tests have already been reviewed as part of https://reviewboard.asterisk.org/r/3649/ and that only the reinterpret DTMF test is new along with helper function reinterpret_callback().</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/tests/test_channel_feature_hooks.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/main/bridge_channel.c <span style="color: grey">(416731)</span></li>
<li>trunk/include/asterisk/bridge_features.h <span style="color: grey">(416731)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3672/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>