<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/1013/">https://reviewboard.asterisk.org/r/1013/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 16th, 2010, 5:14 p.m., <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/1013/diff/1/?file=12919#file12919line545" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/apps/app_macro.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 int _macro_exec(struct ast_channel *chan, const char *data, int exclusive)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">545</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> if (!(ast_check_hangup(chan) & AST_SOFTHANGUP_ASYNCGOTO)) {</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">545</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> if (!(ast_check_hangup(chan) & AST_SOFTHANGUP_ASYNCGOTO)) {/* BUGBUG this is stupid here. ast_check_hangup() returns 0|1 and AST_SOFTHANGUP_ASYNCGOTO is 2. */</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This needs a resolution. I am leaning toward just removing the test since it is always true.
The comment about softhangup and ASYNCGOTO does not seem to apply anymore. (For code 1.6.2 and later anyway.)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You'd have to dig through svn history on this one to find out what happened here. I bet that this got changed as a part of a mass change to move away from doing direct reads of chan->softhangup. If you look back to when the line was first added, there may have been a good reason for it.
Other than coming to a resolution on this part, the rest of this patch looks good to me.</pre>
<br />
<p>- Russell</p>
<br />
<p>On November 16th, 2010, 4:32 p.m., rmudgett wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Russell Bryant, Terry Wilson, and David Vossel.</div>
<div>By rmudgett.</div>
<p style="color: grey;"><i>Updated 2010-11-16 16:32:25</i></p>
<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;">To recreate the problem:
1) Party A calls Party B
2) Invoke CLI "channel redirect" command to redirect channel call leg associated with A.
3) All associated channels are hung up.
Note that if the CLI command were done on the channel call leg associated with B it works.
This regression was a result of the fix for issue 16946 (https://reviewboard.asterisk.org/r/740/).
The regression affects all features that use an async goto to execute the dialplan because of an external event: Channel redirect, AMI redirect, SIP REFER, FAX detection.
The struct ast_channel._softhangup code is a mess. The variable is used for several purposes that do not necessarily result in the call being hung up. I have added doxygen comments to describe how the various _softhangup bits are used. I have corrected all the places where the variable was tested in a non-bit oriented manner.
</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;">The channel redirect command no longer causes the call leg of party A to be hungup.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/view.php?id=18171">18171</a>,
<a href="https://issues.asterisk.org/view.php?id=18185">18185</a>,
<a href="https://issues.asterisk.org/view.php?id=18192">18192</a>,
<a href="https://issues.asterisk.org/view.php?id=18211">18211</a>,
<a href="https://issues.asterisk.org/view.php?id=18230">18230</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/apps/app_macro.c <span style="color: grey">(295221)</span></li>
<li>/branches/1.8/include/asterisk/channel.h <span style="color: grey">(295221)</span></li>
<li>/branches/1.8/include/asterisk/frame.h <span style="color: grey">(295221)</span></li>
<li>/branches/1.8/main/channel.c <span style="color: grey">(295221)</span></li>
<li>/branches/1.8/main/pbx.c <span style="color: grey">(295221)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1013/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>