Re: [PATCH V2] misc: replace strict_strtoul() with kstrtoul()

[Andy Shevchenko]

On Mon, Jun 3, 2013 at 12:08 PM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
> The usage of strict_strtoul() is not preferred, because
> strict_strtoul() is obsolete. Thus, kstrtoul() should be
> used.
>
> Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx>

There are few issues. Afrter addressing them take my
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> --- a/drivers/misc/isl29003.c
> +++ b/drivers/misc/isl29003.c
> @@ -208,8 +208,9 @@ static ssize_t isl29003_store_range(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 3))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 3);
> +       if (ret)
> +               return ret;

It's not correct.
You have to get return code from kstrtoul() separately and use it,
after check top boundary and return -EINVAL.

Three more cases below.

> @@ -239,8 +240,9 @@ static ssize_t isl29003_store_resolution(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 3))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 3);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_resolution(client, val);
>         if (ret < 0)
> @@ -267,8 +269,9 @@ static ssize_t isl29003_store_mode(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 2))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 2);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_mode(client, val);
>         if (ret < 0)
> @@ -298,8 +301,9 @@ static ssize_t isl29003_store_power_state(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 1))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 1);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_power_state(client, val);
>         return ret ? ret : count;

> --- a/drivers/misc/sgi-gru/gruprocfs.c
> +++ b/drivers/misc/sgi-gru/gruprocfs.c
> @@ -161,14 +161,17 @@ static ssize_t options_write(struct file *file, const char __user *userbuf,
>                              size_t count, loff_t *data)
>  {
>         char buf[20];
> +       int ret;
>
>         if (count >= sizeof(buf))
>                 return -EINVAL;
>         if (copy_from_user(buf, userbuf, count))
>                 return -EFAULT;
>         buf[count] = '\0';
> -       if (strict_strtoul(buf, 0, &gru_options))
> -               return -EINVAL;
> +
> +       ret = kstrtoul(buf, 0, &gru_options);

kstrtoul_from_user(), please.

> +       if (ret)
> +               return ret;
>
>         return count;
>  }

--
With Best Regards,
Andy Shevchenko
--
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/