I don't know what you mean here?


-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
[ message continues ]

How about changing the callback prototypes to match the call arguments (changing
the type expected in register/unregister_trace, as well as an additional "check
type" that I proposed for Ftrace) ?

Otherwise, you basically expect here that:

void fct(void *foo, void *bar, etc etc) (N parameters expected)
{

}

called by:

fct(foo, bar, etc etc, foobar) (N + 1 parameters)

will always work.

Can you show me where the C standard says it is safe to do so ?

Thanks,

[...]

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--

[ message continues ]

This can not happen!!!! As I said before, the register is done in C,
there is no macro that will help here. We create the call back with the
macro, but the registering is in kernel/trace/trace_events.c. One
register for ___ALL___ events!!!

 Thus there is no check.


No, but it seems safe in the kernel ;-)

But that said. There is another option that will conform to this, and
that is to add flags to registering tracepoints. I already wrote a patch
for this in trying to do some other work (that I threw away).


So here's the proposal.

Change struct tracepoint_func to...

struct tracepoint_func {
	void *func;
	void *data;
	unsigned int flags;
};


The flags is set when registered. If a function is registered with data,
then the flags field will be set. Then the calling of the function can
be:

	if ((it_func_ptr)->flags & TP_FL_DATA)
		((void(*)(proto, void *))(it_func)(args, __data);
	else
		((void(*)(proto))(it_func)(args);

This would comply with the C standard.

-- Steve


--

[ message continues ]

Clearly understood. I was referring to add a static inline
check_trace_##name##_callback_type(...) { } call within the callbacks you
generate. It generates no code and adds compiler type-checking (rather than


This would also add a branch on the tracing fast path, which I would like to
avoid. Why can't we simply change all prototypes to take an extra void *__data
parameter instead ?

Thanks,


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--

[ message continues ]

I'm fine with making the data parameter mandatory with all tracers. Thus
the call back must require it. I would then move the data parameter from
the end to the beginning.

So a tracepoint with proto, will have a callback:

	void callback(void *data, proto);

I'm fine with forcing all callbacks to include a data parameter if you
are. This would also make the changes simpler.

-- Steve


--

[ message continues ]

Yes, I am all for it.

As for the extra type checking, it is basically just trying to force you to
generate matching caller-callee prototypes in your CPP macros. The goal is
really to check that the data parameter type match in both the caller and
callee. I see that as a mean to make sure nobody is going to try to take
shortcuts by playing with the callback types in the "undefined behavior" zone of
the C standard in future TRACE_EVENT() modifications.

Thanks,


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--

[ message continues ]

OK, I'll wait for your patch. As my favorite saying goes...

"When people ask me what language my mother tongue is, I simply reply 'C'."

-- Steve


--

[ message continues ]