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

From: Andy Shevchenko
Date: Mon Jun 03 2013 - 07:01:48 EST


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/