Neverending module_param() bugs

Neverending module_param() bugs

[Alexey Dobriyan]

Can someone think of a way to explicitly tell driver author that last
argument of module_param is PERMISSIONS, not default value? It's late
here I can't. Preferably resulting in compilation failure.

drivers/acpi/sbs.c:101:module_param(capacity_mode, int, CAPACITY_UNIT);
drivers/acpi/sbs.c:102:module_param(update_mode, int, UPDATE_MODE);
drivers/acpi/sbs.c:103:module_param(update_info_mode, int, UPDATE_INFO_MODE);
drivers/acpi/sbs.c:104:module_param(update_time, int, UPDATE_TIME);
drivers/acpi/sbs.c:105:module_param(update_time2, int, UPDATE_TIME2);
drivers/char/watchdog/sbc8360.c:203:module_param(timeout, int, 27);

P.S.: drivers/media/video/tuner-simple.c:13:module_param(offset, int, 0666);
								      ^^^^

Re: Neverending module_param() bugs

[Mauro Carvalho Chehab]

Alexey,

Em Dom, 2006-08-13 às 01:47 +0400, Alexey Dobriyan escreveu:
> P.S.: drivers/media/video/tuner-simple.c:13:module_param(offset, int,
> 0666);

Good catch. We should change it to 0x664. I'll prepare such patch.

Anyway, this is not dangerous, since it just allows an offset adjustment
at tuning frequency of a TV capture board.

Cheers, 
Mauro.

Re: Neverending module_param() bugs

[Jan-Benedict Glaw]

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

On Sat, 2006-08-12 21:54:01 -0300, Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
> Em Dom, 2006-08-13 às 01:47 +0400, Alexey Dobriyan escreveu:
> > P.S.: drivers/media/video/tuner-simple.c:13:module_param(offset, int,
> > 0666);
> 
> Good catch. We should change it to 0x664. I'll prepare such patch.

But keep in mind it's really octal, not hex.

MfG, JBG

-- 
      Jan-Benedict Glaw      jbglaw@lug-owl.de              +49-172-7608481
Signature of:                   ...und wenn Du denkst, es geht nicht mehr,
the second  :                          kommt irgendwo ein Lichtlein her.

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

Re: Neverending module_param() bugs

[Mauro Carvalho Chehab]

Em Dom, 2006-08-13 às 04:19 +0200, Jan-Benedict Glaw escreveu:
> On Sat, 2006-08-12 21:54:01 -0300, Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
> > Em Dom, 2006-08-13 às 01:47 +0400, Alexey Dobriyan escreveu:
> > > P.S.: drivers/media/video/tuner-simple.c:13:module_param(offset, int,
> > > 0666);
> > 
> > Good catch. We should change it to 0x664. I'll prepare such patch.
> 
> But keep in mind it's really octal, not hex.
Ah, sorry for the bad representation :)
> 
> MfG, JBG
> 
Cheers, 
Mauro.

[PATCH][SBC8360] Re: Neverending module_param() bugs

[Ian E. Morgan]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1544 bytes --]

On Sun, 13 Aug 2006, Alexey Dobriyan wrote:

> Can someone think of a way to explicitly tell driver author that last
> argument of module_param is PERMISSIONS, not default value? It's late
> here I can't. Preferably resulting in compilation failure.
> 
> drivers/char/watchdog/sbc8360.c:203:module_param(timeout, int, 27);

Here's my fix for sbc8360, inlined and attached. Please merge.

Regards,
Ian Morgan

-- 
-------------------------------------------------------------------
 Ian E. Morgan          Vice President & C.O.O.       Webcon, Inc.
 imorgan at webcon dot ca       PGP: #2DA40D07       www.webcon.ca
    *  Customized Linux Network Solutions for your Business  *
-------------------------------------------------------------------

--- linux-2.6.17.11/drivers/char/watchdog/sbc8360.c.orig	2006-08-29 12:55:26.000000000 -0400
+++ linux-2.6.17.11/drivers/char/watchdog/sbc8360.c	2006-08-29 12:58:20.000000000 -0400
@@ -201,7 +201,7 @@ static int wd_margin = 0xB;
 static int wd_multiplier = 2;
 static int nowayout = WATCHDOG_NOWAYOUT;
 
-module_param(timeout, int, 27);
+module_param(timeout, int, 0);
 MODULE_PARM_DESC(timeout, "Index into timeout table (0-63) (default=27 (60s))");
 module_param(nowayout, int, 0);
 MODULE_PARM_DESC(nowayout,
@@ -408,7 +408,7 @@ module_exit(sbc8360_exit);
 MODULE_AUTHOR("Ian E. Morgan <imorgan@webcon.ca>");
 MODULE_DESCRIPTION("SBC8360 watchdog driver");
 MODULE_LICENSE("GPL");
-MODULE_VERSION("1.0");
+MODULE_VERSION("1.01");
 MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
 
 /* end of sbc8360.c */

[-- Attachment #2: Type: TEXT/PLAIN, Size: 818 bytes --]

--- linux-2.6.17.11/drivers/char/watchdog/sbc8360.c.orig	2006-08-29 12:55:26.000000000 -0400
+++ linux-2.6.17.11/drivers/char/watchdog/sbc8360.c	2006-08-29 12:58:20.000000000 -0400
@@ -201,7 +201,7 @@ static int wd_margin = 0xB;
 static int wd_multiplier = 2;
 static int nowayout = WATCHDOG_NOWAYOUT;
 
-module_param(timeout, int, 27);
+module_param(timeout, int, 0);
 MODULE_PARM_DESC(timeout, "Index into timeout table (0-63) (default=27 (60s))");
 module_param(nowayout, int, 0);
 MODULE_PARM_DESC(nowayout,
@@ -408,7 +408,7 @@ module_exit(sbc8360_exit);
 MODULE_AUTHOR("Ian E. Morgan <imorgan@webcon.ca>");
 MODULE_DESCRIPTION("SBC8360 watchdog driver");
 MODULE_LICENSE("GPL");
-MODULE_VERSION("1.0");
+MODULE_VERSION("1.01");
 MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
 
 /* end of sbc8360.c */