[Adding lkml,lm-sensors to cc]
On Sat, Nov 15, 2008 at 04:33:03PM +0100, Julia Lawall wrote:
> Hello,
>
> I saw your recent patch that converts calls to simple_strtol to
> strict_strtol and wondered if the same transformation would be useful
> elsewhere.
I think so.
> diff -u -p a/drivers/hwmon/ad7414.c b/drivers/hwmon/ad7414.c
> diff -u -p a/drivers/hwmon/ad7418.c b/drivers/hwmon/ad7418.c
> diff -u -p a/drivers/hwmon/adm1029.c b/drivers/hwmon/adm1029.c
> diff -u -p a/drivers/hwmon/f71805f.c b/drivers/hwmon/f71805f.c
> diff -u -p a/drivers/hwmon/gl520sm.c b/drivers/hwmon/gl520sm.c
> diff -u -p a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
> diff -u -p a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> diff -u -p a/drivers/hwmon/lm83.c b/drivers/hwmon/lm83.c
> diff -u -p a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> diff -u -p a/drivers/hwmon/lm93.c b/drivers/hwmon/lm93.c
Seems like a generally good idea.
> There are a few other cases where the call to simple_strtol is part of a
> more complicated expression or where the result is stored in a variable of
> type int, rather than long. I didn't check for cases where the buffer
> parameter is not declared as const.
Since it's a sysfs write attribute, I think it's always const.
int/long confusion might be a problem if sizeof(int) != sizeof(long). I
think those are called LP64 machines, but I don't know if Linux runs in
those environments (it probably does).
> I was wondering whether you think the transformation is of more general
> interest than the cases you have considered, and whether it is worthwhile
Yes!
> to pursue this (and whether you have done it all already in some patch I
> didn't notice).
I have only patched those two drivers (adt7470/73) that I wrote myself.
I don't really have time to clean up the other drivers, so thank you for
coming along with a cool-looking cleanup tool. :)
> A concern is whether it is always reasonable to return -EINVAL. I assume
As far as the hwmon drivers go, I think that it's fairly safe to return
-EINVAL if the input is something totally unintelligible.
> it is, because these functions are all stored in the same kind of
> structure, but there is perhaps something I am overlooking. In
> particular, I didn't manage to find out where these functions are actually
> called.
Something like:
echo notanumber > /sys/class/hwmon/hwmon0/device/temp1_max
--D