From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753409AbbG2GzW (ORCPT <rfc822;w@1wt.eu>);
	Wed, 29 Jul 2015 02:55:22 -0400
Received: from mx2.suse.de ([195.135.220.15]:48857 "EHLO mx2.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752259AbbG2GzT (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Wed, 29 Jul 2015 02:55:19 -0400
Date: Wed, 29 Jul 2015 08:55:16 +0200
Message-ID: <s5hpp3be9m3.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -mm v2] sound: convert to parse_integer()
In-Reply-To: <20150727210301.GA24623@p183.telecom.by>
References: <55ad7455.egmJBODfpfpsZpe5%akpm@linux-foundation.org>
	<s5hlhe6vyq2.wl-tiwai@suse.de>
	<20150727210301.GA24623@p183.telecom.by>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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