Re: obsolete modparam change busted.

2005-08-15 Thread Dave Jones
On Tue, Aug 16, 2005 at 02:39:10PM +1000, Rusty Russell wrote:
 > On Sat, 2005-08-13 at 14:27 -0400, Dave Jones wrote:
 > > We're now munching the end of the boot command line it seems.
 > 
 > Wow, if we had testcases in the kernel source, I would not have to keep
 > rewriting them (badly) every time I touched this code.

Maybe someone should write a userspace test harness for kernel code. :-P

 > Throw away that stupid patch, apply this stupid patch.

I'll throw it into the Fedora kernel tomorrow, and re-test.
Thanks for chasing this so quickly.

Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-15 Thread Rusty Russell
On Sat, 2005-08-13 at 14:27 -0400, Dave Jones wrote:
> We're now munching the end of the boot command line it seems.

Wow, if we had testcases in the kernel source, I would not have to keep
rewriting them (badly) every time I touched this code.

Throw away that stupid patch, apply this stupid patch.


Name: Ignore trailing whitespace on kernel parameters correctly: Fixed version
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>

Dave Jones says:

... if the modprobe.conf has trailing whitespace, modules fail to load
with the following helpful message..

snd_intel8x0: Unknown parameter `'

Previous version truncated last argument.

Index: linux-2.6.13-rc6-git7-Module/kernel/params.c
===
--- linux-2.6.13-rc6-git7-Module.orig/kernel/params.c   2005-08-10 
16:12:45.0 +1000
+++ linux-2.6.13-rc6-git7-Module/kernel/params.c2005-08-16 
14:31:16.0 +1000
@@ -80,8 +80,6 @@
int in_quote = 0, quoted = 0;
char *next;
 
-   /* Chew any extra spaces */
-   while (*args == ' ') args++;
if (*args == '"') {
args++;
in_quote = 1;
@@ -121,6 +119,9 @@
next = args + i + 1;
} else
next = args + i;
+
+   /* Chew up trailing spaces. */
+   while (*next == ' ') next++;
return next;
 }
 
@@ -134,6 +135,9 @@
char *param, *val;
 
DEBUGP("Parsing ARGS: %s\n", args);
+   
+   /* Chew leading spaces */
+   while (*args == ' ') args++;
 
while (*args) {
int ret;

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-15 Thread Rusty Russell
On Sat, 2005-08-13 at 14:27 -0400, Dave Jones wrote:
 We're now munching the end of the boot command line it seems.

Wow, if we had testcases in the kernel source, I would not have to keep
rewriting them (badly) every time I touched this code.

Throw away that stupid patch, apply this stupid patch.


Name: Ignore trailing whitespace on kernel parameters correctly: Fixed version
Signed-off-by: Rusty Russell [EMAIL PROTECTED]

Dave Jones says:

... if the modprobe.conf has trailing whitespace, modules fail to load
with the following helpful message..

snd_intel8x0: Unknown parameter `'

Previous version truncated last argument.

Index: linux-2.6.13-rc6-git7-Module/kernel/params.c
===
--- linux-2.6.13-rc6-git7-Module.orig/kernel/params.c   2005-08-10 
16:12:45.0 +1000
+++ linux-2.6.13-rc6-git7-Module/kernel/params.c2005-08-16 
14:31:16.0 +1000
@@ -80,8 +80,6 @@
int in_quote = 0, quoted = 0;
char *next;
 
-   /* Chew any extra spaces */
-   while (*args == ' ') args++;
if (*args == '') {
args++;
in_quote = 1;
@@ -121,6 +119,9 @@
next = args + i + 1;
} else
next = args + i;
+
+   /* Chew up trailing spaces. */
+   while (*next == ' ') next++;
return next;
 }
 
@@ -134,6 +135,9 @@
char *param, *val;
 
DEBUGP(Parsing ARGS: %s\n, args);
+   
+   /* Chew leading spaces */
+   while (*args == ' ') args++;
 
while (*args) {
int ret;

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-15 Thread Dave Jones
On Tue, Aug 16, 2005 at 02:39:10PM +1000, Rusty Russell wrote:
  On Sat, 2005-08-13 at 14:27 -0400, Dave Jones wrote:
   We're now munching the end of the boot command line it seems.
  
  Wow, if we had testcases in the kernel source, I would not have to keep
  rewriting them (badly) every time I touched this code.

Maybe someone should write a userspace test harness for kernel code. :-P

  Throw away that stupid patch, apply this stupid patch.

I'll throw it into the Fedora kernel tomorrow, and re-test.
Thanks for chasing this so quickly.

Dave

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-13 Thread Dave Jones
On Tue, Aug 09, 2005 at 02:01:16PM +1000, Rusty Russell wrote:
 > On Mon, 2005-08-08 at 14:49 -0400, Dave Jones wrote:
 > > However this change was broken, and if the modprobe.conf
 > > has trailing whitespace, modules fail to load with the
 > > following helpful message..
 > 
 > Hi Dave,
 > 
 >  This fix should be preferable, I think.
 > 
 > Name: Ignore trailing whitespace on kernel parameters correctly
 > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>


Hey Rusty,
 This seems to have introduced a different regression :-)

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165633

