On Thu, 2010-04-29 at 10:55 -0400, Mathieu Desnoyers wrote:
quoted text
> * Steven Rostedt (rostedt@goodmis.org) wrote:

quoted text
> > 
> > The tracepoint is created in include/linux/tracepoint.h:
> > 
> > #define TRACE_EVENT(name, proto, args, struct, assign, print)	\
> > 	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
> 
> Can we add something like this to DECLARE_TRACE ? (not convinced it is
> valid though)
> 
> static inline void check_trace_##name(cb_type)
> {
> 	BUILD_BUG_ON(!__same_type(void (*probe)(TP_PROTO(proto), void *data),
> 				  cb_type));
> }
> 

We could add it, but I'm not sure it would add any more protection. If
for some strange reason the prototype got out of sync, would would
prevent the cb_type from getting out of sync with it too, and not cause
this to fail, but still have the same bug.

Honestly, I find this a bit too paranoid. Again, the callback and the
tracepoint are made with the same data. I find it hard to think that it
would break somehow. Yes, perhaps it will break if you modify ftrace.h,
but then if you are doing that, you should know better than to break
things :-)


quoted text
> > 
> > The callback is created in include/trace/ftrace.h:
> > 
> > #undef TRACE_EVENT
> > #define TRACE_EVENT(name, proto, args, tstuct, assign, print)	\
> > 	DECLARE_EVENT_CLASS(name,				\
> > 				PARAMS(proto),			\
> > 				PARAMS(args),			\
> > 				PARAMS(tstruct),		\
> > 				PARAMS(assign),			\
> > 				PARAMS(print));			\
> > 	DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
> > 
> > #undef DECLARE_EVENT_CLASS
> > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
> > 									\
> > static notrace void							\
> > ftrace_raw_event_##call(proto,						\
> > 			struct ftrace_event_call *event_call)		\
> > [...]
> > 
> 
> Either within this callback, or in a dummy static function after, we
> could add:
> 
> check_trace_##call(ftrace_raw_event_##call);
> 
> So.. you are the preprocessor expert, do you think this could fly ? ;)



Sure, the static function you did could be added, and hope that gcc is
smart enough to get rid of it (add __unused to it). But what are we
really checking here? If CPP works?

-- Steve


--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/