<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/4160/">https://reviewboard.asterisk.org/r/4160/</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 11th, 2014, 1:43 p.m. EST, <b>Mark Michelson</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;">If I understand the purpose of p->refer correctly, it's supposed to be details relating to a specific REFER (or REFER-esque in some cases) transaction. I think that any in-dialog places where p->refer may be allocated, the previous p->refer should be freed. In addition to the transmit_refer() change you have, this would mean that handle_request_refer() and get_also_info() should free p->refer and then allocate a new one.
Honestly, the best way to do this is perhaps to just have sip_refer_alloc() destroy the old p->refer and then allocate a new one.</pre>
</blockquote>
</blockquote>
<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 seams reasonable to me. Now that this will change existing behaviour, I want to run it through testsuite tests/channels/SIP. Once that passes I'll update the review. Unless I'm misunderstanding, the call to sip_refer_alloc from handle_request_invite should destroy any existing p->refer as well?</pre>
<br />
<p>- Corey</p>
<br />
<p>On November 10th, 2014, 1:25 a.m. EST, Corey Farrell 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 Corey Farrell.</div>
<p style="color: grey;"><i>Updated Nov. 10, 2014, 1:25 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-15242">ASTERISK-15242</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;">If transmit_refer is called when p->refer is already allocated, it will leak the previous allocation. I checked for all occurrences of sip_refer_alloc, found that transmit_refer was the only caller that didn't check p->refer first. This change moves the check for !p->refer to sip_refer_alloc.
I made transmit_refer destroy any previous p->refer so it will have a clean structure after reallocation like it does currently. Unsure if it's needed, but the little bit of extra processing is worth keeping this fix low risk.
The change is slightly different in 12+, as p->refer->refer_call only exists in 11.</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;">Compiled, visual inspection.</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/11/channels/chan_sip.c <span style="color: grey">(427666)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4160/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>