| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 4 Mar 2010 12:49:43 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Joe Perches <joe@...ches.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] printk: Convert pr_<level> macros to functions
On Wed, 03 Mar 2010 07:20:18 -0800
Joe Perches <joe@...ches.com> wrote:
> printk is defined asmlinkage (extern "C")
> For this RFC patch, pr_<level> calls are as well.
> Is this declaration style really necessary?
> Maybe moving printed_len to file scope is racy somehow?
Yes, printed_len will get corrupted when multiple CPU's are running
printk/vprintk simultaneously. That'll need to be fixed.
> Save 3 bytes per pr_<level> use after printk overhead
>
> Does not store the KERN_<level> string as constant string
> when using pr_<level> calls
>
> printk.o increases ~200 bytes
> defconfig decreases ~700 bytes
>
> Minor printk neatening, moving comments above function declaration
> Renamed vprint to __vprintk, added vprintk wrapper
> Moved automatic printed_len to file static
> Moved EXPORT_SYMBOL after functions
>
Looks good at the first peek. Is it possible to reduce the amount of
code movement in the patch, make it a bit easier to follow? Maybe do
it as two patches, with the move-stuff-around patch being [1/2]?
>
> ...
>
> +asmlinkage int pr_emerg(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('0', fmt, args);
> + r = __vprintk(fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_emerg);
> +
> +asmlinkage int pr_alert(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('1', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_alert);
> +
> +asmlinkage int pr_crit(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('2', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_crit);
> +
> +asmlinkage int pr_err(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('3', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_err);
> +
> +asmlinkage int pr_warning(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('4', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_warning);
> +
> +asmlinkage int pr_notice(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('5', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_notice);
> +
> +asmlinkage int pr_info(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('6', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_info);
> +
> +asmlinkage int pr_cont(const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> +
> + va_start(args, fmt);
> + r = pr_printk('c', fmt, args);
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(pr_cont);
I think it would be justifiable to cook up a freaky macro and expand it
eight times to avoid this duplication. Ugly, but better than lots of
duplication.
Or perhaps we can do it via a helper function which takes the
additional argument?
asmlinkage int pr_everything(char levelchar, const char *fmt, ...)
?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists