<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/2112/">https://reviewboard.asterisk.org/r/2112/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 14th, 2012, 12:33 a.m., <b>Alec Davis</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/2112/diff/2/?file=31225#file31225line8590" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/channel.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">char *ast_print_group(char *buf, int buflen, ast_group_t group)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">8574</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">it</span> <span class="o">=</span> <span class="n">ao2_iterator_init</span><span class="p">(</span><span class="n">gr<span class="hl">oup</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">container</span></span><span class="p">,</span> <span class="mi">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">8563</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">it</span> <span class="o">=</span> <span class="n">ao2_iterator_init</span><span class="p">(</span><span class="n">gr<span class="hl">p</span></span><span class="p">,</span> <span class="mi">0</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;">why not
for ((it = ao2_iterator_init(grp, 0)); (ng = ao2_iterator_next(&it)); ao2_ref(ng, -1)) {
In other words, the orignal was easier to read, and achieved the same.
The a02_iterator_next is evaluated before the loop is executed.
</pre>
</blockquote>
<p>On September 14th, 2012, 10:42 a.m., <b>rmudgett</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;">Using the for loop gets you thinking about what is going on.
For all items in the container do...
I initially put the iterator init there but put it before the for loop to shorten the line length.
I moved it back to the for init section since it fits the line length guideline.
</pre>
</blockquote>
<p>On September 14th, 2012, 4:40 p.m., <b>Alec Davis</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 the line didn't fit, it 'could' be done like below, note: never seen it done this way before, but makes me wonder whether the brackets around the "ng =..." are required.
for (
it = ao2_iterator_init(grp, 0);
(ng = ao2_iterator_next(&it));
ao2_ref(ng, -1)) {</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;">There are several examples in pbx.c.
The parentheses around the "ng =..." are necessary to avoid a compiler warning about assignments in test expressions.
I have broken long for loop lines that way before. I haven't been thrilled with the appearance but it is the best places to break the line.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On September 14th, 2012, 11:13 a.m., rmudgett 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 rmudgett.</div>
<p style="color: grey;"><i>Updated Sept. 14, 2012, 11:13 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;">* ASTERISK-20383
Missing named call pickup group features:
CHANNEL(callgroup) - Need CHANNEL(namedcallgroup)
CHANNEL(pickupgroup) - Need CHANNEL(namedpickupgroup)
Pickup() - Needs to also select from named pickup groups.
* ASTERISK-20384
Using the pickupexten, the pickup channel selection could fail even though there was a call it could have picked up. In a call pickup race when there are multiple calls to pickup and two extensions try to pickup a call, it is conceivable that the loser will not pick up any call even though it could have picked up the next oldest matching call.
Regression because of the named call pickup group feature.
* See ASTERISK-20386 for the improvements.
* Fixed some locking issues in CHANNEL().</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;">Tested call pickupexten, Pickup(), CHANNEL(namedcallgroup), and CHANNEL(namedpickupgroup) for named call group functionality.</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-20383">ASTERISK-20383</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20384">ASTERISK-20384</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20386">ASTERISK-20386</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>/trunk/apps/app_directed_pickup.c <span style="color: grey">(373056)</span></li>
<li>/trunk/funcs/func_channel.c <span style="color: grey">(373056)</span></li>
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(373056)</span></li>
<li>/trunk/include/asterisk/features.h <span style="color: grey">(373056)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(373056)</span></li>
<li>/trunk/main/features.c <span style="color: grey">(373056)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2112/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>