Re: [PATCH -next] macintosh/therm_adt746x: Replaced simple_strtol() with kstrtoint()

[Christophe Leroy]

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