public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Mierswa <impulze@impulze.org>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC] Re: Parsing kernel parameters and escaping "
Date: Mon, 13 Jul 2009 04:49:17 +0200	[thread overview]
Message-ID: <4A5AA0AD.2030502@impulze.org> (raw)
In-Reply-To: <200907130927.16203.rusty@rustcorp.com.au>

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

Rusty Russell wrote:
> It might be nice to have that test code somewhere at the bottom of param.c,
> at least while we're playing with the code.
Umm, I'm not sure where test-code is supposed to go in kernel code.
Should it be a main() function, a test() function, just a comment, could
you elaborate? All i did now was to build a small program that reads
argv[1] and uses next_arg just like parse_args() in params.c does.

> Well, IMO it's a maintainer's job to give feedback, and patches should always be welcomed (even if not applied!).
Ok, we'll see where this goes. So far I'm fine chatting about this with
everyone.

> I really prefer "i" instead of "iterator".  I actually think i as an
> unsigned/size_t here would probably make the code neater, but that's an aside.
Fixed.

> memmove?
Fixed.

> How about:
> 	if (strchr(delim, *iterator))
> 		return length;
I really should do more C. :-P Fixed.

> Note that this will undo another pending patch, which changes this to
> isspace() to handle tabs et al.
I will re-do the patch against that commit then once it's done.

> Thanks!
Ditto.


-- 
Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply
know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

[-- Attachment #2: kernel2.patch --]
[-- Type: text/plain, Size: 3797 bytes --]

>From 36d93d40b535ce7f1253d7740f214dc65b2975b9 Mon Sep 17 00:00:00 2001
From: Daniel Mierswa <impulze@impulze.org>
Date: Tue, 7 Jul 2009 00:54:38 +0200
Subject: [PATCH] Attempt to handle quotes in kernel parameters

There was a limitation for kernel parameters with regards to quoting. It
wasn't possible to escape quotes or use quotes to form space-filled
values _inside_ parameters. This patch attempts to make that possible,
kernel parameters are now parsed as follows:
'"param= value"' [param= value][]
'param=" value "" combination "' [param][ value  combination ]
'param=" \" test"' [param][ " test]
'"param"=another' [param][another]

Signed-off-by: Daniel Mierswa <impulze@impulze.org>
---
 kernel/params.c |  111 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index 7f6912c..5908623 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -72,58 +72,83 @@ static int parse_one(char *param,
 	return -ENOENT;
 }
 
-/* You can use " around spaces, but can't escape ". */
-/* Hyphens and underscores equivalent in parameter names. */
-static char *next_arg(char *args, char **param, char **val)
+/* modifies args with handled quotes
+ * [" foo bar "] => [ foo bar ]
+ * [" foo \" "]  => [ foo " ]
+ * [\"foo]       => [\"foo]
+ * [\"foo\" ]    => [\"foo" ]
+ */
+int printf(char const *,...);
+static size_t pull_token(char *args, char const *delim)
 {
-	unsigned int i, equals = 0;
-	int in_quote = 0, quoted = 0;
-	char *next;
-
-	if (*args == '"') {
-		args++;
-		in_quote = 1;
-		quoted = 1;
-	}
+	size_t i;
+	char *last_quote = NULL;
 
 	for (i = 0; args[i]; i++) {
-		if (args[i] == ' ' && !in_quote)
+		if (args[i] == '"') {
+			if (last_quote) {
+				size_t remain_len = strlen(args + i + 1);
+
+				/* remove both quotes by moving strings */
+				memmove(last_quote, last_quote + 1, args + i - 1 - last_quote);
+				memmove(args + i - 1, args + i + 1, remain_len + 1);
+
+				/* fill the blank we've left */
+				args[i + remain_len] = '\0';
+
+				/* removed 2 quotes, decrease the iterator  */
+				i -= 2;
+				last_quote = NULL;
+			} else {
+				last_quote = args + i;
+			}
+
+			continue;
+		}
+
+		if (last_quote) {
+			/* escaped quote */
+			if (args[i] == '\\' && args[i + 1] == '"') {
+				/* move all characters back */
+				memmove(args + i, args + i + 1, strlen(args + i + 1) + 1);
+			}
+
+			continue;
+		}
+
+		if (strchr(delim, args[i])) {
 			break;
-		if (equals == 0) {
-			if (args[i] == '=')
-				equals = i;
 		}
-		if (args[i] == '"')
-			in_quote = !in_quote;
 	}
 
+	return i;
+}
+
+static char *next_arg(char *args, char **param, char **val)
+{
+	size_t len;
+ 
+	/* Chew leading spaces */
+	while (*args == ' ')
+		args++;
+ 
+	len = pull_token(args, " \t\n=");
 	*param = args;
-	if (!equals)
+	args += len;
+
+	if (*args == '=') {
+		(*param)[len] = '\0';
+		args++;
+		len = pull_token(args, " \t\n");
+		*val = args;
+		args += len;
+		(*val)[len] = '\0';
+	} else {
+		(*param)[len] = '\0';
 		*val = NULL;
-	else {
-		args[equals] = '\0';
-		*val = args + equals + 1;
-
-		/* Don't include quotes in value. */
-		if (**val == '"') {
-			(*val)++;
-			if (args[i-1] == '"')
-				args[i-1] = '\0';
-		}
-		if (quoted && args[i-1] == '"')
-			args[i-1] = '\0';
 	}
 
-	if (args[i]) {
-		args[i] = '\0';
-		next = args + i + 1;
-	} else
-		next = args + i;
-
-	/* Chew up trailing spaces. */
-	while (*next == ' ')
-		next++;
-	return next;
+	return args + 1;
 }
 
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */
@@ -137,10 +162,6 @@ int parse_args(const char *name,
 
 	DEBUGP("Parsing ARGS: %s\n", args);
 
-	/* Chew leading spaces */
-	while (*args == ' ')
-		args++;
-
 	while (*args) {
 		int ret;
 		int irq_was_disabled;
-- 
1.6.3.3


  reply	other threads:[~2009-07-13  2:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-05 13:54 Parsing kernel parameters and escaping " Daniel Mierswa
2009-07-06 23:03 ` [RFC] " Daniel Mierswa
2009-07-06 23:05   ` Daniel Mierswa
2009-07-07  0:54   ` Daniel Mierswa
2009-07-12  9:41     ` Rusty Russell
2009-07-12 17:59       ` Daniel Mierswa
2009-07-12 23:57         ` Rusty Russell
2009-07-13  2:49           ` Daniel Mierswa [this message]
2009-07-14  2:49             ` Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A5AA0AD.2030502@impulze.org \
    --to=impulze@impulze.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox