<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/2683/">https://reviewboard.asterisk.org/r/2683/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2013, 6:04 p.m. CDT, <b>Mark Michelson</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/2683/diff/1/?file=41595#file41595line926" style="color: black; font-weight: bold; text-decoration: underline;">trunk/res/res_sip_session.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; ">struct ast_sip_session *ast_sip_session_alloc(struct ast_sip_endpoint *endpoint, pjsip_inv_session *inv_session)</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">926</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_rtp_dtls_cfg_copy</span><span class="p">(</span><span class="o">&</span><span class="n">endpoint</span><span class="o">-></span><span class="n">dtls_cfg</span><span class="p">,</span> <span class="o">&</span><span class="n">session</span><span class="o">-></span><span class="n">dtls_cfg</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;">I can't see a good reason to perform this copy. I couldn't see any place in the code where the dtls_cfg on the session would get changed after being copied from the endpoint, so why not just use the endpoint's config directly? Any place you have a session, you have its corresponding endpoint to get the config from.</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;">Ah, when I first started this patch I assumed that things were being set on the config as DTLS information was being processed and when I found out otherwise I forgot to get rid of it. Good catch. The only place where this is not the case is the encryption suite which is controlled directly by the endpoint configuration anyway and so will be consistent.</pre>
<br />
<p>- opticron</p>
<br />
<p>On July 18th, 2013, 8:40 a.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.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 July 18, 2013, 8:40 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-21419">ASTERISK-21419</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 introduces DTLS-SRTP support to chan_pjsip and the options necessary to configure it including an option to allow choosing between 32 and 80 byte SRTP tag lengths.
During the implementation and testing of this patch, three other bugs were found and their fixes are included with this patch. The two in chan_sip were a segfault relating to DTLS setup and mistaken call rejection. The third bug fix prevents chan_pjsip from attempting to perform bridge optimization between two endpoints if either of them is running any form of SRTP.</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;">Hand testing against chan_sip and verification that behavior does not change when replaced with chan_sip.</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/channels/chan_gulp.c <span style="color: grey">(394643)</span></li>
<li>trunk/channels/chan_sip.c <span style="color: grey">(394643)</span></li>
<li>trunk/include/asterisk/res_sip.h <span style="color: grey">(394643)</span></li>
<li>trunk/include/asterisk/res_sip_session.h <span style="color: grey">(394643)</span></li>
<li>trunk/res/res_sip.c <span style="color: grey">(394643)</span></li>
<li>trunk/res/res_sip/sip_configuration.c <span style="color: grey">(394643)</span></li>
<li>trunk/res/res_sip_sdp_rtp.c <span style="color: grey">(394643)</span></li>
<li>trunk/res/res_sip_session.c <span style="color: grey">(394643)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2683/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>