* Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> On Sun, Aug 16, 2009 at 04:01:54PM +0200, Ingo Molnar wrote:
> >
> > * 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
>
>
> Ok, it's a bit annoying to cover all enum cases but well, I agree
> with the above possibility...
Note, there's a GCC extension for a range of enums:
case OPTION_BIT ... OPTION_SET_PTR:
so in case the enums are in a continuous range, it's just a single
line to list them.
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
|