<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/2991/">https://reviewboard.asterisk.org/r/2991/</a>
</td>
</tr>
</table>
<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;">Just a minor nit. Michaels ship-it still stands.</pre>
<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/2991/diff/1/?file=47945#file47945line35" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/include/asterisk/netsock2.h</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; ">extern "C" {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">35</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Values for address families that we support. This is reproduced from socket.h</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">35</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Values for address families that we support. This is reproduced from socket.h</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * because we do not want users to include that file. Only netsock2.c should</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * because we do not want users to include that file. Only netsock2.c should</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * ever include socket.h.</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * ever include socket.h.</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;">That's an odd thing to say in a (header) file that includes sys/socket.h itself. I don't see anyone undefining those values anywhere. And the rest of the code is sprinkled with [^_]AF_INET.</pre>
</div>
<br />
<p>- wdoekes</p>
<br />
<p>On November 2nd, 2013, 10:25 p.m. UTC, Matt Jordan 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 Matt Jordan.</div>
<p style="color: grey;"><i>Updated Nov. 2, 2013, 10:25 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;">This started off as a fix for the failing IAX2 acl_call test in the Asterisk Test Suite.
When inspecting why that test was failing, I found that all attempts to bind to any local loopback address was failing:
[Nov 2 15:56:28] VERBOSE[15787] chan_iax2.c: == Binding IAX2 to address 127.0.0.1:4569
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: Splitting '127.0.0.1' into...
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: ...host '127.0.0.1' and port ''.
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: Splitting '127.0.0.1' into...
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: ...host '127.0.0.1' and port ''.
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: Splitting '127.0.0.1' into...
[Nov 2 15:56:28] DEBUG[15787] netsock2.c: ...host '127.0.0.1' and port ''.
[Nov 2 15:56:28] ERROR[15787] netsock2.c: getaddrinfo("127.0.0.1", "(null)", ...): ai_family not supported
[Nov 2 15:56:28] WARNING[15787] acl.c: Unable to lookup '127.0.0.1'
[Nov 2 15:56:28] WARNING[15787] chan_iax2.c: Non-local or unbound address specified (127.0.0.1) in sourceaddress for 'local-1', reverting to default
While there's conceivably other ways for getaddrino to return EAI_FAMILY, the most common way is if AF_INET, AF_INET6, or AF_UNSPEC is not provided as the desired family. However, chan_iax2 is (more or less) doing the right thing when it calls into ast_sockaddr_resolve:
...
if (!ast_sockaddr_resolve(&hostaddr, tmp->value, PARSE_PORT_FORBID, 0)
...
Other than passing 0 in as the family (which should be AST_AF_UNSPEC), this should be fine. After some further debugging, it turned out that the culprit was the call to ast_get_ip. This function - which strangely enough is defined in acl.h - actually uses the family from the passed in addr object (which it will also populate when it returns!) when it eventually calls getaddrinfo. (Yes, we're using an out param as input to our function. Ew.) Some searching through the code revealed a number of places - not just in chan_iax2 - where the ast_sockaddr structure passed to ast_get_ip does not have its family initialized or its family is not specified (there's no guarantee that 0 == AF_UNSPEC on all systems).
This patch fixes those users of ast_get_ip that were not specifying the family. It also defines AST_AF_* to their actual equivalents, instead of defining them to integer values. This should make this code less brittle in case something actually doesn't use those integer values for those constants.</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 IAX2 acl_call test now passes on Asterisk 12.</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/include/asterisk/netsock2.h <span style="color: grey">(402448)</span></li>
<li>/branches/12/include/asterisk/acl.h <span style="color: grey">(402448)</span></li>
<li>/branches/12/channels/chan_sip.c <span style="color: grey">(402448)</span></li>
<li>/branches/12/channels/chan_iax2.c <span style="color: grey">(402448)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2991/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>