Re: [PATCH -mm v2] sound: convert to parse_integer()

[Takashi Iwai]

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@xxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/