<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/3580/">https://reviewboard.asterisk.org/r/3580/</a>
</td>
</tr>
</table>
<br />
<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 is one of those cases that really could use a test in the Asterisk Test Suite.
(1) It's dealing with odd behavior in a very old part of chan_sip
(2) The existing behavior works around bad behavior in an old PBX; the new patch properly handles a corner case of a modern system (multiple 183's aren't super common)
(3) The risk of someone coming along and making an inappropriate code change that breaks either the workaround for the Alcatel PBX or Lync is reasonably high
The purpose of a test here is less to verify the correctness of the patch - I believe you tested it with several thousand calls! - but rather to help "future proof" Asterisk against subsequent changes.
I'd be happy to help work some SIPp scenarios into the Asterisk Test Suite - do you think you could construct a SIPp scenario that sends the multiple 183's in the same fashion that Lync does?</pre>
<br />
<p>- Matt Jordan</p>
<br />
<p>On June 3rd, 2014, 9:34 a.m. CDT, Olle E Johansson 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 Olle E Johansson.</div>
<p style="color: grey;"><i>Updated June 3, 2014, 9:34 a.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-15852">ASTERISK-15852</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;">Lync sends the first 183 with SDP using prack. Following 183s have no SDP. The current code treats follow-ups as RINGING, which is a bug.
Red blobs are a bonus in the review, but will not be offered in commit.</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;">Just a few thousand calls. That's all.</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/1.8/channels/chan_sip.c <span style="color: grey">(414989)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3580/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>