[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
