<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/1583/">https://reviewboard.asterisk.org/r/1583/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 12th, 2011, 12:21 a.m., <b>wdoekes</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/1583/diff/1/?file=21722#file21722line1502" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/main/channel.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 __ast_queue_frame(struct ast_channel *chan, struct ast_frame *fin, int head, struct ast_frame *after)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1502</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">write</span><span class="p">(</span><span class="n">chan</span><span class="o">-></span><span class="n">alertpipe</span><span class="p">[</span><span class="mi">1</span><span class="p">],</span> <span class="o">&</span><span class="n">blah</span><span class="p">,</span> <span class="n">new_frames</span> <span class="o">*</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">blah</span><span class="p">))</span> <span class="o">!=</span> <span class="p">(</span><span class="n">new_frames</span> <span class="o">*</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">blah</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">1501</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">blah</span><span class="p">[</span><span class="n">new_frames</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">1502</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<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">1503</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">memset</span><span class="p">(</span><span class="n">blah</span><span class="p">,</span> <span class="mi">1</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">blah</span><span class="p">));</span></pre></td>
</tr>
<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">1504</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">write</span><span class="p">(</span><span class="n">chan</span><span class="o">-></span><span class="n">alertpipe</span><span class="p">[</span><span class="mi">1</span><span class="p">],</span> <span class="o">&</span><span class="n">blah</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">blah</span><span class="p">))</span> <span class="o">!=</span> <span class="p">(</span><span class="k">sizeof</span><span class="p">(</span><span class="n">blah</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;">Consider renaming 'blah' to something. Even 'temp' would be better, but in this case perhaps 'onoff' or 'nonzero'.
int buf[variable] is not C90 according to gcc (compile with -pedantic). It will probably work with all gcc's, but other compilers may complain.
Also, I haven't seen this construct anywhere else in the source. Maybe use alloca instead?</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 had it in my mind as invalid under any circumstances and asked Kevin about it when I saw it. Apparently I'm just *really* old fashioned. It is valid C99, and GCC has supported it since C89 or 90. If we can't start using a feature that has been around in GCC for over 20 years and has been standardized for over 10, then something has gone wrong somewhere. Also, from the README in the source tree:
Asterisk requires either the GNU Compiler Collection (GCC) version
3.0 or higher, or a compiler that supports the C99 specification and some of
the gcc language extensions.
We use the same variable name here and in __ast_read(). I'll change the name in a separate patch when I change it to writing a char instead of an int.</pre>
<br />
<p>- Terry</p>
<br />
<p>On November 11th, 2011, 10:25 p.m., Terry Wilson 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 Terry Wilson.</div>
<p style="color: grey;"><i>Updated Nov. 11, 2011, 10:25 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;">int blah = 1;
...
write(chan->alertpipe[1], &blah, new_frames * sizeof(blah)) != (new_frames * sizeof(blah)))
is only valid when new_frames == 1. Otherwise we start reading into adjacent variables declared on the stack. The read end discards what is read, so the values don't matter but it's not a good idea to read past where we want event though new_frames is almost always 1 and should never be large. This patch is basically taken out of kpfleming's eventfd branch, as he mentioned that he remembered fixing it there when I talked to him about this issue.
Another thing that could be done is to change blah to a char[] here and where we read from the alertpipe. There is no reason to write four bytes when we could just write one, but I can address that in a later patch.</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. Calls still seem to work.</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/1.8/main/channel.c <span style="color: grey">(344438)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1583/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>