<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/4547/">https://reviewboard.asterisk.org/r/4547/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2015, 3:59 p.m. CET, <b>Matt Jordan</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/4547/diff/1/?file=73110#file73110line7674" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/channels/chan_iax2.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 __send_command(struct chan_iax2_pvt *i, char type, int command, unsigned int ts, const unsigned char *data, int datalen, int seqno,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7674</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">{</pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7674</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">{</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7675</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>struct ast_frame f = { 0, };</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7675</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>struct ast_frame f = { <span class="hl">.frametype = </span>0, };</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7676</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>int res = 0;</pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7676</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>int res = 0;</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;">I really dislike this warning.
In C, it has always been perfectly valid to initialize all members of a struct using { 0 } as a universal zero-initializer. This nomenclature actually makes it less readable, as it makes it look like we only wanted to initialize the .frametype member, and ignored the rest. That may not be the actual effect, but the syntax here is not clearer by any stretch.
:-(
Interestingly, when compiling with clang 3.4, I don't get this warning issued. What version are you compiling with? And what do you think about simply ignoring this warning?</pre>
</blockquote>
<p>On March 28th, 2015, 4:21 p.m. CET, <b>Diederik de Groot</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 "struct ast_frame f = { 0, };" is perfectly fine and does not raise an error
However: "struct ast_frame f = { AST_FRAME_CONTROL, { AST_CONTROL_CONGESTION } };" however does raise this warning, which does make a little more sense.
I updated all of these (i hope i got them all) so that they all use the same syntax. In case someone ones to update/change/extend one of them.
FYI: There is one major benefit in using the named variety, namely allowing you to change the order of the fields in the struct without consequence. For example the ast_frame struct could be optimized a little by reordering the fields to improve packing, as in:
struct ast_frame {
/*! Kind of frame */
enum ast_frame_type frametype;
/*! Length of data */
int datalen;
/*! Subclass, frame dependent */
struct ast_frame_subclass subclass;
/*! Number of samples in this frame */
int samples;
/*! Was the data malloc'd? i.e. should we free it when we discard the frame? */
int mallocd;
/*! The number of bytes allocated for a malloc'd frame header */
size_t mallocd_hdr_len;
/*! How many bytes exist _before_ "data" that can be used if needed */
int offset;
/*! Misc. frame flags */
unsigned int flags;
/*! Optional source of frame for debugging */
const char *src;
/*! Pointer to actual data */
union { void *ptr; uint32_t uint32; char pad[8]; } data;
/*! Global delivery time */
struct timeval delivery;
/*! For placing in a linked list */
AST_LIST_ENTRY(ast_frame) frame_list;
/*! Timestamp in milliseconds */
long ts;
/*! Length in milliseconds */
long len;
/*! Sequence number */
int seqno;
};
would get rid of some of the padding (on x86_64).
Note: this warning might have occured after playing with the struct layout of ast_frame to be honest (Need to recheck this). If so this change should be considered an enhancement.
Note2: I compile using clang-3.6</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;">Any update on this ?</pre>
<br />
<p>- Diederik</p>
<br />
<p>On March 27th, 2015, 7:34 p.m. CET, Diederik de Groot 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 Diederik de Groot.</div>
<p style="color: grey;"><i>Updated March 27, 2015, 7:34 p.m.</i></p>
<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-24917">ASTERISK-24917</a>
</div>
<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;">clang's static analyzer will throw quite a number warnings / errors during compilation, some of which can be very helpfull in finding corner-case bugs. clang compiler warning:braces-around-scalar-initializer</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/13/res/res_pjsip_dtmf_info.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/sig_ss7.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/sig_pri.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/pjsip/dialplan_functions.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/console_gui.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_unistim.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_skinny.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_sip.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_oss.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_mgcp.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_iax2.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_dahdi.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_console.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/channels/chan_alsa.c <span style="color: grey">(433444)</span></li>
<li>/branches/13/apps/app_dictate.c <span style="color: grey">(433444)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4547/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>