Re: [PATCH 1/2] hwmon: (lm80) fixed checkpatch errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2012-01-10 at 13:50 -0500, Frans Meulenbroeks wrote:
> 
> 
> 2012/1/10 Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
>         Hi Frans,
>         
>         On Tue, 2012-01-10 at 09:49 -0500, Frans Meulenbroeks wrote:
>         > fixed:
>         > WARNING: simple_strtol is obsolete, use kstrtol instead
>         > WARNING: simple_strtoul is obsolete, use kstrtoul instead
>         >
>         > Signed-off-by: Frans Meulenbroeks
>         <fransmeulenbroeks@xxxxxxxxx>
>         > ---
>         >  drivers/hwmon/lm80.c |   20 ++++++++++++++++----
>         >  1 files changed, 16 insertions(+), 4 deletions(-)
>         >
>         > diff --git a/drivers/hwmon/lm80.c b/drivers/hwmon/lm80.c
>         > index 616f470..18e5ea4 100644
>         > --- a/drivers/hwmon/lm80.c
>         > +++ b/drivers/hwmon/lm80.c
>         > @@ -185,7 +185,10 @@ static ssize_t set_in_##suffix(struct
>         device *dev, \
>         >       int nr = to_sensor_dev_attr(attr)->index; \
>         >       struct i2c_client *client = to_i2c_client(dev); \
>         >       struct lm80_data *data = i2c_get_clientdata(client); \
>         > -     long val = simple_strtol(buf, NULL, 10); \
>         > +     long val; \
>         > +     int err = kstrtol(buf, 10, &val); \
>         > +     if (err < 0) \
>         > +             return err; \
>         >  \
>         >       mutex_lock(&data->update_lock);\
>         >       data->value[nr] = IN_TO_REG(val); \
>         > @@ -226,7 +229,10 @@ static ssize_t set_fan_min(struct
>         device *dev, struct device_attribute *attr,
>         >       int nr = to_sensor_dev_attr(attr)->index;
>         >       struct i2c_client *client = to_i2c_client(dev);
>         >       struct lm80_data *data = i2c_get_clientdata(client);
>         > -     long val = simple_strtoul(buf, NULL, 10);
>         > +     long val;
>         > +     int err = kstrtol(buf, 10, &val);
>         > +     if (err < 0)
>         > +             return err;
>         >
>         
>         You replaced simple_strtoul() with kstrtol(). Should have been
>         unsigned
>         long and kstrtoul(). No need to resubmit, I fixed that. Queued
>         for -rc2.
>         
>         Thanks,
>         Guenter
> 
> Hi  Guenter, 
> 
> The change was on two places and intentional.
> 
> The original code reads:
> long val = simple_strtoul(buf, NULL, 10);
> I feel this is a flaw and that simple_strtol should have been used
> here (and eventually kstrtol.
> (or alternately the variable should have been an unsigned long)
> 
Frans,

I am lost. Why, and what do you think is the flaw ? You said yourself in
the other thread that negative values don't make sense here. Why do you
want to successfully parse negative values if they are not supported.?

I'll drop the two patches for now until that is resolved.

Guenter




_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux