linux-kernel - Re: [PATCH -mm v2] sound: convert to parse

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] [day] [month] [year] [list]

Date:	Wed, 29 Jul 2015 08:55:16 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -mm v2] sound: convert to parse_integer()

On Mon, 27 Jul 2015 23:03:01 +0200,
Alexey Dobriyan wrote:
> 
> Convert away from deprecated simple_strto*() interfaces to
> parse_integer() and kstrto*().
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@...il.com>

The error handling looks good to me.  In addition to Andrew's
suggestion and the removal of word termination check, some nitpicking
below:

> --- a/sound/core/oss/mixer_oss.c
> +++ b/sound/core/oss/mixer_oss.c
> @@ -1180,6 +1180,7 @@ static void snd_mixer_oss_proc_write(struct snd_info_entry *entry,
>  	int ch, idx;
>  	struct snd_mixer_oss_assign_table *tbl;
>  	struct slot *slot;
> +	int rv;

A more common variable name is err or ret for such a purpose.

> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -250,7 +250,7 @@ static ssize_t codec_reg_write_file(struct file *file,
>  	char buf[32];
>  	size_t buf_size;
>  	char *start = buf;
> -	unsigned long reg, value;
> +	unsigned int reg, value;
>  	struct snd_soc_codec *codec = file->private_data;
>  	int ret;
>  
> @@ -261,10 +261,13 @@ static ssize_t codec_reg_write_file(struct file *file,
>  
>  	while (*start == ' ')
>  		start++;
> -	reg = simple_strtoul(start, &start, 16);
> +	ret = parse_integer(start, 16, &reg);
> +	if (ret < 0)
> +		return ret;
> +	start += ret;
>  	while (*start == ' ')
>  		start++;
> -	ret = kstrtoul(start, 16, &value);
> +	ret = kstrtouint(start, 16, &value);
>  	if (ret)
>  		return ret;

This looks inconsistent, the first one uses parse_integer() while the
second kstrtouint().  Better to stick with one API.


thanks,

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