[PATCH 1/2] Hwmon: f71882fg: fixed braces coding style issues

[PATCH 1/2] Hwmon: f71882fg: fixed braces coding style issues

[Giel van Schijndel]

Fixed several coding style issues.

Signed-off-by: Giel van Schijndel <me@mortis.eu>
---
 drivers/hwmon/f71882fg.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
index a95fa42..21bc661 100644
--- a/drivers/hwmon/f71882fg.c
+++ b/drivers/hwmon/f71882fg.c
@@ -866,11 +866,11 @@ static int superio_inw(int base, int reg)
 static inline void superio_enter(int base)
 {
 	/* according to the datasheet the key must be send twice! */
-	outb( SIO_UNLOCK_KEY, base);
-	outb( SIO_UNLOCK_KEY, base);
+	outb(SIO_UNLOCK_KEY, base);
+	outb(SIO_UNLOCK_KEY, base);
 }
 
-static inline void superio_select( int base, int ld)
+static inline void superio_select(int base, int ld)
 {
 	outb(SIO_REG_LDSEL, base);
 	outb(ld, base + 1);
@@ -945,7 +945,7 @@ static struct f71882fg_data *f71882fg_update_device(struct device *dev)
 	mutex_lock(&data->update_lock);
 
 	/* Update once every 60 seconds */
-	if ( time_after(jiffies, data->last_limits + 60 * HZ ) ||
+	if (time_after(jiffies, data->last_limits + 60 * HZ) ||
 			!data->valid) {
 		if (data->type == f71882fg || data->type == f71889fg) {
 			data->in1_max =
@@ -2151,8 +2151,7 @@ static int __init f71882fg_find(int sioaddr, unsigned short *address,
 	}
 
 	*address = superio_inw(sioaddr, SIO_REG_ADDR);
-	if (*address == 0)
-	{
+	if (*address == 0) {
 		printk(KERN_WARNING DRVNAME ": Base address not set\n");
 		goto exit;
 	}
-- 
1.6.4.4

[PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Giel van Schijndel]

Use the strict_strol and strict_stroul functions instead of simple_strol
and simple_stroul respectively in sysfs functions.

Signed-off-by: Giel van Schijndel <me@mortis.eu>
---
 drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
index 21bc661..5c725a3 100644
--- a/drivers/hwmon/f71882fg.c
+++ b/drivers/hwmon/f71882fg.c
@@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	val = SENSORS_LIMIT(val, 23, 1500000);
 	val = fan_to_reg(val);
@@ -1158,7 +1161,10 @@ static ssize_t store_fan_beep(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 	data->fan_beep = f71882fg_read8(data, F71882FG_REG_FAN_BEEP);
@@ -1206,7 +1212,12 @@ static ssize_t store_in_max(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	long val = simple_strtol(buf, NULL, 10) / 8;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 8;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1234,7 +1245,10 @@ static ssize_t store_in_beep(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 	data->in_beep = f71882fg_read8(data, F71882FG_REG_IN_BEEP);
@@ -1300,7 +1314,12 @@ static ssize_t store_temp_max(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 1000;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1334,9 +1353,14 @@ static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
 	ssize_t ret = count;
 	u8 reg;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 1000;
 
 	mutex_lock(&data->update_lock);
 
@@ -1373,7 +1397,12 @@ static ssize_t store_temp_crit(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 1000;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1428,7 +1457,10 @@ static ssize_t store_temp_beep(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 	data->temp_beep = f71882fg_read8(data, F71882FG_REG_TEMP_BEEP);
@@ -1491,7 +1523,11 @@ static ssize_t store_pwm(struct device *dev,
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1552,7 +1588,10 @@ static ssize_t store_pwm_enable(struct device *dev, struct device_attribute
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	/* Special case for F8000 pwm channel 3 which only does auto mode */
 	if (data->type == f8000 && nr == 2 && val != 2)
@@ -1628,7 +1667,11 @@ static ssize_t store_pwm_auto_point_pwm(struct device *dev,
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1676,8 +1719,13 @@ static ssize_t store_pwm_auto_point_temp_hyst(struct device *dev,
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
 	u8 reg;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 1000;
 
 	mutex_lock(&data->update_lock);
 	data->pwm_auto_point_temp[nr][point] =
@@ -1717,7 +1765,10 @@ static ssize_t store_pwm_interpolate(struct device *dev,
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	unsigned long val;
+
+	if (strict_strtoul(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 	data->pwm_auto_point_mapping[nr] =
@@ -1753,7 +1804,10 @@ static ssize_t store_pwm_auto_point_channel(struct device *dev,
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
 
 	switch (val) {
 	case 1:
@@ -1800,7 +1854,12 @@ static ssize_t store_pwm_auto_point_temp(struct device *dev,
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	long val;
+
+	if (strict_strtol(buf, 10, &val) == -EINVAL)
+		return -EINVAL;
+
+	val /= 1000;
 
 	if (data->type == f71889fg)
 		val = SENSORS_LIMIT(val, -128, 127);
-- 
1.6.4.4

Re: [PATCH 1/2] Hwmon: f71882fg: fixed braces coding style issues

[Hans de Goede]

Ack.

Acked-by: Hans de Goede <hdegoede@redhat.com>

On 03/21/2010 04:37 PM, Giel van Schijndel wrote:
> Fixed several coding style issues.
>
> Signed-off-by: Giel van Schijndel<me@mortis.eu>
> ---
>   drivers/hwmon/f71882fg.c |   11 +++++------
>   1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> index a95fa42..21bc661 100644
> --- a/drivers/hwmon/f71882fg.c
> +++ b/drivers/hwmon/f71882fg.c
> @@ -866,11 +866,11 @@ static int superio_inw(int base, int reg)
>   static inline void superio_enter(int base)
>   {
>   	/* according to the datasheet the key must be send twice! */
> -	outb( SIO_UNLOCK_KEY, base);
> -	outb( SIO_UNLOCK_KEY, base);
> +	outb(SIO_UNLOCK_KEY, base);
> +	outb(SIO_UNLOCK_KEY, base);
>   }
>
> -static inline void superio_select( int base, int ld)
> +static inline void superio_select(int base, int ld)
>   {
>   	outb(SIO_REG_LDSEL, base);
>   	outb(ld, base + 1);
> @@ -945,7 +945,7 @@ static struct f71882fg_data *f71882fg_update_device(struct device *dev)
>   	mutex_lock(&data->update_lock);
>
>   	/* Update once every 60 seconds */
> -	if ( time_after(jiffies, data->last_limits + 60 * HZ ) ||
> +	if (time_after(jiffies, data->last_limits + 60 * HZ) ||
>   			!data->valid) {
>   		if (data->type == f71882fg || data->type == f71889fg) {
>   			data->in1_max =
> @@ -2151,8 +2151,7 @@ static int __init f71882fg_find(int sioaddr, unsigned short *address,
>   	}
>
>   	*address = superio_inw(sioaddr, SIO_REG_ADDR);
> -	if (*address == 0)
> -	{
> +	if (*address == 0) {
>   		printk(KERN_WARNING DRVNAME ": Base address not set\n");
>   		goto exit;
>   	}

Re: [PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Hans de Goede]

Ack.

Acked-by: Hans de Goede <hdegoede@redhat.com>


On 03/21/2010 04:37 PM, Giel van Schijndel wrote:
> Use the strict_strol and strict_stroul functions instead of simple_strol
> and simple_stroul respectively in sysfs functions.
>
> Signed-off-by: Giel van Schijndel<me@mortis.eu>
> ---
>   drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
>   1 files changed, 74 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> index 21bc661..5c725a3 100644
> --- a/drivers/hwmon/f71882fg.c
> +++ b/drivers/hwmon/f71882fg.c
> @@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	val = SENSORS_LIMIT(val, 23, 1500000);
>   	val = fan_to_reg(val);
> @@ -1158,7 +1161,10 @@ static ssize_t store_fan_beep(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	mutex_lock(&data->update_lock);
>   	data->fan_beep = f71882fg_read8(data, F71882FG_REG_FAN_BEEP);
> @@ -1206,7 +1212,12 @@ static ssize_t store_in_max(struct device *dev, struct device_attribute
>   	*devattr, const char *buf, size_t count)
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
> -	long val = simple_strtol(buf, NULL, 10) / 8;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 8;
>   	val = SENSORS_LIMIT(val, 0, 255);
>
>   	mutex_lock(&data->update_lock);
> @@ -1234,7 +1245,10 @@ static ssize_t store_in_beep(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	mutex_lock(&data->update_lock);
>   	data->in_beep = f71882fg_read8(data, F71882FG_REG_IN_BEEP);
> @@ -1300,7 +1314,12 @@ static ssize_t store_temp_max(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>   	val = SENSORS_LIMIT(val, 0, 255);
>
>   	mutex_lock(&data->update_lock);
> @@ -1334,9 +1353,14 @@ static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
>   	ssize_t ret = count;
>   	u8 reg;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>
>   	mutex_lock(&data->update_lock);
>
> @@ -1373,7 +1397,12 @@ static ssize_t store_temp_crit(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>   	val = SENSORS_LIMIT(val, 0, 255);
>
>   	mutex_lock(&data->update_lock);
> @@ -1428,7 +1457,10 @@ static ssize_t store_temp_beep(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	mutex_lock(&data->update_lock);
>   	data->temp_beep = f71882fg_read8(data, F71882FG_REG_TEMP_BEEP);
> @@ -1491,7 +1523,11 @@ static ssize_t store_pwm(struct device *dev,
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
>   	val = SENSORS_LIMIT(val, 0, 255);
>
>   	mutex_lock(&data->update_lock);
> @@ -1552,7 +1588,10 @@ static ssize_t store_pwm_enable(struct device *dev, struct device_attribute
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	/* Special case for F8000 pwm channel 3 which only does auto mode */
>   	if (data->type == f8000&&  nr == 2&&  val != 2)
> @@ -1628,7 +1667,11 @@ static ssize_t store_pwm_auto_point_pwm(struct device *dev,
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int pwm = to_sensor_dev_attr_2(devattr)->index;
>   	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
>   	val = SENSORS_LIMIT(val, 0, 255);
>
>   	mutex_lock(&data->update_lock);
> @@ -1676,8 +1719,13 @@ static ssize_t store_pwm_auto_point_temp_hyst(struct device *dev,
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
>   	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
>   	u8 reg;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>
>   	mutex_lock(&data->update_lock);
>   	data->pwm_auto_point_temp[nr][point] =
> @@ -1717,7 +1765,10 @@ static ssize_t store_pwm_interpolate(struct device *dev,
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	mutex_lock(&data->update_lock);
>   	data->pwm_auto_point_mapping[nr] =
> @@ -1753,7 +1804,10 @@ static ssize_t store_pwm_auto_point_channel(struct device *dev,
>   {
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
>
>   	switch (val) {
>   	case 1:
> @@ -1800,7 +1854,12 @@ static ssize_t store_pwm_auto_point_temp(struct device *dev,
>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>   	int pwm = to_sensor_dev_attr_2(devattr)->index;
>   	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>
>   	if (data->type == f71889fg)
>   		val = SENSORS_LIMIT(val, -128, 127);

Re: [PATCH 1/2] Hwmon: f71882fg: fixed braces coding style issues

[Jean Delvare]

On Mon, 22 Mar 2010 10:30:54 +0100, Hans de Goede wrote:
> Ack.
> 
> Acked-by: Hans de Goede <hdegoede@redhat.com>

Applied, thanks.

> On 03/21/2010 04:37 PM, Giel van Schijndel wrote:
> > Fixed several coding style issues.
> >
> > Signed-off-by: Giel van Schijndel<me@mortis.eu>
> > ---
> >   drivers/hwmon/f71882fg.c |   11 +++++------
> >   1 files changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> > index a95fa42..21bc661 100644
> > --- a/drivers/hwmon/f71882fg.c
> > +++ b/drivers/hwmon/f71882fg.c
> > @@ -866,11 +866,11 @@ static int superio_inw(int base, int reg)
> >   static inline void superio_enter(int base)
> >   {
> >   	/* according to the datasheet the key must be send twice! */
> > -	outb( SIO_UNLOCK_KEY, base);
> > -	outb( SIO_UNLOCK_KEY, base);
> > +	outb(SIO_UNLOCK_KEY, base);
> > +	outb(SIO_UNLOCK_KEY, base);
> >   }
> >
> > -static inline void superio_select( int base, int ld)
> > +static inline void superio_select(int base, int ld)
> >   {
> >   	outb(SIO_REG_LDSEL, base);
> >   	outb(ld, base + 1);
> > @@ -945,7 +945,7 @@ static struct f71882fg_data *f71882fg_update_device(struct device *dev)
> >   	mutex_lock(&data->update_lock);
> >
> >   	/* Update once every 60 seconds */
> > -	if ( time_after(jiffies, data->last_limits + 60 * HZ ) ||
> > +	if (time_after(jiffies, data->last_limits + 60 * HZ) ||
> >   			!data->valid) {
> >   		if (data->type == f71882fg || data->type == f71889fg) {
> >   			data->in1_max =
> > @@ -2151,8 +2151,7 @@ static int __init f71882fg_find(int sioaddr, unsigned short *address,
> >   	}
> >
> >   	*address = superio_inw(sioaddr, SIO_REG_ADDR);
> > -	if (*address == 0)
> > -	{
> > +	if (*address == 0) {
> >   		printk(KERN_WARNING DRVNAME ": Base address not set\n");
> >   		goto exit;
> >   	}


-- 
Jean Delvare

Re: [PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Jean Delvare]

On Sun, 21 Mar 2010 16:37:14 +0100, Giel van Schijndel wrote:
> Use the strict_strol and strict_stroul functions instead of simple_strol
> and simple_stroul respectively in sysfs functions.
> 
> Signed-off-by: Giel van Schijndel <me@mortis.eu>
> ---
>  drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 74 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> index 21bc661..5c725a3 100644
> --- a/drivers/hwmon/f71882fg.c
> +++ b/drivers/hwmon/f71882fg.c
> @@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;

That's not correct. You want to return an error if strict_strtol()
returns _any_ error, not just -EINVAL. Maybe the current implementation
can't return any other error code, but you should not assume this will
always be the case in the future.

>  
>  	val = SENSORS_LIMIT(val, 23, 1500000);
>  	val = fan_to_reg(val);
> @@ -1158,7 +1161,10 @@ static ssize_t store_fan_beep(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	mutex_lock(&data->update_lock);
>  	data->fan_beep = f71882fg_read8(data, F71882FG_REG_FAN_BEEP);
> @@ -1206,7 +1212,12 @@ static ssize_t store_in_max(struct device *dev, struct device_attribute
>  	*devattr, const char *buf, size_t count)
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
> -	long val = simple_strtol(buf, NULL, 10) / 8;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 8;
>  	val = SENSORS_LIMIT(val, 0, 255);
>  
>  	mutex_lock(&data->update_lock);
> @@ -1234,7 +1245,10 @@ static ssize_t store_in_beep(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	mutex_lock(&data->update_lock);
>  	data->in_beep = f71882fg_read8(data, F71882FG_REG_IN_BEEP);
> @@ -1300,7 +1314,12 @@ static ssize_t store_temp_max(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>  	val = SENSORS_LIMIT(val, 0, 255);
>  
>  	mutex_lock(&data->update_lock);
> @@ -1334,9 +1353,14 @@ static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
>  	ssize_t ret = count;
>  	u8 reg;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>  
>  	mutex_lock(&data->update_lock);
>  
> @@ -1373,7 +1397,12 @@ static ssize_t store_temp_crit(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>  	val = SENSORS_LIMIT(val, 0, 255);
>  
>  	mutex_lock(&data->update_lock);
> @@ -1428,7 +1457,10 @@ static ssize_t store_temp_beep(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	mutex_lock(&data->update_lock);
>  	data->temp_beep = f71882fg_read8(data, F71882FG_REG_TEMP_BEEP);
> @@ -1491,7 +1523,11 @@ static ssize_t store_pwm(struct device *dev,
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
>  	val = SENSORS_LIMIT(val, 0, 255);
>  
>  	mutex_lock(&data->update_lock);
> @@ -1552,7 +1588,10 @@ static ssize_t store_pwm_enable(struct device *dev, struct device_attribute
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	/* Special case for F8000 pwm channel 3 which only does auto mode */
>  	if (data->type == f8000 && nr == 2 && val != 2)
> @@ -1628,7 +1667,11 @@ static ssize_t store_pwm_auto_point_pwm(struct device *dev,
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int pwm = to_sensor_dev_attr_2(devattr)->index;
>  	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
>  	val = SENSORS_LIMIT(val, 0, 255);
>  
>  	mutex_lock(&data->update_lock);
> @@ -1676,8 +1719,13 @@ static ssize_t store_pwm_auto_point_temp_hyst(struct device *dev,
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
>  	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
>  	u8 reg;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>  
>  	mutex_lock(&data->update_lock);
>  	data->pwm_auto_point_temp[nr][point] =
> @@ -1717,7 +1765,10 @@ static ssize_t store_pwm_interpolate(struct device *dev,
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	unsigned long val = simple_strtoul(buf, NULL, 10);
> +	unsigned long val;
> +
> +	if (strict_strtoul(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	mutex_lock(&data->update_lock);
>  	data->pwm_auto_point_mapping[nr] =
> @@ -1753,7 +1804,10 @@ static ssize_t store_pwm_auto_point_channel(struct device *dev,
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
>  
>  	switch (val) {
>  	case 1:
> @@ -1800,7 +1854,12 @@ static ssize_t store_pwm_auto_point_temp(struct device *dev,
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>  	int pwm = to_sensor_dev_attr_2(devattr)->index;
>  	int point = to_sensor_dev_attr_2(devattr)->nr;
> -	long val = simple_strtol(buf, NULL, 10) / 1000;
> +	long val;
> +
> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> +		return -EINVAL;
> +
> +	val /= 1000;
>  
>  	if (data->type == f71889fg)
>  		val = SENSORS_LIMIT(val, -128, 127);


-- 
Jean Delvare

Re: [PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Giel van Schijndel]

[-- Attachment #1: Type: text/plain, Size: 9865 bytes --]

On Mon, Mar 22, 2010 at 11:23:08AM +0100, Jean Delvare wrote:
> On Sun, 21 Mar 2010 16:37:14 +0100, Giel van Schijndel wrote:
>> Use the strict_strol and strict_stroul functions instead of simple_strol
>> and simple_stroul respectively in sysfs functions.
>> 
>> Signed-off-by: Giel van Schijndel <me@mortis.eu>
>> ---
>>  drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
>>  1 files changed, 74 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
>> index 21bc661..5c725a3 100644
>> --- a/drivers/hwmon/f71882fg.c
>> +++ b/drivers/hwmon/f71882fg.c
>> @@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
>>  {
>>  	struct f71882fg_data *data = dev_get_drvdata(dev);
>>  	int nr = to_sensor_dev_attr_2(devattr)->index;
>> -	long val = simple_strtol(buf, NULL, 10);
>> +	long val;
>> +
>> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
>> +		return -EINVAL;
>
> That's not correct. You want to return an error if strict_strtol()
> returns _any_ error, not just -EINVAL. Maybe the current
> implementation can't return any other error code, but you should not
> assume this will always be the case in the future.

Agreed. New patch follows this line:
------------------------------------------------------------------------
Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

Use the strict_strol and strict_stroul functions instead of simple_strol
and simple_stroul respectively in sysfs functions.

Signed-off-by: Giel van Schijndel <me@mortis.eu>
---
 drivers/hwmon/f71882fg.c |  133 ++++++++++++++++++++++++++++++++++++----------
 1 files changed, 104 insertions(+), 29 deletions(-)

diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
index 21bc661..4230729 100644
--- a/drivers/hwmon/f71882fg.c
+++ b/drivers/hwmon/f71882fg.c
@@ -1127,8 +1127,12 @@ static ssize_t store_fan_full_speed(struct device *dev,
 				    const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
 
 	val = SENSORS_LIMIT(val, 23, 1500000);
 	val = fan_to_reg(val);
@@ -1157,8 +1161,12 @@ static ssize_t store_fan_beep(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	unsigned long val;
+
+	err = strict_strtoul(buf, 10, &val);
+	if (err)
+		return err;
 
 	mutex_lock(&data->update_lock);
 	data->fan_beep = f71882fg_read8(data, F71882FG_REG_FAN_BEEP);
@@ -1206,7 +1214,14 @@ static ssize_t store_in_max(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	long val = simple_strtol(buf, NULL, 10) / 8;
+	int err;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 8;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1233,8 +1248,12 @@ static ssize_t store_in_beep(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	unsigned long val;
+
+	err = strict_strtoul(buf, 10, &val);
+	if (err)
+		return err;
 
 	mutex_lock(&data->update_lock);
 	data->in_beep = f71882fg_read8(data, F71882FG_REG_IN_BEEP);
@@ -1299,8 +1318,14 @@ static ssize_t store_temp_max(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 1000;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1333,10 +1358,16 @@ static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
 	ssize_t ret = count;
 	u8 reg;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 1000;
 
 	mutex_lock(&data->update_lock);
 
@@ -1372,8 +1403,14 @@ static ssize_t store_temp_crit(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 1000;
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1427,8 +1464,12 @@ static ssize_t store_temp_beep(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	unsigned long val;
+
+	err = strict_strtoul(buf, 10, &val);
+	if (err)
+		return err;
 
 	mutex_lock(&data->update_lock);
 	data->temp_beep = f71882fg_read8(data, F71882FG_REG_TEMP_BEEP);
@@ -1490,8 +1531,13 @@ static ssize_t store_pwm(struct device *dev,
 			 size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1551,8 +1597,12 @@ static ssize_t store_pwm_enable(struct device *dev, struct device_attribute
 				*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
 
 	/* Special case for F8000 pwm channel 3 which only does auto mode */
 	if (data->type == f8000 && nr == 2 && val != 2)
@@ -1626,9 +1676,14 @@ static ssize_t store_pwm_auto_point_pwm(struct device *dev,
 					const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int pwm = to_sensor_dev_attr_2(devattr)->index;
+	int err, pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10);
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
@@ -1674,10 +1729,16 @@ static ssize_t store_pwm_auto_point_temp_hyst(struct device *dev,
 					      const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
 	u8 reg;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 1000;
 
 	mutex_lock(&data->update_lock);
 	data->pwm_auto_point_temp[nr][point] =
@@ -1716,8 +1777,12 @@ static ssize_t store_pwm_interpolate(struct device *dev,
 				     const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	unsigned long val = simple_strtoul(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	unsigned long val;
+
+	err = strict_strtoul(buf, 10, &val);
+	if (err)
+		return err;
 
 	mutex_lock(&data->update_lock);
 	data->pwm_auto_point_mapping[nr] =
@@ -1752,8 +1817,12 @@ static ssize_t store_pwm_auto_point_channel(struct device *dev,
 					    const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int nr = to_sensor_dev_attr_2(devattr)->index;
-	long val = simple_strtol(buf, NULL, 10);
+	int err, nr = to_sensor_dev_attr_2(devattr)->index;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
 
 	switch (val) {
 	case 1:
@@ -1798,9 +1867,15 @@ static ssize_t store_pwm_auto_point_temp(struct device *dev,
 					 const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int pwm = to_sensor_dev_attr_2(devattr)->index;
+	int err, pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	long val = simple_strtol(buf, NULL, 10) / 1000;
+	long val;
+
+	err = strict_strtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	val /= 1000;
 
 	if (data->type == f71889fg)
 		val = SENSORS_LIMIT(val, -128, 127);
-- 
1.6.4.4


-- 
Met vriendelijke groet,
With kind regards,
Giel van Schijndel

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Jean Delvare]

Hi Giel,

On Mon, 22 Mar 2010 12:41:57 +0100, Giel van Schijndel wrote:
> On Mon, Mar 22, 2010 at 11:23:08AM +0100, Jean Delvare wrote:
> > On Sun, 21 Mar 2010 16:37:14 +0100, Giel van Schijndel wrote:
> >> Use the strict_strol and strict_stroul functions instead of simple_strol
> >> and simple_stroul respectively in sysfs functions.
> >> 
> >> Signed-off-by: Giel van Schijndel <me@mortis.eu>
> >> ---
> >>  drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
> >>  1 files changed, 74 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> >> index 21bc661..5c725a3 100644
> >> --- a/drivers/hwmon/f71882fg.c
> >> +++ b/drivers/hwmon/f71882fg.c
> >> @@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
> >>  {
> >>  	struct f71882fg_data *data = dev_get_drvdata(dev);
> >>  	int nr = to_sensor_dev_attr_2(devattr)->index;
> >> -	long val = simple_strtol(buf, NULL, 10);
> >> +	long val;
> >> +
> >> +	if (strict_strtol(buf, 10, &val) == -EINVAL)
> >> +		return -EINVAL;
> >
> > That's not correct. You want to return an error if strict_strtol()
> > returns _any_ error, not just -EINVAL. Maybe the current
> > implementation can't return any other error code, but you should not
> > assume this will always be the case in the future.
> 
> Agreed. New patch follows this line:
> ------------------------------------------------------------------------
> Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1
> 
> Use the strict_strol and strict_stroul functions instead of simple_strol
> and simple_stroul respectively in sysfs functions.
> 
> Signed-off-by: Giel van Schijndel <me@mortis.eu>
> ---
>  drivers/hwmon/f71882fg.c |  133 ++++++++++++++++++++++++++++++++++++----------
>  1 files changed, 104 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
> index 21bc661..4230729 100644
> --- a/drivers/hwmon/f71882fg.c
> +++ b/drivers/hwmon/f71882fg.c
> @@ -1127,8 +1127,12 @@ static ssize_t store_fan_full_speed(struct device *dev,
>  				    const char *buf, size_t count)
>  {
>  	struct f71882fg_data *data = dev_get_drvdata(dev);
> -	int nr = to_sensor_dev_attr_2(devattr)->index;
> -	long val = simple_strtol(buf, NULL, 10);
> +	int err, nr = to_sensor_dev_attr_2(devattr)->index;
> +	long val;
> +
> +	err = strict_strtol(buf, 10, &val);
> +	if (err)
> +		return err;
>  
>  	val = SENSORS_LIMIT(val, 23, 1500000);
>  	val = fan_to_reg(val);
> (...)

Looks good to me this time. I'll apply this patch unless Hans objects.

-- 
Jean Delvare

Re: [PATCH 2/2] Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1

[Hans de Goede]

Hi,

On 03/23/2010 11:59 AM, Jean Delvare wrote:
> Hi Giel,
>
> On Mon, 22 Mar 2010 12:41:57 +0100, Giel van Schijndel wrote:
>> On Mon, Mar 22, 2010 at 11:23:08AM +0100, Jean Delvare wrote:
>>> On Sun, 21 Mar 2010 16:37:14 +0100, Giel van Schijndel wrote:
>>>> Use the strict_strol and strict_stroul functions instead of simple_strol
>>>> and simple_stroul respectively in sysfs functions.
>>>>
>>>> Signed-off-by: Giel van Schijndel<me@mortis.eu>
>>>> ---
>>>>   drivers/hwmon/f71882fg.c |   89 ++++++++++++++++++++++++++++++++++++++--------
>>>>   1 files changed, 74 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
>>>> index 21bc661..5c725a3 100644
>>>> --- a/drivers/hwmon/f71882fg.c
>>>> +++ b/drivers/hwmon/f71882fg.c
>>>> @@ -1128,7 +1128,10 @@ static ssize_t store_fan_full_speed(struct device *dev,
>>>>   {
>>>>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>>>>   	int nr = to_sensor_dev_attr_2(devattr)->index;
>>>> -	long val = simple_strtol(buf, NULL, 10);
>>>> +	long val;
>>>> +
>>>> +	if (strict_strtol(buf, 10,&val) == -EINVAL)
>>>> +		return -EINVAL;
>>>
>>> That's not correct. You want to return an error if strict_strtol()
>>> returns _any_ error, not just -EINVAL. Maybe the current
>>> implementation can't return any other error code, but you should not
>>> assume this will always be the case in the future.
>>
>> Agreed. New patch follows this line:
>> ------------------------------------------------------------------------
>> Hwmon: f71882fg: use strict_stro(l|ul) instead of simple_strto$1
>>
>> Use the strict_strol and strict_stroul functions instead of simple_strol
>> and simple_stroul respectively in sysfs functions.
>>
>> Signed-off-by: Giel van Schijndel<me@mortis.eu>
>> ---
>>   drivers/hwmon/f71882fg.c |  133 ++++++++++++++++++++++++++++++++++++----------
>>   1 files changed, 104 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
>> index 21bc661..4230729 100644
>> --- a/drivers/hwmon/f71882fg.c
>> +++ b/drivers/hwmon/f71882fg.c
>> @@ -1127,8 +1127,12 @@ static ssize_t store_fan_full_speed(struct device *dev,
>>   				    const char *buf, size_t count)
>>   {
>>   	struct f71882fg_data *data = dev_get_drvdata(dev);
>> -	int nr = to_sensor_dev_attr_2(devattr)->index;
>> -	long val = simple_strtol(buf, NULL, 10);
>> +	int err, nr = to_sensor_dev_attr_2(devattr)->index;
>> +	long val;
>> +
>> +	err = strict_strtol(buf, 10,&val);
>> +	if (err)
>> +		return err;
>>
>>   	val = SENSORS_LIMIT(val, 23, 1500000);
>>   	val = fan_to_reg(val);
>> (...)
>
> Looks good to me this time. I'll apply this patch unless Hans objects.
>

No objections from me, IOW I'm still ack.

Regards,

Hans