> On March 28, 2015, 3:59 p.m., Matt Jordan wrote:
> > /branches/13/channels/chan_iax2.c, lines 7674-7676
> > <https://reviewboard.asterisk.org/r/4547/diff/1/?file=73110#file73110line7674>
> >
> > 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?
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
- Diederik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4547/#review14941
-----------------------------------------------------------
On March 27, 2015, 7:34 p.m., Diederik de Groot wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4547/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 7:34 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-24917
> https://issues.asterisk.org/jira/browse/ASTERISK-24917
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> 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
>
>
> Diffs
> -----
>
> /branches/13/res/res_pjsip_dtmf_info.c 433444
> /branches/13/channels/sig_ss7.c 433444
> /branches/13/channels/sig_pri.c 433444
> /branches/13/channels/pjsip/dialplan_functions.c 433444
> /branches/13/channels/console_gui.c 433444
> /branches/13/channels/chan_unistim.c 433444
> /branches/13/channels/chan_skinny.c 433444
> /branches/13/channels/chan_sip.c 433444
> /branches/13/channels/chan_oss.c 433444
> /branches/13/channels/chan_mgcp.c 433444
> /branches/13/channels/chan_iax2.c 433444
> /branches/13/channels/chan_dahdi.c 433444
> /branches/13/channels/chan_console.c 433444
> /branches/13/channels/chan_alsa.c 433444
> /branches/13/apps/app_dictate.c 433444
>
> Diff: https://reviewboard.asterisk.org/r/4547/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Diederik de Groot
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
