* Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote: > On Sun, Aug 16, 2009 at 08:57:54AM +0000, tip-bot for Ingo Molnar wrote: > > Commit-ID: 83a0944fa919fb2ebcfc1f8933d86e437b597ca6 > > Gitweb: http://git.kernel.org/tip/83a0944fa919fb2ebcfc1f8933d86e437b597ca6 > > Author: Ingo Molnar <mingo@xxxxxxx> > > AuthorDate: Sat, 15 Aug 2009 12:26:57 +0200 > > Committer: Ingo Molnar <mingo@xxxxxxx> > > CommitDate: Sun, 16 Aug 2009 10:47:47 +0200 > > > > perf: Enable more compiler warnings > > > > Related to a shadowed variable bug fix Valdis Kletnieks noticed > > that perf does not get built with -Wshadow, which could have > > helped us avoid the bug. > > > > So enable -Wshadow and also enable the following warnings on > > perf builds, in addition to the already enabled -Wall -Wextra > > -std=gnu99 warnings: > > > > -Wcast-align > > -Wformat=2 > > -Wshadow > > -Winit-self > > -Wpacked > > -Wredundant-decls > > -Wstack-protector > > -Wstrict-aliasing=3 > > -Wswitch-default > > -Wswitch-enum > > > > This one looks like more a pain than something useful. > > I have this code in perf trace: > > static int event_item_type(enum event_type type) > { > switch (type) { > case EVENT_ITEM: > case EVENT_DQUOTE: > case EVENT_SQUOTE: > return 1; > default: > return 0; > } > } > > Which results in: > > util/trace-event-parse.c: In function ‘event_item_type’: > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_ERROR’ not handled in switch > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_NONE’ not handled in switch > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_SPACE’ not handled in switch > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_NEWLINE’ not handled in switch > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_OP’ not handled in switch > util/trace-event-parse.c:377: erreur: enumeration value ‘EVENT_DELIM’ not handled in switch > > The default case already handles these and I guess we don't want workarounds like: > > static int event_item_type(enum event_type type) > { > switch (type) { > case EVENT_ITEM: > case EVENT_DQUOTE: > case EVENT_SQUOTE: > return 1; > case EVENT_ERROR: > case EVENT_NONE: > case EVENT_SPACE: > case EVENT_NEWLINE: > case EVENT_OP: > case EVENT_DELIM: > default: > return 0; > } > } > > > Right? :-) > > This warning might be useful for *very* specific cases, but not here IMO. i think it's useful when an enum gets extended. Say we add a new event enum and want to make sure it's handled in _all_ switch statements that deals with them. This warning ensures that we propagate the new case to all usage sites. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |