linux-kernel - Re: [PATCH 00/15] tools: Unify perf and trace-cmd trace event format parsing v3

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]

Date:	Mon, 07 May 2012 09:37:01 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Borislav Petkov <bp@...en8.de>, Jiri Olsa <jolsa@...hat.com>,
	Arun Sharma <asharma@...com>,
	Namhyung Kim <namhyung.kim@....com>,
	Michael Rubin <mrubin@...gle.com>,
	David Sharp <dhsharp@...gle.com>,
	Vaibhav Nagarnaik <vnagarnaik@...gle.com>,
	Julia Lawall <julia@...u.dk>, Tom Zanussi <tzanussi@...il.com>,
	David Ahern <dsahern@...il.com>
Subject: Re: [PATCH 00/15] tools: Unify perf and trace-cmd trace event
 format parsing v3

On Mon, 2012-05-07 at 10:14 +0200, Ingo Molnar wrote:

> So can we please make a libevent.so, built sanely within 
> tools/perf/lib/ or such and distributed together with perf so 
> that the two can never get out of sync?

If you do this, you need to find a way to turn off -Wswitch-enum as that
seems to (at least on my gcc) ignore defaults.

Thus we end up with:

diff --git a/Makefile b/Makefile
index 18ad079..217548b 100644

diff --git a/parse-filter.c b/parse-filter.c
index d09fbd2..0ac249c 100644
--- a/parse-filter.c
+++ b/parse-filter.c
@@ -367,6 +367,12 @@ create_arg_item(struct event_format *event, const
char *token,
 		arg->type = FILTER_ARG_FIELD;
 		arg->field.field = field;
 		break;
+	case EVENT_ERROR:
+	case EVENT_NONE:
+	case EVENT_SPACE:
+	case EVENT_NEWLINE:
+	case EVENT_OP:
+	case EVENT_DELIM:
 	default:
 		free_arg(arg);
 		show_error(error_str, "expected a value but found %s",

I found that I added over 20 of these trying to keep -Wswitch-enum
happy, before i decided to give up (there's many more than 20 places
that need updates).

Looking at what was done in the current code, there's lots of :

	case PRINT_NULL:
	case PRINT_FIELD ... PRINT_SYMBOL:
	case PRINT_STRING:
	default:

Which looks more of a maintenance nightmare. How does this help? The
FOO ... BAR, will hide the same errors that you are trying to prevent.
If you were suppose to handle something between FOO and BAR, then you
just ignored it too.

It also makes that "default" redundant.

This is one of those warnings that causes more pain than it helps.

If you strongly believe that all these warnings are helpful, than we
should push to add these warnings to the kernel too.

-- Steve

--
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/