* [PATCH 0/6] typesafe callbacks @ 2008-04-20 22:58 Rusty Russell 2008-04-20 23:00 ` [PATCH 1/6] cast_if_type: allow macros functions which take more than one type Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 22:58 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds The idea is to create a general typesafe callback mechanism which doesn't break compile on existing code and doesn't allow any unsafe callback types. Al had a very long timer conversion series followed by a different mechanism, and that just covered timers; unfortunately that technique provides insufficient typechecking for the general case (eg. int return types and callback functions which take integer args as well as the data arg). Note that these typechecks end up being *too* strict in some cases, disallowing some potentially valid cases. But since you can still use the current via-void* method, these corner cases lose nothing. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/6] cast_if_type: allow macros functions which take more than one type. 2008-04-20 22:58 [PATCH 0/6] typesafe callbacks Rusty Russell @ 2008-04-20 23:00 ` Rusty Russell 2008-04-20 23:01 ` [PATCH 2/6] typesafe_cb: wrappers for typesafe callbacks Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:00 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds 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). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/compiler-gcc.h | 19 +++++++++++++++++++ include/linux/compiler-intel.h | 2 ++ 2 files changed, 21 insertions(+) diff -r bde8a949d956 include/linux/compiler-gcc.h --- a/include/linux/compiler-gcc.h Mon Apr 07 15:27:30 2008 +1000 +++ b/include/linux/compiler-gcc.h Mon Apr 07 16:19:49 2008 +1000 @@ -53,3 +53,22 @@ #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):0),\ + oktype), \ + (desttype)(expr), (expr)) diff -r bde8a949d956 include/linux/compiler-intel.h --- a/include/linux/compiler-intel.h Mon Apr 07 15:27:30 2008 +1000 +++ b/include/linux/compiler-intel.h Mon Apr 07 16:19:49 2008 +1000 @@ -29,3 +29,5 @@ #endif #define uninitialized_var(x) x + +#define cast_if_type(expr, oktype, desttype) ((desttype)(expr)) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/6] typesafe_cb: wrappers for typesafe callbacks. 2008-04-20 23:00 ` [PATCH 1/6] cast_if_type: allow macros functions which take more than one type Rusty Russell @ 2008-04-20 23:01 ` Rusty Russell 2008-04-20 23:05 ` [PATCH 3/6] typesafe: Convert stop_machine Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:01 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds 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(). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/kernel.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff -r 907c55be656f include/linux/kernel.h --- a/include/linux/kernel.h Mon Apr 21 06:58:00 2008 +1000 +++ b/include/linux/kernel.h Mon Apr 21 07:04:02 2008 +1000 @@ -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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/6] typesafe: Convert stop_machine 2008-04-20 23:01 ` [PATCH 2/6] typesafe_cb: wrappers for typesafe callbacks Rusty Russell @ 2008-04-20 23:05 ` Rusty Russell 2008-04-20 23:07 ` [PATCH 4/6] typesafe: kthread_create and kthread_run Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:05 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds Straightforward conversion of stop_machine_run. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/stop_machine.h | 12 +++-- kernel/stop_machine.c | 89 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 6 deletions(-) diff -r 103cfcb193b5 include/linux/stop_machine.h --- a/include/linux/stop_machine.h Mon Apr 07 15:39:49 2008 +1000 +++ b/include/linux/stop_machine.h Mon Apr 07 15:44:29 2008 +1000 @@ -5,9 +5,9 @@ (and more). So the "read" side to such a lock is anything which diables preeempt. */ #include <linux/cpu.h> +#include <linux/compiler.h> #include <asm/system.h> -#if defined(CONFIG_STOP_MACHINE) && defined(CONFIG_SMP) /** * stop_machine_run: freeze the machine on all CPUs and run this function * @fn: the function to run @@ -21,7 +21,11 @@ * * This can be thought of as a very heavy write lock, equivalent to * grabbing every spinlock in the kernel. */ -int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu); +#define stop_machine_run(fn, data, cpu) \ + stop_machine_run_notype(typesafe_cb(int, (fn), (data)), (data), (cpu)) + +#if defined(CONFIG_STOP_MACHINE) && defined(CONFIG_SMP) +int stop_machine_run_notype(int (*fn)(void *), void *data, unsigned int cpu); /** * __stop_machine_run: freeze the machine on all CPUs and run this function @@ -38,8 +42,8 @@ struct task_struct *__stop_machine_run(i #else -static inline int stop_machine_run(int (*fn)(void *), void *data, - unsigned int cpu) +static inline int stop_machine_run_notype(int (*fn)(void *), void *data, + unsigned int cpu) { int ret; local_irq_disable(); diff -r 103cfcb193b5 kernel/stop_machine.c --- a/kernel/stop_machine.c Mon Apr 07 15:39:49 2008 +1000 +++ b/kernel/stop_machine.c Mon Apr 07 15:44:29 2008 +1000 @@ -197,7 +197,7 @@ struct task_struct *__stop_machine_run(i return p; } -int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) +int stop_machine_run_notype(int (*fn)(void *), void *data, unsigned int cpu) { struct task_struct *p; int ret; @@ -213,4 +213,4 @@ int stop_machine_run(int (*fn)(void *), return ret; } -EXPORT_SYMBOL_GPL(stop_machine_run); +EXPORT_SYMBOL_GPL(stop_machine_run_notype); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/6] typesafe: kthread_create and kthread_run 2008-04-20 23:05 ` [PATCH 3/6] typesafe: Convert stop_machine Rusty Russell @ 2008-04-20 23:07 ` Rusty Russell 2008-04-20 23:09 ` [PATCH 5/6] typesafe: request_irq and devm_request_irq Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:07 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds Straight-forward conversion to allow typesafe callbacks. Needs the previously-posted __attribute__((format)) one-liner patch. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/kthread.h | 28 +++++++++++++++++++++++++--- kernel/kthread.c | 29 +++++------------------------ 2 files changed, 30 insertions(+), 27 deletions(-) diff -r f5a7f7f9683c include/linux/kthread.h --- a/include/linux/kthread.h Mon Apr 07 18:36:20 2008 +1000 +++ b/include/linux/kthread.h Mon Apr 07 19:12:04 2008 +1000 @@ -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))); /** diff -r f5a7f7f9683c kernel/kthread.c --- a/kernel/kthread.c Mon Apr 07 18:36:20 2008 +1000 +++ b/kernel/kthread.c Mon Apr 07 19:12:04 2008 +1000 @@ -112,29 +112,10 @@ static void create_kthread(struct kthrea complete(&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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5/6] typesafe: request_irq and devm_request_irq 2008-04-20 23:07 ` [PATCH 4/6] typesafe: kthread_create and kthread_run Rusty Russell @ 2008-04-20 23:09 ` Rusty Russell 2008-04-20 23:10 ` [PATCH 6/6] typesafe: TIMER_INITIALIZER and setup_timer Rusty Russell 0 siblings, 1 reply; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:09 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds This patch lets interrupt handler functions have their natural type (ie. exactly match the data pointer type); it allows the old irq_handler_t type as well. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/interrupt.h | 14 ++++++++++++-- kernel/irq/devres.c | 10 +++++----- kernel/irq/manage.c | 6 +++--- 3 files changed, 20 insertions(+), 10 deletions(-) diff -r b285fd1d3230 include/linux/interrupt.h --- a/include/linux/interrupt.h Mon Apr 07 15:48:58 2008 +1000 +++ b/include/linux/interrupt.h Mon Apr 07 15:57:39 2008 +1000 @@ -69,13 +69,23 @@ struct irqaction { }; extern irqreturn_t no_action(int cpl, void *dev_id); -extern int __must_check request_irq(unsigned int, irq_handler_t handler, + +/* Typesafe version of request_irq. Also takes old-style void * handlers. */ +#define request_irq(irq, handler, flags, name, dev_id) \ + __request_irq((irq), typesafe_cb_preargs(int,(handler),(dev_id),int), \ + (flags), (name), (dev_id)) + +extern int __must_check __request_irq(unsigned int, irq_handler_t handler, unsigned long, const char *, void *); extern void free_irq(unsigned int, void *); struct device; -extern int __must_check devm_request_irq(struct device *dev, unsigned int irq, +#define devm_request_irq(dev, irq, handler, flags, name, dev_id) \ + __devm_request_irq((dev), (irq), \ + typesafe_cb_preargs(int, (handler), (dev_id), int), \ + (flags), (name), (dev_id)) +extern int __must_check __devm_request_irq(struct device *dev, unsigned int irq, irq_handler_t handler, unsigned long irqflags, const char *devname, void *dev_id); extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id); diff -r b285fd1d3230 kernel/irq/devres.c --- a/kernel/irq/devres.c Mon Apr 07 15:48:58 2008 +1000 +++ b/kernel/irq/devres.c Mon Apr 07 15:57:39 2008 +1000 @@ -41,9 +41,9 @@ static int devm_irq_match(struct device * If an IRQ allocated with this function needs to be freed * separately, dev_free_irq() must be used. */ -int devm_request_irq(struct device *dev, unsigned int irq, - irq_handler_t handler, unsigned long irqflags, - const char *devname, void *dev_id) +int __devm_request_irq(struct device *dev, unsigned int irq, + irq_handler_t handler, unsigned long irqflags, + const char *devname, void *dev_id) { struct irq_devres *dr; int rc; @@ -53,7 +53,7 @@ int devm_request_irq(struct device *dev, if (!dr) return -ENOMEM; - rc = request_irq(irq, handler, irqflags, devname, dev_id); + rc = __request_irq(irq, handler, irqflags, devname, dev_id); if (rc) { devres_free(dr); return rc; @@ -65,7 +65,7 @@ int devm_request_irq(struct device *dev, return 0; } -EXPORT_SYMBOL(devm_request_irq); +EXPORT_SYMBOL(__devm_request_irq); /** * devm_free_irq - free an interrupt diff -r b285fd1d3230 kernel/irq/manage.c --- a/kernel/irq/manage.c Mon Apr 07 15:48:58 2008 +1000 +++ b/kernel/irq/manage.c Mon Apr 07 15:57:39 2008 +1000 @@ -517,8 +517,8 @@ EXPORT_SYMBOL(free_irq); * IRQF_SAMPLE_RANDOM The interrupt can be used for entropy * */ -int request_irq(unsigned int irq, irq_handler_t handler, - unsigned long irqflags, const char *devname, void *dev_id) +int __request_irq(unsigned int irq, irq_handler_t handler, + unsigned long irqflags, const char *devname, void *dev_id) { struct irqaction *action; int retval; @@ -579,4 +579,4 @@ int request_irq(unsigned int irq, irq_ha return retval; } -EXPORT_SYMBOL(request_irq); +EXPORT_SYMBOL(__request_irq); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 6/6] typesafe: TIMER_INITIALIZER and setup_timer 2008-04-20 23:09 ` [PATCH 5/6] typesafe: request_irq and devm_request_irq Rusty Russell @ 2008-04-20 23:10 ` Rusty Russell 0 siblings, 0 replies; 7+ messages in thread From: Rusty Russell @ 2008-04-20 23:10 UTC (permalink / raw) To: linux-kernel; +Cc: Andrew Morton, Al Viro, Linus Torvalds 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. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/linux/timer.h | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff -r 1a72bd2d9ea6 include/linux/timer.h --- a/include/linux/timer.h Mon Apr 21 07:04:32 2008 +1000 +++ b/include/linux/timer.h Mon Apr 21 07:28:46 2008 +1000 @@ -24,11 +24,21 @@ struct timer_list { extern struct tvec_base boot_tvec_bases; -#define TIMER_INITIALIZER(_function, _expires, _data) { \ - .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) { \ + .function = typesafe_timerfn((_function), (_data)), \ + .expires = (_expires), \ + .data = (unsigned long)(_data), \ + .base = &boot_tvec_bases, \ } #define DEFINE_TIMER(_name, _function, _expires, _data) \ @@ -38,9 +48,13 @@ void init_timer(struct timer_list *timer void init_timer(struct timer_list *timer); void init_timer_deferrable(struct timer_list *timer); -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; ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-04-21 3:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-20 22:58 [PATCH 0/6] typesafe callbacks Rusty Russell 2008-04-20 23:00 ` [PATCH 1/6] cast_if_type: allow macros functions which take more than one type Rusty Russell 2008-04-20 23:01 ` [PATCH 2/6] typesafe_cb: wrappers for typesafe callbacks Rusty Russell 2008-04-20 23:05 ` [PATCH 3/6] typesafe: Convert stop_machine Rusty Russell 2008-04-20 23:07 ` [PATCH 4/6] typesafe: kthread_create and kthread_run Rusty Russell 2008-04-20 23:09 ` [PATCH 5/6] typesafe: request_irq and devm_request_irq Rusty Russell 2008-04-20 23:10 ` [PATCH 6/6] typesafe: TIMER_INITIALIZER and setup_timer Rusty Russell
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).