From: Steven Rostedt <rostedt@goodmis.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
LKML <linux-kernel@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>,
Borislav Petkov <bp@alien8.de>, Jiri Olsa <jolsa@redhat.com>,
Arun Sharma <asharma@fb.com>, Namhyung Kim <namhyung.kim@lge.com>,
Michael Rubin <mrubin@google.com>,
David Sharp <dhsharp@google.com>,
Vaibhav Nagarnaik <vnagarnaik@google.com>,
Julia Lawall <julia@diku.dk>, Tom Zanussi <tzanussi@gmail.com>,
David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH 00/15] tools: Unify perf and trace-cmd trace event format parsing v3
Date: Mon, 07 May 2012 09:37:01 -0400 [thread overview]
Message-ID: <1336397821.14207.136.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <20120507081424.GE16608@gmail.com>
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
next prev parent reply other threads:[~2012-05-07 13:37 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-25 12:26 [PATCH 00/15] tools: Unify perf and trace-cmd trace event format parsing v3 Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 01/15] perf: Separate out trace-cmd parse-events from perf files Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 02/15] tools/events: Add files to create libtraceevent.a Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 03/15] perf: Build libtraceevent.a Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 04/15] events: Update tools/lib/traceevent to work with perf Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 05/15] perf: Have perf use the new libtraceevent.a library Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 06/15] perf/events: Add flag to produce nsec output Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 07/15] perf/events: Add flag/symbol format_flags Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 08/15] perf/events: Correct size given to memset Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 09/15] parse-events: Handle invalid opcode parsing gracefully Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 10/15] parse-events: Handle opcode parsing error Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 11/15] parse-events: Let pevent_free() take a NULL pointer Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 12/15] parse-events: Support '+' opcode in print format Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 13/15] parse-events: Allow '*' and '/' operations in TP_printk Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 14/15] parse-event: Fix memset pointer size bug in handle Frederic Weisbecker
2012-04-25 12:26 ` [PATCH 15/15] parse-events: Rename struct record to struct pevent_record Frederic Weisbecker
2012-05-07 8:14 ` [PATCH 00/15] tools: Unify perf and trace-cmd trace event format parsing v3 Ingo Molnar
2012-05-07 8:36 ` Namhyung Kim
2012-05-07 12:57 ` Steven Rostedt
2012-05-07 13:01 ` Steven Rostedt
2012-05-07 13:37 ` Steven Rostedt [this message]
2012-05-07 14:13 ` Ingo Molnar
2012-05-21 6:40 ` Namhyung Kim
2012-05-21 8:44 ` Ingo Molnar
2012-05-21 11:36 ` Frederic Weisbecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1336397821.14207.136.camel@gandalf.stny.rr.com \
--to=rostedt@goodmis.org \
--cc=acme@infradead.org \
--cc=asharma@fb.com \
--cc=bp@alien8.de \
--cc=dhsharp@google.com \
--cc=dsahern@gmail.com \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=julia@diku.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mrubin@google.com \
--cc=namhyung.kim@lge.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tzanussi@gmail.com \
--cc=vnagarnaik@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).