<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/2877/">https://reviewboard.asterisk.org/r/2877/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 27th, 2013, 8:49 p.m. UTC, <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/2877/diff/1/?file=46217#file46217line2543" style="color: black; font-weight: bold; text-decoration: underline;">./branches/12/main/cdr.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 handle_parked_call_message(void *data, struct stasis_subscription *sub,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2543</font></th>
<td bgcolor="#c5ffc4" 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="n">new_cdr</span><span class="o">-></span><span class="n">fn_table</span><span class="o">-></span><span class="n">process_parked_channel</span><span class="p">(</span><span class="n">new_cdr</span><span class="p">,</span> <span class="n">payload</span><span class="p">);</span></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;">You should test for NULL function in the table.</pre>
</blockquote>
<p>On September 27th, 2013, 9:55 p.m. UTC, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nope - as the comment states:
"/* This is guaranteed to succeed: the new CDR is created in the
* single state and will be able to handle the parkedcall message
*/
All new CDRs are created in the single state. The single state *must* handle the parkedcall message. It is invalid for it not to, and would be a programming error.
(Note that most handlers for messages do test for whether or not the CDR's state is NULL because they don't know what state the CDR is in - here, we know.)</pre>
</blockquote>
<p>On September 27th, 2013, 10:09 p.m. UTC, <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think that comment should be reworded. The "This is guaranteed to succeed" comment looks more like it is describing the call will always succeed not that the function pointer must exist.</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;">That is true - and it actually means both. I'll update it to say something like:
/* As the new CDR is created in the single state, it is guaranteed
* to have a function for the parked call message and will handle
* the message */</pre>
<br />
<p>- Matt</p>
<br />
<p>On September 24th, 2013, 9:37 p.m. UTC, Matt Jordan 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated Sept. 24, 2013, 9:37 p.m.</i></p>
<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/jira/browse/ASTERISK-22842">ASTERISK-22842</a>
</div>
<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 patch covers two problems:
(1) Currently, when a call is transferred into a parking lot from a bridge (using either the blind transfer or one touch parking mechanisms), the application fails to be set a "Park" in the resulting CDR record for the parked channel. This is due to the ParkedCall message arriving before the BridgeEnter into the parking bridge. The ParkedCall message isn't handled as the CDR for the channel has already been finalized (due to the channel having left its two party bridge), and the BridgeEnter - which creates the new CDR - doesn't have the parking information. This patch modifies the behavior so that reception of a ParkedCall message will - if not handled by a CDR chain - cause a new CDR to be created and put into the Parking state.
Handy table to show the problem:
OLD BEHAVIOR:
Msg State
BridgeEnter (two party) (???)->Bridge
BridgeLeave (two party) Bridge->Finalized
ParkedCall Finalized (doesn't handle the message)
BridgeEnter (parking) Single->Parked (new CDR)
BridgeLeave (parking) Parked->Finalized
NEW:
Msg State
BridgeEnter (two party) (???)->Bridge
BridgeLeave (two party) Bridge->Finalized
ParkedCall Single->Parked (new CDR)
BridgeEnter (parking) Parked
BridgeLeave (parking) Parked->Finalized
(2) It fixes a FRACK that occurred when a channel is originated into a parking space. The DialedPending state - which occurs for both Dialed and Originated channels - assumed that it couldn't handle the parking transitions due to it having a Party B; however, Originated channels don't have a Party B. As such, the existing CDR needs to transition into the parking state - this patch does that.
States:
Msg State
DialBegin Single->Dial
DialEnd Dial->DialPending
ParkedCall DialPending->Parked
</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>./branches/12/main/cdr.c <span style="color: grey">(399746)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2877/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>