Re: [tip:x86/cleanups] x86/early_printk: Replace obsoletesimple_strtoul() usage with kstrtoint()

[Ingo Molnar]

* tip-bot for Shuah Khan <shuahkhan@xxxxxxxxx> wrote:

> Commit-ID:  fbd24153c48b8425b09c161a020483cd77da870e
> Gitweb:     http://git.kernel.org/tip/fbd24153c48b8425b09c161a020483cd77da870e
> Author:     Shuah Khan <shuahkhan@xxxxxxxxx>
> AuthorDate: Wed, 30 May 2012 18:40:03 -0600
> Committer:  Ingo Molnar <mingo@xxxxxxxxxx>
> CommitDate: Wed, 6 Jun 2012 11:44:22 +0200
> 
> x86/early_printk: Replace obsolete simple_strtoul() usage with kstrtoint()
> 
> Change early_serial_init() to call kstrtoul() instead of calling
> obsoleted simple_strtoul().
> 
> Signed-off-by: Shuah Khan <shuahkhan@xxxxxxxxx>
> Cc: Joe Perches <joe@xxxxxxxxxxx>
> Link: http://lkml.kernel.org/r/1338424803.3569.5.camel@lorien2
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
>  arch/x86/kernel/early_printk.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> index 9b9f18b..5e47712 100644
> --- a/arch/x86/kernel/early_printk.c
> +++ b/arch/x86/kernel/early_printk.c
> @@ -119,7 +119,7 @@ static __init void early_serial_init(char *s)
>  	unsigned char c;
>  	unsigned divisor;
>  	unsigned baud = DEFAULT_BAUD;
> -	char *e;
> +	ssize_t ret;
>  
>  	if (*s == ',')
>  		++s;
> @@ -127,14 +127,14 @@ static __init void early_serial_init(char *s)
>  	if (*s) {
>  		unsigned port;
>  		if (!strncmp(s, "0x", 2)) {
> -			early_serial_base = simple_strtoul(s, &e, 16);
> +			ret = kstrtoint(s, 16, &early_serial_base);
>  		} else {
>  			static const int __initconst bases[] = { 0x3f8, 0x2f8 };
>  
>  			if (!strncmp(s, "ttyS", 4))
>  				s += 4;
> -			port = simple_strtoul(s, &e, 10);
> -			if (port > 1 || s == e)
> +			ret = kstrtouint(s, 10, &port);
> +			if (ret || port > 1)
>  				port = 0;
>  			early_serial_base = bases[port];
>  		}
> @@ -149,8 +149,8 @@ static __init void early_serial_init(char *s)
>  	outb(0x3, early_serial_base + MCR);	/* DTR + RTS */
>  
>  	if (*s) {
> -		baud = simple_strtoul(s, &e, 0);
> -		if (baud == 0 || s == e)
> +		ret = kstrtouint(s, 0, &baud);
> +		if (ret || baud == 0)
>  			baud = DEFAULT_BAUD;
>  	}

This commit is quite buggy: kstrto*int() can return an error but 
it's not checked in every path above. simple_strtoul() on the 
other hand could not fail, so this patch subtly intruduces new 
failure modes.

I'm dropping this patch.

Thanks,

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