<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/3652/">https://reviewboard.asterisk.org/r/3652/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2014, 6:01 p.m. UTC, <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/3652/diff/1/?file=59869#file59869line683" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/main/acl.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">683</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">const</span> <span class="kt">char</span> <span class="o">*</span><span class="n">addr</span> <span class="o">=</span> <span class="n">ast_strdupa</span><span class="p">(</span><span class="n">ast_sockaddr_stringify_addr</span><span class="p">(</span><span class="o">&</span><span class="n">ha</span><span class="o">-></span><span class="n">addr</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;">It's typically a good idea to avoid ast_strdupa() inside a for loop since it theoretically could blow the stack on a very large list.
Since addr is const, you don't even need to make a stack copy of it at all.</pre>
</blockquote>
<p>On June 19th, 2014, 7:29 p.m. UTC, <b>George Joseph</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;">This was a cut and paste from the original ast_ha_join. I removed the strdup here and there don't seem to be any ill effects but I left ast_ha_join alone as it would require more testing. After all, somebody put it there for a reason. :)</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;">I can answer why ast_ha_join() does it. ast_sockaddr_stringify_* family of functions uses a thread-local buffer to store the stringified result. Each subsequent call to a function in that family will overwrite the buffer with whatever is being stringified. So in that function, the address gets stringified and then the netmask does. The stringified address has to be stored someplace else or else stringifying the netmask will overwrite it. Using ast_strdupa() in ast_ha_join() should probably be changed to use ast_copy_string() into a fixed-sized buffer since IP addresses have an easily-defined maximum size.</pre>
<br />
<p>- Mark</p>
<br />
<p>On June 19th, 2014, 7:29 p.m. UTC, George Joseph 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 George Joseph.</div>
<p style="color: grey;"><i>Updated June 19, 2014, 7:29 p.m.</i></p>
<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;">In 'pjsip show endpoints' wouldn't you rather see...
Identify: 192.168.101.54/32,192.168.147.1/32
or
Identify: 26ff:ff:6622:e020::220/128
instead of...
Identify: 192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255
or
Identify: 26ff:ff:6622:e020::220/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff
?
I would. :)
Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr.
Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask instead of ast_sockaddr_stringify_addr.
Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead of ast_ha_join().
This is a CLI change only. AMI was not affected.</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;">Made sure both ipv4 and ipv6 addresses were formatted correctly.</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/12/res/res_pjsip_endpoint_identifier_ip.c <span style="color: grey">(416729)</span></li>
<li>branches/12/main/netsock2.c <span style="color: grey">(416729)</span></li>
<li>branches/12/main/acl.c <span style="color: grey">(416729)</span></li>
<li>branches/12/include/asterisk/netsock2.h <span style="color: grey">(416729)</span></li>
<li>branches/12/include/asterisk/acl.h <span style="color: grey">(416729)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3652/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>