Re: [lm-sensors] PATCH:

[Jean Delvare <khali@linux-fr.org>]

Hi Hans,

On Mon, 24 Sep 2007 16:28:16 +0200, Jean Delvare wrote:
> Hi Hans,
> 
> On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
> > Here is a patch adding some text to the sysfs interface documentation on how
> > settings written to sysfs attributes should be handled, focussing mainly on
> > error handling. This version incorperates Jean's latest comments.
> > 
> > signed-off-by: j.w.r.degoede@hhs.nl
> 
> Thanks for doing this.
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>

While reviewing your new fschmd driver, I found some mistakes which
also appear in the code examples of this documentation update:

> +--- begin code ---
> +long v = simple_strtol(buf, NULL, 10) / 1000;
> +SENSORS_LIMIT(v, -128, 127);

This is a no-op, should be v = ...

> +/* write v to register */
> +--- end code ---

> +--- begin code ---
> +unsigned long v = simple_strtoul(buf, NULL, 10);
> +
> +switch (v) {
> +       case 2: v = 1; break;
> +       case 4: v = 2; break;
> +       case 8: v = 3; break;
> +       default:
> +               return -EINVAL;
> +}

Indentation doesn't comply with CodingStyle.

> +/* write v to register */
> +--- end code ---

Additionally, I think that the examples are a bit difficult to read,
one level of indentation would probably be better than the begin/end
code markers. Thus, I propose the following incremental patch:

 Documentation/hwmon/sysfs-interface |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

--- linux-2.6.23-rc9.orig/Documentation/hwmon/sysfs-interface	2007-10-07 10:12:33.000000000 +0200
+++ linux-2.6.23-rc9/Documentation/hwmon/sysfs-interface	2007-10-07 10:13:43.000000000 +0200
@@ -450,22 +450,20 @@ continuous like for example a tempX_type
 written, -EINVAL should be returned.
 
 Example1, temp1_max, register is a signed 8 bit value (-128 - 127 degrees):
---- begin code ---
-long v = simple_strtol(buf, NULL, 10) / 1000;
-SENSORS_LIMIT(v, -128, 127);
-/* write v to register */
---- end code ---
+
+	long v = simple_strtol(buf, NULL, 10) / 1000;
+	v = SENSORS_LIMIT(v, -128, 127);
+	/* write v to register */
 
 Example2, fan divider setting, valid values 2, 4 and 8:
---- begin code ---
-unsigned long v = simple_strtoul(buf, NULL, 10);
 
-switch (v) {
-       case 2: v = 1; break;
-       case 4: v = 2; break;
-       case 8: v = 3; break;
-       default:
-               return -EINVAL;
-}
-/* write v to register */
---- end code ---
+	unsigned long v = simple_strtoul(buf, NULL, 10);
+
+	switch (v) {
+	case 2: v = 1; break;
+	case 4: v = 2; break;
+	case 8: v = 3; break;
+	default:
+		return -EINVAL;
+	}
+	/* write v to register */


-- 
Jean Delvare

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