Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <cfdf8627-8ede-871b-138c-e256fa4b67ee@csgroup.eu>
Date:   Wed, 2 Mar 2022 15:57:19 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH -next] macintosh/therm_adt746x: Replaced simple_strtol()
 with kstrtoint()
Content-Language: fr-FR
To:     Liu Shixin <liushixin2@huawei.com>, Colin Leroy <colin@colino.net>
Cc:     linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
References: <20210524120835.1580420-1-liushixin2@huawei.com>
From:   Christophe Leroy <christophe.leroy@csgroup.eu>
In-Reply-To: <20210524120835.1580420-1-liushixin2@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


Le 24/05/2021 à 14:08, Liu Shixin a écrit :
> The simple_strtol() function is deprecated in some situation since
> it does not check for the range overflow. Use kstrtoint() instead.
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---
>   drivers/macintosh/therm_adt746x.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
> index 7e218437730c..0d7ef55126ce 100644
> --- a/drivers/macintosh/therm_adt746x.c
> +++ b/drivers/macintosh/therm_adt746x.c
> @@ -352,7 +352,8 @@ static ssize_t store_##name(struct device *dev, struct device_attribute *attr, c
>   	struct thermostat *th = dev_get_drvdata(dev);		\
>   	int val;						\
>   	int i;							\
> -	val = simple_strtol(buf, NULL, 10);			\
> +	if (unlikely(kstrtoint(buf, 10, &val))			\
> +		return -EINVAL;					\
>   	printk(KERN_INFO "Adjusting limits by %d degrees\n", val);	\
>   	limit_adjust = val;					\
>   	for (i=0; i < 3; i++)					\
> @@ -364,7 +365,8 @@ static ssize_t store_##name(struct device *dev, struct device_attribute *attr, c
>   static ssize_t store_##name(struct device *dev, struct device_attribute *attr, const char *buf, size_t n) \
>   {								\
>   	int val;						\
> -	val = simple_strtol(buf, NULL, 10);			\
> +	if (unlikely(kstrtoint(buf, 10, &val))			\
> +		return -EINVAL;					\
>   	if (val < 0 || val > 255)				\
>   		return -EINVAL;					\
>   	printk(KERN_INFO "Setting specified fan speed to %d\n", val);	\

Obviously no build test has been performed:

   CC [M]  drivers/macintosh/therm_adt746x.o
drivers/macintosh/therm_adt746x.c: In function 'store_specified_fan_speed':
drivers/macintosh/therm_adt746x.c:369:17: error: expected ')' before 
'return'
   369 |                 return -EINVAL;                                 \
       |                 ^~~~~~
drivers/macintosh/therm_adt746x.c:385:1: note: in expansion of macro 
'BUILD_STORE_FUNC_INT'
   385 | BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:368:12: note: to match this '('
   368 |         if (unlikely(kstrtoint(buf, 10, &val))                  \
       |            ^
drivers/macintosh/therm_adt746x.c:385:1: note: in expansion of macro 
'BUILD_STORE_FUNC_INT'
   385 | BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:375:1: error: expected expression 
before '}' token
   375 | }
       | ^
drivers/macintosh/therm_adt746x.c:385:1: note: in expansion of macro 
'BUILD_STORE_FUNC_INT'
   385 | BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:375:1: error: no return statement in 
function returning non-void [-Werror=return-type]
   375 | }
       | ^
drivers/macintosh/therm_adt746x.c:385:1: note: in expansion of macro 
'BUILD_STORE_FUNC_INT'
   385 | BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c: In function 'store_limit_adjust':
drivers/macintosh/therm_adt746x.c:356:17: error: expected ')' before 
'return'
   356 |                 return -EINVAL;                                 \
       |                 ^~~~~~
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:355:12: note: to match this '('
   355 |         if (unlikely(kstrtoint(buf, 10, &val))                  \
       |            ^
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:362:1: error: expected expression 
before '}' token
   362 | }
       | ^
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:354:13: warning: unused variable 'i' 
[-Wunused-variable]
   354 |         int i;                                                  \
       |             ^
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:352:28: warning: unused variable 'th' 
[-Wunused-variable]
   352 |         struct thermostat *th = dev_get_drvdata(dev);           \
       |                            ^~
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
drivers/macintosh/therm_adt746x.c:362:1: error: no return statement in 
function returning non-void [-Werror=return-type]
   362 | }
       | ^
drivers/macintosh/therm_adt746x.c:391:1: note: in expansion of macro 
'BUILD_STORE_FUNC_DEG'
   391 | BUILD_STORE_FUNC_DEG(limit_adjust,       th)
       | ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:288: 
drivers/macintosh/therm_adt746x.o] Error 1

Christophe