<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/1993/">https://reviewboard.asterisk.org/r/1993/</a>
</td>
</tr>
</table>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line1383" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 clear_sip_domains(void);</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1383</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">static</span> <span class="kt">void</span> <span class="n">add_diversion<span class="hl">_header</span></span><span class="p">(</span><span class="k">struct</span> <span class="n">sip_request</span> <span class="o">*</span><span class="n">req</span><span class="p">,</span> <span class="k">struct</span> <span class="n">sip_pvt</span> <span class="o">*</span><span class="n">pvt</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1383</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">static</span> <span class="kt">void</span> <span class="n">add_diversion</span><span class="p">(</span><span class="k">struct</span> <span class="n">sip_request</span> <span class="o">*</span><span class="n">req</span><span class="p">,</span> <span class="k">struct</span> <span class="n">sip_pvt</span> <span class="o">*</span><span class="n">pvt</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As a personal preference, I think function names should be more specific rather than less specific unless they are just getting cumbersomely large (which I don't think add_diversion_header would be considered large).
The same note applies to add_date and add_expires. I think they would be better as add_date_header and add_expires_header. Functions that mess with headers I feel should have the word 'header' in there somewhere.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line7453" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 struct ast_channel *sip_new(struct sip_pvt *i, int state, const char *title, const char *linkedid, struct ast_callid *callid)</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">7441</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">const</span> <span class="kt">char</span> <span class="o">*</span><span class="n">line</span><span class="p">;</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7449</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7442</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7450</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">while</span> <span class="p">(</span><span class="o">*</span><span class="n">start</span> <span class="o"><</span> <span class="p">(</span><span class="n">req</span><span class="o">-></span><span class="n">sdp_start</span> <span class="o">+</span> <span class="n">req</span><span class="o">-></span><span class="n">sdp_count</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7443</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">while</span> <span class="p">(</span><span class="o">*</span><span class="n">start</span> <span class="o"><</span> <span class="p">(</span><span class="n">req</span><span class="o">-></span><span class="n">sdp_start</span> <span class="o">+</span> <span class="n">req</span><span class="o">-></span><span class="n">sdp_count</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7451</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl">                </span><span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="kt"><span class="hl">char</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">r</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n"><span class="hl">get_body_by_</span>line</span><span class="p"><span class="hl">(</span></span><span class="n">REQ_OFFSET_TO_STR</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">line</span><span class="p">[(</span><span class="o">*</span><span class="n">start</span><span class="p">)</span><span class="o">++</span><span class="p">])<span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">name</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">len</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="sc"><span class="hl">'='</span></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">7444</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">line</span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n">REQ_OFFSET_TO_STR</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">line</span><span class="p">[(</span><span class="o">*</span><span class="n">start</span><span class="p">)</span><span class="o">++</span><span class="p">]);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7452</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">(</span><span class="n">r</span><span class="p">[</span><span class="mi">0</span><span class="p">]</span> <span class="o">!=</span> <span class="sc">'\0'</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7445</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strncasecmp</span><span class="p">(</span><span class="n">line</span><span class="p">,</span> <span class="n">name</span><span class="p">,</span> <span class="n">len</span><span class="p">)</span> <span class="o">&&</span> <span class="n">line</span><span class="p">[</span><span class="n">len</span><span class="p">]</span> <span class="o">==</span> <span class="sc">'='</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7453</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="k">return</span> <span class="n">r</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7446</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="k">return</span> <span class="n">ast_skip_blanks</span><span class="p">(</span><span class="n">line</span> <span class="o">+</span> <span class="n">len</span> <span class="o">+</span> <span class="mi">1</span><span class="p">);</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">7447</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This seems to be a minor optimization (to avoid redeclaring the character pointer) that doesn't really have anything to do with other stuff in the patch. I'd recommend splitting optimizations like this into a separate patch.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line15220" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 enum check_auth_result check_auth(struct sip_pvt *p, struct sip_request *req, const char *username,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">15159</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">authtoken</span> <span class="o">=</span> <span class="n">sip_get_header</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">reqheader</span><span class="p">);</span><span class="ew">        </span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">15194</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">authtoken</span> <span class="o">=</span> <span class="n">sip_get_header</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">reqheader</span><span class="p">);</span><span class="ew">        </span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">15160</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="n">ignore</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n"><span class="hl">randdata</span></span><span class="p">)</span> <span class="o">&&</span> <span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">authtoken</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">15195</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="n"><span class="hl">req</span></span><span class="o"><span class="hl">-></span></span><span class="n">ignore</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n"><span class="hl">nonce</span></span><span class="p">)</span> <span class="o">&&</span> <span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">authtoken</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just another bit of inappropriate white space that you got near.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line16227" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 enum sip_get_dest_result get_destination(struct sip_pvt *p, struct sip_request *oreq, int *cc_recall_core_id)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16166</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="n">req</span><span class="o">-></span><span class="n">rl<span class="hl">P</span>art2</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16201</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">if</span> <span class="p">(</span><span class="n">req</span><span class="o">-></span><span class="n">rl<span class="hl">p</span>art2</span><span class="p">)</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16167</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_copy_string</span><span class="p">(</span><span class="n">tmp</span><span class="p">,</span> <span class="n">REQ_OFFSET_TO_STR</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">rl<span class="hl">P</span>art2</span><span class="p">),</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">tmp</span><span class="p">));</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16202</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_copy_string</span><span class="p">(</span><span class="n">tmp</span><span class="p">,</span> <span class="n">REQ_OFFSET_TO_STR</span><span class="p">(</span><span class="n">req</span><span class="p">,</span> <span class="n">rl<span class="hl">p</span>art2</span><span class="p">),</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">tmp</span><span class="p">));</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16168</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">        </span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16203</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">        </span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Since you are changing this, you might as well make it fit the style guidelines.
Remove the trailing white space, add braces to the condition.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line16508" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 int get_refer_info(struct sip_pvt *transferer, struct sip_request *outgoing_req)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16447</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="kt">char</span> <span class="o">*</span><span class="n">lessthan</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16448</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">h_referred_by</span> <span class="o">=</span> <span class="n">ast_strdupa</span><span class="p">(</span><span class="n">p_referred_by</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16481</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">h_referred_by</span> <span class="o">=</span> <span class="n">ast_strdupa</span><span class="p">(</span><span class="n">p_referred_by</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16449</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16482</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16450</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="cm">/* Store referrer's caller ID name */</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16483</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="cm">/* Store referrer's caller ID name */</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16451</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast<span class="hl">_copy</span>_string</span><span class="p">(</span><span class="n">refer<span class="hl">data</span></span><span class="o"><span class="hl">-></span></span><span class="n">referred_by_name</span><span class="p">,</span> <span class="n">h_referred_by</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="k"><span class="hl">sizeof</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">referdata</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">referred_by_name</span></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">16484</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_string<span class="hl">_field_set</span></span><span class="p">(</span><span class="n">refer</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n">referred_by_name</span><span class="p">,</span> <span class="n">h_referred_by</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16452</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">((</span><span class="n"><span class="hl">lessthan</span></span> <span class="o">=</span> <span class="n">strchr</span><span class="p">(</span><span class="n">refer<span class="hl">data</span></span><span class="o">-></span><span class="n">referred_by_name</span><span class="p">,</span> <span class="sc">'<'</span><span class="p">)))</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16485</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">((</span><span class="n"><span class="hl">ptr</span></span> <span class="o">=</span> <span class="n">strchr</span><span class="p">(</span><span class="n">refer</span><span class="o">-></span><span class="n">referred_by_name</span><span class="p">,</span> <span class="sc">'<'</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">refer</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">referred_by_name</span></span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16453</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="o">*</span><span class="p">(</span><span class="n"><span class="hl">lessthan</span></span> <span class="o">-</span> <span class="mi">1</span><span class="p">)</span> <span class="o">=</span> <span class="sc">'\0'</span><span class="p">;</span>        <span class="cm">/* Space */</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16486</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="o">*</span><span class="p">(</span><span class="n"><span class="hl">ptr</span></span> <span class="o">-</span> <span class="mi">1</span><span class="p">)</span> <span class="o">=</span> <span class="sc">'\0'</span><span class="p">;</span> <span class="cm">/* Space */</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't really see why this kind of minor optimization is being grouped with some of the other stuff in this patch. I'm also not to fond of variables with less than useful names like 'ptr' for a character pointer... though if it's going to be recycled like this, there probably isn't a real problem with that. Maybe something like delimiter_ptr or something would be more appropriate than ptr.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line16531" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 int get_refer_info(struct sip_pvt *transferer, struct sip_request *outgoing_req)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16470</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="kt">char</span> <span class="o">*</span><span class="n">to</span> <span class="o">=</span> <span class="nb">NULL</span><span class="p">,</span> <span class="o">*</span><span class="n">from</span> <span class="o">=</span> <span class="nb">NULL</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16503</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="kt">char</span> <span class="o">*</span><span class="n">to</span> <span class="o">=</span> <span class="nb">NULL</span><span class="p">,</span> <span class="o">*</span><span class="n">from</span> <span class="o">=</span> <span class="nb">NULL</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">callid</span></span><span class="p">;</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16471</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">                </span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16504</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">                </span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16472</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="cm">/* This is an attended transfer */</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16505</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="cm">/* This is an attended transfer */</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16473</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">refer<span class="hl">data</span></span><span class="o">-></span><span class="n">attendedtransfer</span> <span class="o">=</span> <span class="mi">1</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16506</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">refer</span><span class="o">-></span><span class="n">attendedtransfer</span> <span class="o">=</span> <span class="mi">1</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16474</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_copy_string</span><span class="p">(</span><span class="n">referdata</span><span class="o">-></span><span class="n">replaces_callid</span><span class="p">,</span> <span class="n">ptr</span><span class="o">+</span><span class="mi">9</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">referdata</span><span class="o">-></span><span class="n">replaces_callid</span><span class="p">));</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16507</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16475</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_uri_decode</span><span class="p">(</span><span class="n">referdata</span><span class="o">-></span><span class="n">replaces_callid</span><span class="p">,</span> <span class="n">ast_uri_sip_user</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16508</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">callid</span> <span class="o">=</span> <span class="n">ast_strdupa</span><span class="p">(</span><span class="n">ptr</span> <span class="o">+</span> <span class="mi">9</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16476</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">((</span><span class="n">ptr</span> <span class="o">=</span> <span class="n">strchr</span><span class="p">(</span><span class="n">referdata</span><span class="o">-></span><span class="n">replaces_callid</span><span class="p">,</span> <span class="sc">';'</span><span class="p">)))</span><span class="ew">         </span><span class="cm">/* Find options */</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16509</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_uri_decode</span><span class="p">(</span><span class="n">callid</span><span class="p">,</span> <span class="n">ast_uri_sip_user</span><span class="p">);</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">16510</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">ptr</span> <span class="o">=</span> <span class="n">strchr</span><span class="p">(</span><span class="n">callid</span><span class="p">,</span> <span class="sc">';'</span><span class="p">)))</span> <span class="p">{</span> <span class="cm">/* Find options */</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16477</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="o">*</span><span class="n">ptr</span><span class="o">++</span> <span class="o">=</span> <span class="sc">'\0'</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16511</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="o">*</span><span class="n">ptr</span><span class="o">++</span> <span class="o">=</span> <span class="sc">'\0'</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16478</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16512</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="p">}</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">16513</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_string_field_set</span><span class="p">(</span><span class="n">refer</span><span class="p">,</span> <span class="n">replaces_callid</span><span class="p">,</span> <span class="n">callid</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16479</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">                </span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16514</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew">                </span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You have changes that are pretty close to these blobgs... might as well clean up the white space.</pre>
</div>
<br />
<div>
<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/1993/diff/1/?file=28966#file28966line26180" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.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 int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, struct ast_sockaddr *addr, uint32_t seqno, const char *e)</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">26053</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="cm">/* Initialize tag for new subscriptions */</span><span class="ew">        </span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Remove the extra white space.</pre>
</div>
<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;">I've just taken a brief minute look at this so far, but I'm just going to point out that there are a lot of changes in this patch. Would you be opposed to breaking this up into a series of smaller patches? There are a number of benefits to this, among them being that if you introduce a problem with these changes that causes a test failure or something we can reproduce later on, we'll be able to pinpoint which change specifically causes this more easily. Another benefit would be that smaller reviews tend to be approved much more quickly.</pre>
<p>- jrose</p>
<br />
<p>On June 19th, 2012, 10:08 p.m., gareth 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.</div>
<div>By gareth.</div>
<p style="color: grey;"><i>Updated June 19, 2012, 10:08 p.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;">This is the first part split out from the much larger patch at https://reviewboard.asterisk.org/r/1976/ - it contains just the various chan_sip code cleanups.
- struct sip_refer converted to use the stringfields API.
- sip_{refer|notify}_allocate -> sip_{notify|refer}_alloc to match other *alloc functions.
- Replace get_msg_text, get_msg_text2 and get_pidf_body -> No, not get_pidf_msg_text_body3 but get_content, to match add_content.
- get_body doesn't get the request body, renamed to get_content_line.
- get_body_by_line doesn't get the body line, and is just a simple if test. Moved code inline and removed function.
- Remove camelCase in struct sip_peer peer state variables, onHold -> onhold, inUse -> inuse, inRinging -> ringing.
- Remove camelCase in struct sip_request rlPart1 -> rlpart1, rlPart2 -> rlpart2.
- Rename instances of pvt->randdata to pvt->nonce because that is what it is, no need to update struct sip_pvt because _it already has a nonce field_.
- Removed struct sip_pvt randdata stringfield.
- Remove useless (and inconsistent) 'header' suffix on variables in handle_request_subscribe.
- Use ast_strdupa on Event header in handle_request_subscribe to avoid overly complicated strncmp calls to find the event package.
- Move get_destination check in handle_request_subscribe to avoid duplicate checking for packages that don't need it.
- Move extension state callback management in handle_request_subscribe to avoid duplicate checking for packages that don't need it.
- Remove duplicate append_date prototype.
- Rename append_date -> add_date to match other add_xxx functions.
- Added add_expires helper function, removed code that manually added expires header.
- Remove _header suffix on add_diversion_header (no other header adding functions have this).
- Don't pass req->debug to request handle_request_XXXXX handlers if req is also being passed.
- Don't pass req->ignore to check_auth as req is already being passed.
- Don't create a subscription in handle_request_subscribe if p->expiry == 0.
- Don't walk of the back of referred_by_name when splitting string in get_refer_info
- Remove duplicate check for no dialog in handle_incoming when sipmethod == SIP_REFER, handle_request_refer checks for that.
</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;">It compiles.</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_sip.c <span style="color: grey">(369065)</span></li>
<li>/trunk/channels/sip/include/sip.h <span style="color: grey">(369065)</span></li>
<li>/trunk/channels/sip/security_events.c <span style="color: grey">(369065)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1993/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>