linux-kernel - Re: [tip:perfcounters/core] perf: Enable more compiler warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090816140154.GA9408@elte.hu>
Date:	Sun, 16 Aug 2009 16:01:54 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	mingo@...hat.com, hpa@...or.com, paulus@...ba.org, acme@...hat.com,
	linux-kernel@...r.kernel.org, a.p.zijlstra@...llo.nl,
	efault@....de, tglx@...utronix.de,
	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:perfcounters/core] perf: Enable more compiler warnings


* Frederic Weisbecker <fweisbec@...il.com> 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@...e.hu>
> > AuthorDate: Sat, 15 Aug 2009 12:26:57 +0200
> > Committer:  Ingo Molnar <mingo@...e.hu>
> > 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/