<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/2228/">https://reviewboard.asterisk.org/r/2228/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 20th, 2012, 10:54 a.m., <b>Matt Jordan</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/2228/diff/4/?file=32477#file32477line369" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/res/res_srtp.c</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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; ">tryagain:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">369</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">i<span class="hl">f</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n">srtp_create</span><span class="p">(</span><span class="o">&</span><span class="n">srtp</span><span class="o">-></span><span class="n">session</span><span class="p">,</span> <span class="o">&</span><span class="n">policy</span><span class="o">-></span><span class="n">sp</span><span class="p">)</span><span class="hl"> </span><span class="o"><span class="hl">==</span></span><span class="hl"> </span><span class="n"><span class="hl">err_status_ok</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="p"><span class="hl">{</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">369</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">i<span class="hl">nt</span></span><span class="hl"> </span><span class="n"><span class="hl">res</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n">srtp_create</span><span class="p">(</span><span class="o">&</span><span class="n">srtp</span><span class="o">-></span><span class="n">session</span><span class="p">,</span> <span class="o">&</span><span class="n">policy</span><span class="o">-></span><span class="n">sp</span><span class="p">)<span class="hl">;</span></span></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">370</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">res</span> <span class="o">==</span> <span class="n">err_status_ok</span><span class="p">)</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'm not a fan of variables in an inner block scope hiding variables in outer blocks. In this case, the declaration of res here hides the previous declaration of res at the beginning of the function. In general, it this can make the code more difficult to maintain.
Unfortunately, as you probably noticed, re-using res here is also not advisable since the original value is needed outside of this scope. You're probably better off just making a new variable (res_create?).</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;">Alright, I changed it to res_srtp_create and am now using the new variable for the if statement and the log message. I'm not going to repost right now since it's a small change.
The only reason I used res was because I failed to notice that it was used outside of the scope I was working in, so oops.</pre>
<br />
<p>- jrose</p>
<br />
<p>On December 20th, 2012, 10:32 a.m., jrose wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/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, Mark Michelson and Matt Jordan.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Dec. 20, 2012, 10:32 a.m.</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;">If srtp_create fails, session we create for temp will either not be created or else will be dealloced within srtp_create. At present, when we then run ast_srtp_destroy, attempting to dealloc this can cause Asterisk to crash. This patch addresses that by setting the session pointer to NULL so ast_srtp_destroy doesn't attempt to dealloc it.
As one might expect, this patch doesn't resolve the reporter's problems with actually setting up an srtp enabled SIP call, but it does fix a crash which shouldn't be happening.</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 reporter has tested the patch and confirmed that it eliminates the crash.</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/jira/browse/ASTERISK-20499">ASTERISK-20499</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/res/res_srtp.c <span style="color: grey">(378146)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2228/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>