On Friday, July 19, 2013 5:49 PM, Heiko Carstens wrote:
> On Fri, Jul 19, 2013 at 04:21:28PM +0900, Jingoo Han wrote:
> > The usage of strict_strtoul() and strict_strtol() is not preferred,
> > because strict_strtoul() and strict_strtol() are obsolete. Thus,
> > kstrtoul() and kstrtol() should be used.
> >
> > Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx>
>
> Nice cleanup, however I'm not going to apply this one:
>
> > @@ -1220,13 +1224,15 @@ dasd_expires_store(struct device *dev, struct device_attribute *attr,
> > {
> > struct dasd_device *device;
> > unsigned long val;
> > + int err;
> >
> > device = dasd_device_from_cdev(to_ccwdev(dev));
> > if (IS_ERR(device))
> > return -ENODEV;
> > -
> > - if ((strict_strtoul(buf, 10, &val) != 0) ||
> > - (val > DASD_EXPIRES_MAX) || val == 0) {
> > + err = kstrtoul(buf, 10, &val);
> > + if (err)
> > + return err;
> > + if ((val > DASD_EXPIRES_MAX) || val == 0)
> > dasd_put_device(device);
> > return -EINVAL;
> > }
>
> -ECOMPILE (missing opening brace at if statement).
>
> > @@ -1302,13 +1314,17 @@ dasd_timeout_store(struct device *dev, struct device_attribute *attr,
> > struct dasd_device *device;
> > struct request_queue *q;
> > unsigned long val, flags;
> > + int err;
> >
> > device = dasd_device_from_cdev(to_ccwdev(dev));
> > if (IS_ERR(device) || !device->block)
> > return -ENODEV;
> >
> > - if ((strict_strtoul(buf, 10, &val) != 0) ||
> > - val > UINT_MAX / HZ) {
> > + err = kstrtoul(buf, 10, &val);
> > + if (err)
> > + return err;
> > +
>
> Missing dasd_put_device().
>
> > diff --git a/drivers/s390/cio/ccwgroup.c b/drivers/s390/cio/ccwgroup.c
> > index 84846c2..959135a 100644
> > --- a/drivers/s390/cio/ccwgroup.c
> > +++ b/drivers/s390/cio/ccwgroup.c
> > @@ -137,7 +137,7 @@ static ssize_t ccwgroup_online_store(struct device *dev,
> > if (!try_module_get(gdrv->driver.owner))
> > return -EINVAL;
> >
> > - ret = strict_strtoul(buf, 0, &value);
> > + ret = kstrtoul(buf, 0, &value);
> > if (ret)
> > goto out;
>
> If all conversions were like this one, I would apply your patch.
OK, I see.
>
> > --- a/drivers/s390/net/qeth_l3_sys.c
> > +++ b/drivers/s390/net/qeth_l3_sys.c
> > @@ -208,11 +208,10 @@ static ssize_t qeth_l3_dev_sniffer_store(struct device *dev,
> > goto out;
> > }
> >
> > - rc = strict_strtoul(buf, 16, &i);
> > - if (rc) {
> > - rc = -EINVAL;
> > + rc = kstrtoul(buf, 16, &i);
> > + if (rc)
> > goto out;
>
> This hunk e.g. changes the behaviour of a user space interface. Before
> your patch -ERANGE was mapped to -EINVAL. Now it is not true anymore.
> This may or may not be a problem, however I'd like to keep the behaviour
> as is in order to not risk any user space breakage.
I agree with you opinion. :)
Thank you for informing me.
>
> So if you could resend a new version which just mechanically replaces
> strict_strtoul() witch kstrtoul() that would be great.
OK, I will resend v2 patch which "just mechanically replaces strict_strtoul()
witch kstrtoul()".
I appreciate your comments.
Best regards,
Jingoo Han
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
