linux-kernel - Re: [RFC PATCH] printk: Convert pr

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1267738192.12993.48.camel@Joe-Laptop.home>
Date:	Thu, 04 Mar 2010 13:29:52 -0800
From:	Joe Perches <joe@...ches.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] printk: Convert pr_<level> macros to functions

On Thu, 2010-03-04 at 12:49 -0800, Andrew Morton wrote:
> On Wed, 03 Mar 2010 07:20:18 -0800 Joe Perches <joe@...ches.com> wrote:
> > 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.

I think it'll be easier to use the %pV construct that
I posted today:

http://lkml.org/lkml/2010/3/4/19

> Is it possible to reduce the amount of
> code movement in the patch, make it a bit easier to follow?

I can do the move-around separately.

I think it'll be better as:

asmlinkage int pr_emerg(const char *fmt, ...)
{
	struct va_format vaf;
	va_list args;

	va_start(args, fmt);
	vaf.fmt = fmt;
	vaf.va = &args;
	r = printk(KERN_EMERG "%pV", &vaf);
	va_end(args);

	return r;
}
EXPORT_SYMBOL(pr_emerg);

I believe that avoids any racing.

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

Maybe something like:

#define define_pr_level(function, level)	\
asmlinkage int function(const char *fmt, ...)	\
{						\
	struct va_format vaf;			\
	va_list args;				\
						\
	va_start(args, fmt);			\
	vaf.fmt = fmt;				\
	vaf.va = &args;				\
	r = printk(level "%pV", &vaf);		\
	va_end(args);				\
						\
	return r;				\
}						\
EXPORT_SYMBOL(function)

define_pr_level(pr_emerg,	KERN_EMERG);
define_pr_level(pr_alert,	KERN_ALERT);
define_pr_level(pr_crit,	KERN_CRIT);
define_pr_level(pr_err,		KERN_ERR);
define_pr_level(pr_warning,	KERN_WARNING);
define_pr_level(pr_notice,	KERN_NOTICE);
define_pr_level(pr_info,	KERN_INFO);

I think that's a bit ugly though myself.

> Or perhaps we can do it via a helper function which takes the
> additional argument?
> asmlinkage int pr_everything(char levelchar, const char *fmt, ...)

I believe that could not work.
The symbol names to link against wouldn't be found.


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