<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/1147/">https://reviewboard.asterisk.org/r/1147/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 6th, 2011, 4:58 p.m., <b>Russell Bryant</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/1147/diff/3/?file=15999#file15999line1176" style="color: black; font-weight: bold; text-decoration: underline;">/apps/app_confbridge.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int conf_get_pin(struct ast_channel *chan, struct conference_bridge_user *conference_bridge_user)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">663</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ao2_unlock</span><span class="p">(</span><span class="n">conference_bridge</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">1069</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">char</span> <span class="n">pin_guess</span><span class="p">[</span><span class="n">MAX_PIN</span><span class="p">]</span> <span class="o">=</span> <span class="p">{</span> <span class="mi">0</span><span class="p">,</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;">= { '\0', }; would be a bit more explicit, but I would just do = "";
Also, what is MAX_PIN supposed to be? I would read that as the maximum length of a pin, when in reality, the max length of a pin is MAX_PIN - 1 (because of the nul terminator).</pre>
</blockquote>
<p>On April 20th, 2011, 8:28 a.m., <b>jrose</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;">Actually, MAX_PIN would still be the maximum length of the pin. The null terminator is in the MAX_PINth + 1 position since the array starts at 0. But you know that :P
I'm not sure I'm working that properly, so I'll just say... if MAX_PIN is 4 and the array length is 8...
0 - char 1
1 - char 2
2 - char 3
3 - char 4
4 - '\0'</pre>
</blockquote>
<p>On April 20th, 2011, 8:30 a.m., <b>jrose</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;">Oh wait, this is a declaration. Disregard everything I just said.</pre>
</blockquote>
<p>On April 20th, 2011, 8:36 a.m., <b>jrose</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;">Actually though, wouldn't that terminator be at the beginning of the line resulting in the possibility of the array having no terminator after it is set? It might seem like bad form for a string, but the code might always be using comparison functions that take the length into account, which would mean that this could still be the maximum pin length.</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;">Been looking at this, and here are my thoughts...
= { 0, }; (and by extension = { '\0', }) is actually unnecessary. I think. ast_app_getdata is going to take tmp (currently pointing at pin_guess) and set the first value to \0 at the beginning of that function anyway, so the assignment ends up being repeated. Not a big deal of course, it only happens at the beginning of the function.
Since \0 is continually put onto the end of the array and len = MAX_PIN - 1 and since it isn't using something specifically defined to limit comparison to MAX_PIN characters, Russell is right, pin_guess[MAX_PIN+1] would be more appropriate and len = MAX_PIN. Since MAX_PIN is hard defined at 80 right now, this is pretty much trivial, but I don't know... maybe the intention will be to let users set the max pin length in confbridge.conf later.</pre>
<br />
<p>- jrose</p>
<br />
<p>On March 28th, 2011, 3:47 p.m., David Vossel wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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 David Vossel.</div>
<p style="color: grey;"><i>Updated 2011-03-28 15:47:43</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;">The new ConfBridge application. It's kind of a big deal.</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;">All confbridge.conf features have been tested.
Load tested at sample rates ranging from 8-48khz.
AMI actions/events tested
CLI commands tested</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/configs/confbridge.conf.sample <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/include/asterisk/bridging.h <span style="color: grey">(311748)</span></li>
<li>/trunk/include/asterisk/bridging_features.h <span style="color: grey">(311748)</span></li>
<li>/trunk/include/asterisk/bridging_technology.h <span style="color: grey">(311748)</span></li>
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(311748)</span></li>
<li>/trunk/include/asterisk/dsp.h <span style="color: grey">(311748)</span></li>
<li>/trunk/main/bridging.c <span style="color: grey">(311748)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(311748)</span></li>
<li>/trunk/main/dsp.c <span style="color: grey">(311748)</span></li>
<li>/trunk/apps/confbridge/include/confbridge.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/bridges/bridge_builtin_features.c <span style="color: grey">(311748)</span></li>
<li>/trunk/bridges/bridge_softmix.c <span style="color: grey">(311748)</span></li>
<li>/trunk/apps/confbridge/conf_config_parser.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/apps/app_confbridge.c <span style="color: grey">(311748)</span></li>
<li>/trunk/CHANGES <span style="color: grey">(311748)</span></li>
<li>/trunk/UPGRADE.txt <span style="color: grey">(311748)</span></li>
<li>/trunk/apps/Makefile <span style="color: grey">(311748)</span></li>
<li>/trunk/res/res_musiconhold.c <span style="color: grey">(311748)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1147/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>