We're now munching the end of the boot command line it seems.

Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-13 Thread Dave Jones
On Tue, Aug 09, 2005 at 02:01:16PM +1000, Rusty Russell wrote:
  On Mon, 2005-08-08 at 14:49 -0400, Dave Jones wrote:
   However this change was broken, and if the modprobe.conf
   has trailing whitespace, modules fail to load with the
   following helpful message..
  
  Hi Dave,
  
   This fix should be preferable, I think.
  
  Name: Ignore trailing whitespace on kernel parameters correctly
  Signed-off-by: Rusty Russell [EMAIL PROTECTED]


Hey Rusty,
 This seems to have introduced a different regression :-)

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165633

We're now munching the end of the boot command line it seems.

Dave

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-08 Thread Rusty Russell
On Mon, 2005-08-08 at 14:49 -0400, Dave Jones wrote:
> However this change was broken, and if the modprobe.conf
> has trailing whitespace, modules fail to load with the
> following helpful message..

Hi Dave,

This fix should be preferable, I think.

Name: Ignore trailing whitespace on kernel parameters correctly
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>

Dave Jones says:

... if the modprobe.conf has trailing whitespace, modules fail to load
with the following helpful message..

snd_intel8x0: Unknown parameter `'

--- linux-2.6.12/kernel/params.c2005-07-15 04:39:53.0 +1000
+++ /tmp/foo.c  2005-08-09 13:56:04.0 +1000
@@ -1,0 +1,0 @@
@@ -135,11 +80,8 @@
 
DEBUGP("Parsing ARGS: %s\n", args);
 
-   while (*args) {
-   int ret;
-
-   args = next_arg(args, , );
-   ret = parse_one(param, val, params, num, unknown);
+   while (*(args = next_arg(args, , ))) {
+   int ret = parse_one(param, val, params, num, unknown);
switch (ret) {
case -ENOENT:
printk(KERN_ERR "%s: Unknown parameter `%s'\n",

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


obsolete modparam change busted.

2005-08-08 Thread Dave Jones
Circa 2.6.10, the module loader started barfing if
modprobe.conf contained obsolete parameters.

However this change was broken, and if the modprobe.conf
has trailing whitespace, modules fail to load with the
following helpful message..

snd_intel8x0: Unknown parameter `'

This ends up screwing over people who upgrade from previously
working configurations, so we've been backing out that change
in Fedora for a while with the patch below.

It doesn't look like the right thing to do, but it has got
things working again at least. Probably we should just check
explicity for whitespace and ignore it somewhere else in
the module loader.  Rusty?

Dave


diff -urNp --exclude-from=/home/davej/.exclude linux-1503/kernel/module.c 
linux-1700/kernel/module.c
--- linux-1503/kernel/module.c
+++ linux-1700/kernel/module.c
@@ -1707,8 +1707,6 @@ static struct module *load_module(void _
 / sizeof(struct kernel_param),
 NULL);
}
-   if (err < 0)
-   goto arch_cleanup;
 
err = mod_sysfs_setup(mod, 
  (struct kernel_param *)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


obsolete modparam change busted.

2005-08-08 Thread Dave Jones
Circa 2.6.10, the module loader started barfing if
modprobe.conf contained obsolete parameters.

However this change was broken, and if the modprobe.conf
has trailing whitespace, modules fail to load with the
following helpful message..

snd_intel8x0: Unknown parameter `'

This ends up screwing over people who upgrade from previously
working configurations, so we've been backing out that change
in Fedora for a while with the patch below.

It doesn't look like the right thing to do, but it has got
things working again at least. Probably we should just check
explicity for whitespace and ignore it somewhere else in
the module loader.  Rusty?

Dave


diff -urNp --exclude-from=/home/davej/.exclude linux-1503/kernel/module.c 
linux-1700/kernel/module.c
--- linux-1503/kernel/module.c
+++ linux-1700/kernel/module.c
@@ -1707,8 +1707,6 @@ static struct module *load_module(void _
 / sizeof(struct kernel_param),
 NULL);
}
-   if (err  0)
-   goto arch_cleanup;
 
err = mod_sysfs_setup(mod, 
  (struct kernel_param *)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: obsolete modparam change busted.

2005-08-08 Thread Rusty Russell
On Mon, 2005-08-08 at 14:49 -0400, Dave Jones wrote:
 However this change was broken, and if the modprobe.conf
 has trailing whitespace, modules fail to load with the
 following helpful message..

Hi Dave,

This fix should be preferable, I think.

Name: Ignore trailing whitespace on kernel parameters correctly
Signed-off-by: Rusty Russell [EMAIL PROTECTED]

Dave Jones says:

... if the modprobe.conf has trailing whitespace, modules fail to load
with the following helpful message..

snd_intel8x0: Unknown parameter `'

--- linux-2.6.12/kernel/params.c2005-07-15 04:39:53.0 +1000
+++ /tmp/foo.c  2005-08-09 13:56:04.0 +1000
@@ -1,0 +1,0 @@
@@ -135,11 +80,8 @@
 
DEBUGP(Parsing ARGS: %s\n, args);
 
-   while (*args) {
-   int ret;
-
-   args = next_arg(args, param, val);
-   ret = parse_one(param, val, params, num, unknown);
+   while (*(args = next_arg(args, param, val))) {
+   int ret = parse_one(param, val, params, num, unknown);
switch (ret) {
case -ENOENT:
printk(KERN_ERR %s: Unknown parameter `%s'\n,

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/