linux-kernel - Re: [patch 3/3] kmsg: convert xpram messages to kmsg api.

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 30 Jul 2008 12:43:08 -0700
From:	Greg KH <gregkh@...e.de>
To:	Martin Schwidefsky <schwidefsky@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
	lf_kernel_messages@...ts.linux-foundation.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michael Holzheu <holzheu@...ibm.com>,
	Gerrit Huizenga <gh@...ibm.com>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	Jan Kara <jack@...e.cz>, Pavel Machek <pavel@....cz>,
	Sam Ravnborg <sam@...nborg.org>, Joe Perches <joe@...ches.com>,
	Jochen Voß <jochen.voss@...glemail.com>,
	Kunai Takashi <kunai@...ux-foundation.jp>,
	Tim Bird <tim.bird@...sony.com>
Subject: Re: [patch 3/3] kmsg: convert xpram messages to kmsg api.

On Wed, Jul 30, 2008 at 06:56:59PM +0200, Martin Schwidefsky wrote:
> From: Martin Schwidefsky <schwidefsky@...ibm.com>
> 
> Signed-off-by: Martin Schwidefsky <schwidefsky@...ibm.com>
> ---
> 
>  Documentation/kmsg/s390/xpram |   54 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/s390/block/xpram.c    |   41 ++++++++++++++-----------------
>  2 files changed, 73 insertions(+), 22 deletions(-)
> 
> Index: linux-2.6/Documentation/kmsg/s390/xpram
> ===================================================================
> --- /dev/null
> +++ linux-2.6/Documentation/kmsg/s390/xpram
> @@ -0,0 +1,54 @@
> +/*?
> + * Tag: xpram.1

Ick, so you are going to have to define a message number per file?
How is that going to work, it looks like you use ids 0-2 below in the .c
file, yet in this documentation file they are 1-3.  Off by one
somewhere?  :)

> + * Text: "%d is not a valid number of XPRAM devices"
> + * Severity: Error
> + * Parameter:
> + *   @1: number of partitions
> + * Description:
> + * The number of XPRAM partitions specified for the 'devs' module parameter
> + * or with the 'xpram.parts' kernel parameter must be an integer in the
> + * range 1 to 32. The XPRAM device driver created a maximum of 32 partitions
> + * that are probably not configured as intended.
> + * User action:
> + * If the XPRAM device driver has been compiled as a separate module,
> + * unload the module and load it again with a correct value for the
> + * 'devs' module parameter. If the XPRAM device driver has been compiled
> + * into the kernel, correct the 'xpram.parts' parameter in the kernel
> + * parameter line and restart Linux.
> + */

Any way to put this stuff in the .c file itself?  It's hard enough
getting people to update Documentation/ABI/, knowing to go modify
something else in the Documentation directory is going to be _very_
difficult.


>  
> +#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead?  That makes it one less
thing you have to define in the code (and forget about when moving files
around or cut-and-pasting).

>  	/* Check number of devices. */
>  	if (devs <= 0 || devs > XPRAM_MAX_DEVS) {
> -		PRINT_ERR("invalid number %d of devices\n",devs);
> +		kmsg_err(1, "%d is not a valid number of XPRAM devices\n",devs);
>  		return -EINVAL;

Magic number "1" here?  See comment above about this.

thanks,

greg k-h
--
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/