[PATCH] handle quoted module parameters

[PATCH] handle quoted module parameters

[Randy.Dunlap]

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

Here's a patch with better description.


Fix module parameter quote handling.
Module parameter strings (with spaces) are quoted like so:
"modprm=this test"
and not like this:
modprm="this test"

Signed-off-by: Randy Dunlap <rddunlap@osdl.org>

diffstat:=
  kernel/params.c |   15 ++++++++++++---
  1 files changed, 12 insertions(+), 3 deletions(-)


-- 
~Randy

[-- Attachment #2: modprm_quoted.patch --]
[-- Type: text/x-patch, Size: 991 bytes --]


diff -Naurp ./kernel/params.c~modprm_quoted ./kernel/params.c
--- ./kernel/params.c~modprm_quoted	2004-11-11 15:13:26.000000000 -0800
+++ ./kernel/params.c	2004-11-11 19:42:36.714124784 -0800
@@ -77,10 +77,16 @@ static int parse_one(char *param,
 static char *next_arg(char *args, char **param, char **val)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0;
+	int in_quote = 0, quoted = 0;
+	char *next;
 
 	/* Chew any extra spaces */
 	while (*args == ' ') args++;
+	if (*args == '"') {
+		args++;
+		in_quote = 1;
+		quoted = 1;
+	}
 
 	for (i = 0; args[i]; i++) {
 		if (args[i] == ' ' && !in_quote)
@@ -106,13 +112,16 @@ static char *next_arg(char *args, char *
 			if (args[i-1] == '"')
 				args[i-1] = '\0';
 		}
+		if (quoted && args[i-1] == '"')
+			args[i-1] = '\0';
 	}
 
 	if (args[i]) {
 		args[i] = '\0';
-		return args + i + 1;
+		next = args + i + 1;
 	} else
-		return args + i;
+		next = args + i;
+	return next;
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */

RE: [PATCH] handle quoted module parameters

[yiding_wang]

Hello Randy,

Thanks for your two responses!

Based on your patch, the format of argument will be changed from standard format before:
Used to be:
modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.

Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?

Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Regards,

Eddie


-----Original Message-----
From: Randy.Dunlap [mailto:rddunlap@osdl.org]
Sent: Thursday, November 11, 2004 8:17 PM
To: Randy.Dunlap
Cc: yiding_wang@agilent.com; arjan@infradead.org;
linux-kernel@vger.kernel.org; rusty@rustcorp.com.au; akpm
Subject: [PATCH] handle quoted module parameters


Here's a patch with better description.


Fix module parameter quote handling.
Module parameter strings (with spaces) are quoted like so:
"modprm=this test"
and not like this:
modprm="this test"

Signed-off-by: Randy Dunlap <rddunlap@osdl.org>

diffstat:=
  kernel/params.c |   15 ++++++++++++---
  1 files changed, 12 insertions(+), 3 deletions(-)


-- 
~Randy

RE: [PATCH] handle quoted module parameters

[Adam Heath]

On Fri, 12 Nov 2004,  wrote:

> Hello Randy,
>
> Thanks for your two responses!
>
> Based on your patch, the format of argument will be changed from standard format before:
> Used to be:
> modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
> where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.
>
> Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?
>
> Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Er, no, that's a wrong assumption.

Quoting like this is handled by the shell.  It tells it how to parse the
single cmdline string, into separate parts.

There is *no* difference between:

foo="111 222 333"\ 444' 555'

and

foo='111 222 333 444 555'

and

foo="111 222 333 444 555'

RE: [PATCH] handle quoted module parameters

[yiding_wang]

>There is *no* difference between:
>foo="111 222 333"\ 444' 555'
>and
>foo='111 222 333 444 555'
>and
>foo="111 222 333 444 555'

But there is a difference between foo="111 222 333" and "foo=111 222 333". The new patch is changing from former to later.

Eddie


-----Original Message-----
From: Adam Heath [mailto:doogie@debian.org]
Sent: Friday, November 12, 2004 11:41 AM
To: yiding_wang@agilent.com
Cc: rddunlap@osdl.org; arjan@infradead.org;
linux-kernel@vger.kernel.org; rusty@rustcorp.com.au; akpm@osdl.org
Subject: RE: [PATCH] handle quoted module parameters


On Fri, 12 Nov 2004,  wrote:

> Hello Randy,
>
> Thanks for your two responses!
>
> Based on your patch, the format of argument will be changed from standard format before:
> Used to be:
> modprm1=first,ext modprm2=second,ext modprm3="third1,ext third2,ext"
> where the quotation in modprm3 represents the whole string, including space, to be the value of third parameter modprm3.
>
> Now the patch changes modprm3 to "modprm3=third1,ext third2,ext" which equivalent to putting quotation mark on normal parameter define "modprm1=first,ext". Do you think linux community will take that change?
>
> Another question is the parameter length is not limited in 2.4.x kernel. Why this is restricted under 2.6.x. (param_set_charp())?

Er, no, that's a wrong assumption.

Quoting like this is handled by the shell.  It tells it how to parse the
single cmdline string, into separate parts.

There is *no* difference between:

foo="111 222 333"\ 444' 555'

and

foo='111 222 333 444 555'

and

foo="111 222 333 444 555'

Re: [PATCH] handle quoted module parameters

[Rusty Russell]

On Sat, 2004-11-13 at 14:22 -0800, Randy.Dunlap wrote:
> yiding_wang@agilent.com wrote:
> >>There is *no* difference between:
> >>foo="111 222 333"\ 444' 555'
> >>and
> >>foo='111 222 333 444 555'
> >>and
> >>foo="111 222 333 444 555'
> > 
> > 
> > But there is a difference between foo="111 222 333" and "foo=111 222 333". The new patch is changing from former to later.
> 
> Actually the patch allows (or _should allow_) either format for quote
> marks.  I didn't remove the older code, just added support for the
> case of quote marks as "foo=this is a test".

Yes, I have no fundamental problem with the patch, but it'd need
thorough testing (eg. with __setup) since this area has broken before.

> Why is the module param length limit of 1024 a problem?

The 1024 test is there because we want to limit how much we output
through sysfs.  We could up it to PAGE_SIZE-1.

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

Re: [PATCH] handle quoted module parameters

[Rusty Russell]

On Thu, 2004-11-11 at 20:16 -0800, Randy.Dunlap wrote: 
> Here's a patch with better description.
> 
> 
> Fix module parameter quote handling.
> Module parameter strings (with spaces) are quoted like so:
> "modprm=this test"
> and not like this:
> modprm="this test"

Well, the quote handling in insmod was ripped out after 3.0, exactly
because it was broken like this.  But modprobe will use the latter form,
since it will paste it straight from the modprobe.conf file (which needs
quotes in options lines).

Hope that clarifies,
Rusty.
PS. module-init-tools 3.1 just out...
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

RE: [PATCH] handle quoted module parameters

[yiding_wang]

Hello Rusty,

The broken part I encountered is from the latest module-init-tools 3.1. Is that possible to restore the allowable parameter length as it for 2.4.x, at least increase it from 1K to 4K?

Regards,

Eddie

-----Original Message-----
From: Rusty Russell [mailto:rusty@rustcorp.com.au]
Sent: Sunday, November 14, 2004 5:15 PM
To: Randy.Dunlap
Cc: yiding_wang@agilent.com; arjan@infradead.org; lkml - Kernel Mailing
List; Andrew Morton
Subject: Re: [PATCH] handle quoted module parameters


On Thu, 2004-11-11 at 20:16 -0800, Randy.Dunlap wrote: 
> Here's a patch with better description.
> 
> 
> Fix module parameter quote handling.
> Module parameter strings (with spaces) are quoted like so:
> "modprm=this test"
> and not like this:
> modprm="this test"

Well, the quote handling in insmod was ripped out after 3.0, exactly
because it was broken like this.  But modprobe will use the latter form,
since it will paste it straight from the modprobe.conf file (which needs
quotes in options lines).

Hope that clarifies,
Rusty.
PS. module-init-tools 3.1 just out...
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman