typesafe callbacks
cover typesafe callbacks
It's possible to portably make callbacks typesafe, as well as taking the existing void * arg. This makes the callers more readable and more typesafe.
Done in a way that doesn't break non-gcc: they just don't get a warning.
Been in linux-next for about two full cycles now, and I really want to start using this in my own code.
Thanks,
Rusty.
PATCH 1/4 typesafe: cast_if_type: allow macros functions which take more than one type.
To create functions which can take two types, but still warn on any other types, we need a way of casting one type and no others.
To make things more complex, it should correctly handle function args, NULL, and be usable in initializers. __builtin_choose_expr was introduced in gcc 3.1 (kernel needs >= 3.2 anyway).
(includes Harvey Harrison <[email protected]>'s sparse fix)
--- include/linux/compiler-gcc.h | 18 ++++++++++++++++++ include/linux/compiler-intel.h | 2 ++ 2 files changed, 20 insertions(+)
include/linux/compiler-gcc.h +18 -0
@@ -61,3 +61,21 @@#define noinline __attribute__((noinline))#define __attribute_const__ __attribute__((__const__))#define __maybe_unused __attribute__((unused)) + +/** +* cast_if_type - allow an alternate type +* @expr: the expression to optionally cast +* @oktype: the type to allow. +* @desttype: the type to cast to. +* +* This is used to accept a particular alternate type for an expression: +* because any other types will not be cast, they will cause a warning as +* normal. +* +* Note that the unnecessary trinary forces functions to devolve into +* function pointers as users expect, but means @expr must be a pointer or +* integer. +*/ +#define cast_if_type (expr, oktype, desttype) __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(1?(expr):(expr)), oktype), \ + (desttype)(expr), (expr))
include/linux/compiler-intel.h +2 -0
@@ -29,3 +29,5 @@#endif #define uninitialized_var (x) x + +#define cast_if_type (expr, oktype, desttype) ((desttype)(expr))
PATCH 2/4 typesafe: typesafe_cb: wrappers for typesafe callbacks.
cast_if_type is clever but annoying to use, so we make some slightly less annoying wrappers for callback registration.
These allow const pointer-taking functions as well as the normal non-const ones (volatile variant omitted for simplicity). We also have a typesafe_cb_preargs() for callbacks which don't just take one argument.
Here's an example of use:
BEFORE:
void foo_set_callback(int (*callback)(void *), void *arg);
AFTER:
#define foo_set_callback(cb, arg) \ __foo_set_callback(typesafe_cb(int, (cb), (arg)), (arg)) void __foo_set_callback(int (*callback)(void *), void *arg);
o If cb is of form "int cb(typeof(arg))" or "int cb(const typeof(arg))" it will be cast to form "int cb(void *)".
o Otherwise, if cb is not already of form "int cb(void *)" it will cause a warning when passed to __foo_set_callback().
o If arg is not a pointer, then handing it to __foo_set_callback() will cause a warning.
Note: there are at least two possible additions we don't yet need. We don't cover callbacks which take a volatile pointer, even though that would be fine, and we don't have a typesafe_cb_postargs().
--- include/linux/kernel.h | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
include/linux/kernel.h +35 -0
@@ -436,4 +436,39 @@ struct sysinfo {#define NUMA_BUILD 0#endif +/* If fn is of type ok1 or ok2, cast to desttype */ +#define __typesafe_cb (desttype, fn, ok1, ok2) \ + cast_if_type(cast_if_type((fn), ok1, desttype), ok2, desttype) + +/** +* typesafe_cb - cast a callback function if it matches the arg +* @rettype: the return type of the callback function +* @fn: the callback function to cast +* @arg: the (pointer) argument to hand to the callback function. +* +* If a callback function takes a single argument, this macro does +* appropriate casts to a function which takes a single void * argument if the +* callback provided matches the @arg (or a const or volatile version). +* +* It is assumed that @arg is of pointer type: usually @arg is passed +* or assigned to a void * elsewhere anyway. +*/ +#define typesafe_cb (rettype, fn, arg) \ + __typesafe_cb(rettype (*)(void *), (fn), \ + rettype (*)(const typeof(arg)), \ + rettype (*)(typeof(arg))) + +/** +* typesafe_cb_preargs - cast a callback function if it matches the arg +* @rettype: the return type of the callback function +* @fn: the callback function to cast +* @arg: the (pointer) argument to hand to the callback function. +* +* This is a version of typesafe_cb() for callbacks that take other arguments +* before the @arg. +*/ +#define typesafe_cb_preargs (rettype, fn, arg, ...) \ + __typesafe_cb(rettype (*)(__VA_ARGS__, void *), (fn), \ + rettype (*)(__VA_ARGS__, const typeof(arg)), \ + rettype (*)(__VA_ARGS__, typeof(arg)))#endif
PATCH 3/4 typesafe: kthread_create and kthread_run
(Assumes patch to add printf attr to kthread_create is applied, but fixup trivial)
Straight-forward conversion to allow typesafe callbacks.
--- include/linux/kthread.h | 28 +++++++++++++++++++++++++--- kernel/kthread.c | 29 +++++------------------------ 2 files changed, 30 insertions(+), 27 deletions(-)
include/linux/kthread.h +25 -3
@@ -4,9 +4,31 @@#include <linux/err.h> #include <linux/sched.h> -struct task_struct *kthread_create (int (*threadfn)(void *data), -void *data, -const char namefmt[], ...) +/** +* kthread_create - create a kthread. +* @threadfn: the function to run until signal_pending(current). +* @data: data ptr for @threadfn. +* @namefmt: printf-style name for the thread. +* +* Description: This helper function creates and names a kernel +* thread. The thread will be stopped: use wake_up_process() to start +* it. See also kthread_run(), kthread_create_on_cpu(). +* +* When woken, the thread will run @threadfn() with @data as its +* argument. @threadfn() can either call do_exit() directly if it is a +* standalone thread for which noone will call kthread_stop(), or +* return when 'kthread_should_stop()' is true (which means +* kthread_stop() has been called). The return value should be zero +* or a negative error number; it will be passed to kthread_stop(). +* +* Returns a task_struct or ERR_PTR(-ENOMEM). +*/ +#define kthread_create(threadfn, data, namefmt...) \ + __kthread_create(typesafe_cb(int,(threadfn),(data)), (data), namefmt) + +struct task_struct *__kthread_create (int (*threadfn)(void *data), +void *data, +const char namefmt[], ...) __attribute__((format (printf,3 ,4 )));/**
kernel/kthread.c +5 -24
@@ -112,29 +112,10 @@ static void create_kthread(struct kthreacomplete (&create->done); } -/** -* kthread_create - create a kthread. -* @threadfn: the function to run until signal_pending(current). -* @data: data ptr for @threadfn. -* @namefmt: printf-style name for the thread. -* -* Description: This helper function creates and names a kernel -* thread. The thread will be stopped: use wake_up_process() to start -* it. See also kthread_run(), kthread_create_on_cpu(). -* -* When woken, the thread will run @threadfn() with @data as its -* argument. @threadfn() can either call do_exit() directly if it is a -* standalone thread for which noone will call kthread_stop(), or -* return when 'kthread_should_stop()' is true (which means -* kthread_stop() has been called). The return value should be zero -* or a negative error number; it will be passed to kthread_stop(). -* -* Returns a task_struct or ERR_PTR(-ENOMEM). -*/ -struct task_struct *kthread_create (int (*threadfn)(void *data), -void *data, -const char namefmt[], - ...) +struct task_struct *__kthread_create (int (*threadfn)(void *data), +void *data, +const char namefmt[], + ...) {struct kthread_create_info create;
@@ -159,7 +140,7 @@ struct task_struct *kthread_create(int ( }return create.result; } -EXPORT_SYMBOL (kthread_create); +EXPORT_SYMBOL (__kthread_create);/** * kthread_bind - bind a just-created kthread to a cpu.
PATCH 4/4 typesafe: TIMER_INITIALIZER and setup_timer
This patch lets timer callback functions have their natural type (ie. exactly match the data pointer type); it allows the old "unsigned long data" type as well.
Downside: if you use the old "unsigned long" callback type, you won't get a warning if your data is not an unsigned long, due to the cast.
--- include/linux/timer.h | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
include/linux/timer.h +23 -9
@@ -25,12 +25,22 @@ struct timer_list {extern struct tvec_base boot_tvec_bases; -#define TIMER_INITIALIZER (_function, _expires, _data) { \ - .entry = { .prev = TIMER_ENTRY_STATIC }, \ - .function = (_function), \ - .expires = (_expires), \ - .data = (_data), \ - .base = &boot_tvec_bases, \ +/* +* For historic reasons the timer function takes an unsigned long, so +* we use this variant of typesafe_cb. data is converted to an unsigned long +* if it is another integer type, by adding 0UL. +*/ +#define typesafe_timerfn (fn, data) \ + __typesafe_cb(void (*)(unsigned long), (fn), \ + void (*)(const typeof((data)+0UL)), \ + void (*)(typeof((data)+0UL))) + +#define TIMER_INITIALIZER (_function, _expires, _data) { \ + .entry = { .prev = TIMER_ENTRY_STATIC }, \ + .function = typesafe_timerfn((_function), (_data)), \ + .expires = (_expires), \ + .data = (unsigned long)(_data), \ + .base = &boot_tvec_bases, \ }#define DEFINE_TIMER (_name, _function, _expires, _data) \
@@ -51,9 +61,13 @@ static inline void init_timer_on_stack(s }#endif -static inline void setup_timer (struct timer_list * timer, -void (*function)(unsigned long ), -unsigned long data) +#define setup_timer (timer, function, data) \ + __setup_timer((timer), typesafe_timerfn((function), (data)), \ + (unsigned long)(data)) + +static inline void __setup_timer (struct timer_list *timer, +void (*function)(unsigned long ), +unsigned long data) { timer->function = function; timer->data = data;