Messages in this thread | | | Date | Mon, 3 Jun 2013 14:01:39 +0300 | Subject | Re: [PATCH V2] misc: replace strict_strtoul() with kstrtoul() | From | Andy Shevchenko <> |
| |
On Mon, Jun 3, 2013 at 12:08 PM, Jingoo Han <jg1.han@samsung.com> 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@samsung.com>
There are few issues. Afrter addressing them take my Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> --- 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
| |