> 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? > > Diederik de Groot wrote: > 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
Any update on this ? - 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
