Re: [PATCH 00/15] tools: Unify perf and trace-cmd trace eventformat parsing v3

[Steven Rostedt]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/