Re: [PATCH v2 1/2] Return a value from printk_ratelimited

From: Steven Rostedt
Date: Fri Sep 19 2014 - 13:21:33 EST


On Fri, 19 Sep 2014 02:01:29 -0700
Omar Sandoval <osandov@xxxxxxxxxxx> wrote:

> printk returns an integer; there's no reason for printk_ratelimited to swallow
> it.
>
> Signed-off-by: Omar Sandoval <osandov@xxxxxxxxxxx>
> ---
> include/linux/printk.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index d78125f..67534bc 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -343,12 +343,14 @@ extern asmlinkage void dump_stack(void) __cold;
> #ifdef CONFIG_PRINTK
> #define printk_ratelimited(fmt, ...) \
> ({ \
> + int __ret = 0; \

My only issues is with the "__ret" name.

It's not really unique enough. If something else uses __ret and does

printk_ratelimit("some fmt string %d\n", __ret);

This will not print the right value.

printk_ratelimit can be used almost anywhere thus using a really unique
value may be worth while here.

What about:

int ______r

?

-- Steve

> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> \
> if (__ratelimit(&_rs)) \
> - printk(fmt, ##__VA_ARGS__); \
> + __ret = printk(fmt, ##__VA_ARGS__); \
> + __ret; \
> })
> #else
> #define printk_ratelimited(fmt, ...) \

--
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/