linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
@ 2018-06-05 16:43 Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 1/5] lib/cmdline.c: Add backslash support to kernel commandline parsing Michal Suchanek
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

Hello,

due to work on the fadump_extra_args I looked at the kernel parameter parser
and found its grammar rather curious.

It supports double quotes but not any other quoting characters so double quotes
cannot be quoted. What's more, the quotes can be anywhere in the parameter
name or value and are interpteted but are removed only from start and end of
the parameter value.

These are the patches not specific to fadump which somewhat straighten the
qouting grammar to make it on par with common shell interpreters.

Specifically double and single quotes can be used for quoting as well as
backslashes with the usual shell semantic. All quoting characters are removed
while the parameters are parsed.

Previous versions (including the fadump part) can be found here:

https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg126148.html
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg123639.html

Thanks

Michal

Michal Suchanek (5):
  lib/cmdline.c: Add backslash support to kernel commandline parsing.
  Documentation/admin-guide: backslash support in kernel arguments.
  init/main.c: simplify repair_env_string.
  lib/cmdline.c: Implement single quotes in commandline argument
    parsing.
  Documentation/admin-guide: single quotes in kernel arguments.

 Documentation/admin-guide/kernel-parameters.rst |  5 +-
 init/main.c                                     | 13 ++---
 lib/cmdline.c                                   | 63 +++++++++++++++----------
 3 files changed, 46 insertions(+), 35 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v10 1/5] lib/cmdline.c: Add backslash support to kernel commandline parsing.
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
@ 2018-06-05 16:43 ` Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 2/5] Documentation/admin-guide: backslash support in kernel arguments Michal Suchanek
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

 - add \ as quoting character in kernel parameters
 - generalize quoting character removal to be able to remove quoting
 character at any position in kernel parameter rather than start and end
 only

This allows passing quotes in kernel arguments.

It is also useful to have quoting grammar more similar to shells and
bootloaders.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
- more verbose commit message
- optimize final quote removal
---
 lib/cmdline.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 192848d880ea..aa0086dbb082 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -191,32 +191,43 @@ bool parse_option_str(const char *str, const char *option)
 	return false;
 }
 
+#define break_arg_end(i) { \
+	if (isspace(args[i]) && !in_quote && !backslash) \
+		break; \
+	}
+
 /*
  * Parse a string to get a param value pair.
- * You can use " around spaces, but can't escape ".
+ * You can use " around spaces, and you can escape with \
  * Hyphens and underscores equivalent in parameter names.
  */
 char *next_arg(char *args, char **param, char **val, int *state_flags)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0, quoted = 0;
+	int in_quote = 0, backslash = 0;
 	char *next;
 
-	if (*args == '"') {
-		args++;
-		in_quote = 1;
-		quoted = 1;
-	}
-
 	for (i = 0; args[i]; i++) {
-		if (isspace(args[i]) && !in_quote)
-			break;
-		if (equals == 0) {
-			if (args[i] == '=')
-				equals = i;
+		break_arg_end(i);
+
+		if ((equals == 0) && (args[i] == '='))
+			equals = i;
+
+		if (!backslash) {
+			if ((args[i] == '"') || (args[i] == '\\')) {
+				if (args[i] == '"')
+					in_quote = !in_quote;
+				if (args[i] == '\\')
+					backslash = 1;
+
+				break_arg_end(i + 1);
+				memmove(args + 1, args, i);
+				args++;
+				i--;
+			}
+		} else {
+			backslash = 0;
 		}
-		if (args[i] == '"')
-			in_quote = !in_quote;
 	}
 
 	if (state_flags)
@@ -231,16 +242,7 @@ char *next_arg(char *args, char **param, char **val, int *state_flags)
 	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';
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 2/5] Documentation/admin-guide: backslash support in kernel arguments.
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 1/5] lib/cmdline.c: Add backslash support to kernel commandline parsing Michal Suchanek
@ 2018-06-05 16:43 ` Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 3/5] init/main.c: simplify repair_env_string Michal Suchanek
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 Documentation/admin-guide/kernel-parameters.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index b8d0bc07ed0a..327e76ebe37d 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,9 @@ can also be entered as::
 
 	log-buf-len=1M print_fatal_signals=1
 
-Double-quotes can be used to protect spaces in values, e.g.::
+Double-quotes and backslashes can be used to protect spaces in values, e.g.::
 
-	param="spaces in here"
+	param="spaces in here" param2=spaces\ in\ here
 
 cpu lists:
 ----------
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 3/5] init/main.c: simplify repair_env_string.
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 1/5] lib/cmdline.c: Add backslash support to kernel commandline parsing Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 2/5] Documentation/admin-guide: backslash support in kernel arguments Michal Suchanek
@ 2018-06-05 16:43 ` Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 4/5] lib/cmdline.c: Implement single quotes in commandline argument parsing Michal Suchanek
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

Quoting characters are now removed from the parameter so value always
follows directly after the NUL terminating parameter name.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 init/main.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/init/main.c b/init/main.c
index 4bc2819d97ad..d0f4a7a0f010 100644
--- a/init/main.c
+++ b/init/main.c
@@ -251,15 +251,10 @@ static int __init repair_env_string(char *param, char *val,
 				    const char *unused, void *arg)
 {
 	if (val) {
-		/* param=val or param="val"? */
-		if (val == param+strlen(param)+1)
-			val[-1] = '=';
-		else if (val == param+strlen(param)+2) {
-			val[-2] = '=';
-			memmove(val-1, val, strlen(val)+1);
-			val--;
-		} else
-			BUG();
+		int parm_len = strlen(param);
+
+		param[parm_len] = '=';
+		BUG_ON(val != param + parm_len + 1);
 	}
 	return 0;
 }
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 4/5] lib/cmdline.c: Implement single quotes in commandline argument parsing.
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
                   ` (2 preceding siblings ...)
  2018-06-05 16:43 ` [PATCH v10 3/5] init/main.c: simplify repair_env_string Michal Suchanek
@ 2018-06-05 16:43 ` Michal Suchanek
  2018-06-05 16:43 ` [PATCH v10 5/5] Documentation/admin-guide: single quotes in kernel arguments Michal Suchanek
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

This brings the kernel parser about on par with bourne shell, grub, and
other tools that chew the arguments before kernel does.

This should make it easier to deal with multiple levels of
nesting/quoting. With same quoting grammar on each level there is less
room for confusion.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
- optimize end quote removal
---
 lib/cmdline.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index aa0086dbb082..e699ed5aac8a 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -192,19 +192,26 @@ bool parse_option_str(const char *str, const char *option)
 }
 
 #define break_arg_end(i) { \
-	if (isspace(args[i]) && !in_quote && !backslash) \
+	if (isspace(args[i]) && !in_quote && !backslash && !in_single) \
 		break; \
 	}
 
+#define squash_char { \
+	break_arg_end(i + 1); \
+	memmove(args + 1, args, i); \
+	args++; \
+	i--; \
+}
+
 /*
  * Parse a string to get a param value pair.
- * You can use " around spaces, and you can escape with \
+ * You can use " or ' around spaces, and you can escape with \
  * Hyphens and underscores equivalent in parameter names.
  */
 char *next_arg(char *args, char **param, char **val, int *state_flags)
 {
 	unsigned int i, equals = 0;
-	int in_quote = 0, backslash = 0;
+	int in_quote = 0, backslash = 0, in_single = 0;
 	char *next;
 
 	for (i = 0; args[i]; i++) {
@@ -213,17 +220,23 @@ char *next_arg(char *args, char **param, char **val, int *state_flags)
 		if ((equals == 0) && (args[i] == '='))
 			equals = i;
 
-		if (!backslash) {
-			if ((args[i] == '"') || (args[i] == '\\')) {
+		if (in_single) {
+			if (args[i] == '\'') {
+				in_single = 0;
+
+				squash_char;
+			}
+		} else if (!backslash) {
+			if ((args[i] == '"') || (args[i] == '\\') ||
+					(args[i] == '\'')) {
 				if (args[i] == '"')
 					in_quote = !in_quote;
 				if (args[i] == '\\')
 					backslash = 1;
+				if (args[i] == '\'')
+					in_single = 1;
 
-				break_arg_end(i + 1);
-				memmove(args + 1, args, i);
-				args++;
-				i--;
+				squash_char;
 			}
 		} else {
 			backslash = 0;
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 5/5] Documentation/admin-guide: single quotes in kernel arguments.
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
                   ` (3 preceding siblings ...)
  2018-06-05 16:43 ` [PATCH v10 4/5] lib/cmdline.c: Implement single quotes in commandline argument parsing Michal Suchanek
@ 2018-06-05 16:43 ` Michal Suchanek
  2018-06-05 17:05 ` [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Andy Shevchenko
  2018-06-19 23:36 ` Andrew Morton
  6 siblings, 0 replies; 10+ messages in thread
From: Michal Suchanek @ 2018-06-05 16:43 UTC (permalink / raw)
  To: Jonathan Corbet, Michal Suchanek, Arnd Bergmann,
	Frederic Weisbecker, Ingo Molnar, Aaron Wu, Tony Luck,
	Andrew Morton, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
- fix typo
---
 Documentation/admin-guide/kernel-parameters.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 327e76ebe37d..cc81ad7cab0c 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -35,9 +35,10 @@ can also be entered as::
 
 	log-buf-len=1M print_fatal_signals=1
 
-Double-quotes and backslashes can be used to protect spaces in values, e.g.::
+Double-quotes, single-quotes and backslashes can be used to protect spaces
+in values, e.g.::
 
-	param="spaces in here" param2=spaces\ in\ here
+	param="spaces in here" param2=spaces\ in\ here param3='@%# !\'
 
 cpu lists:
 ----------
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
                   ` (4 preceding siblings ...)
  2018-06-05 16:43 ` [PATCH v10 5/5] Documentation/admin-guide: single quotes in kernel arguments Michal Suchanek
@ 2018-06-05 17:05 ` Andy Shevchenko
  2018-06-05 17:25   ` Michal Suchánek
  2018-06-19 23:36 ` Andrew Morton
  6 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2018-06-05 17:05 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Jonathan Corbet, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
	Aaron Wu, Tony Luck, Andrew Morton, Thomas Gleixner,
	Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov,
	Linux Documentation List, Linux Kernel Mailing List

On Tue, Jun 5, 2018 at 7:43 PM, Michal Suchanek <msuchanek@suse.de> wrote:
> Hello,
>
> due to work on the fadump_extra_args I looked at the kernel parameter parser
> and found its grammar rather curious.
>
> It supports double quotes but not any other quoting characters so double quotes
> cannot be quoted. What's more, the quotes can be anywhere in the parameter
> name or value and are interpteted but are removed only from start and end of
> the parameter value.
>
> These are the patches not specific to fadump which somewhat straighten the
> qouting grammar to make it on par with common shell interpreters.

I didn't notice any use of string_unescape_*() functionality. So, your
patch is kinda very specific to some narrow subset of escaping and
unescaping stuff.
Thus, it's still not on par with shell, right?

>
> Specifically double and single quotes can be used for quoting as well as
> backslashes with the usual shell semantic. All quoting characters are removed
> while the parameters are parsed.
>
> Previous versions (including the fadump part) can be found here:
>
> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg126148.html
> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg123639.html
>
> Thanks
>
> Michal
>
> Michal Suchanek (5):
>   lib/cmdline.c: Add backslash support to kernel commandline parsing.
>   Documentation/admin-guide: backslash support in kernel arguments.
>   init/main.c: simplify repair_env_string.
>   lib/cmdline.c: Implement single quotes in commandline argument
>     parsing.
>   Documentation/admin-guide: single quotes in kernel arguments.
>
>  Documentation/admin-guide/kernel-parameters.rst |  5 +-
>  init/main.c                                     | 13 ++---
>  lib/cmdline.c                                   | 63 +++++++++++++++----------
>  3 files changed, 46 insertions(+), 35 deletions(-)
>
> --
> 2.13.6
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
  2018-06-05 17:05 ` [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Andy Shevchenko
@ 2018-06-05 17:25   ` Michal Suchánek
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Suchánek @ 2018-06-05 17:25 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Corbet, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
	Aaron Wu, Tony Luck, Andrew Morton, Thomas Gleixner,
	Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov,
	Linux Documentation List, Linux Kernel Mailing List

On Tue, 5 Jun 2018 20:05:50 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Jun 5, 2018 at 7:43 PM, Michal Suchanek <msuchanek@suse.de>
> wrote:
> > Hello,
> >
> > due to work on the fadump_extra_args I looked at the kernel
> > parameter parser and found its grammar rather curious.
> >
> > It supports double quotes but not any other quoting characters so
> > double quotes cannot be quoted. What's more, the quotes can be
> > anywhere in the parameter name or value and are interpteted but are
> > removed only from start and end of the parameter value.
> >
> > These are the patches not specific to fadump which somewhat
> > straighten the qouting grammar to make it on par with common shell
> > interpreters.  
> 
> I didn't notice any use of string_unescape_*() functionality. So, your
> patch is kinda very specific to some narrow subset of escaping and
> unescaping stuff.

It does what it says. It cannot use string_unescape because it needs to
determine the boundaries of quoted strings.

> Thus, it's still not on par with shell, right?

It does not interpret special character sequences other than quoting.

The feature would not be hard to add but I do not see the need.

Unfortunately, the existing string_unescape is totally not fitting to
be integrated into the parser for the purpose.

Thanks

Michal

> 
> >
> > Specifically double and single quotes can be used for quoting as
> > well as backslashes with the usual shell semantic. All quoting
> > characters are removed while the parameters are parsed.
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
  2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
                   ` (5 preceding siblings ...)
  2018-06-05 17:05 ` [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Andy Shevchenko
@ 2018-06-19 23:36 ` Andrew Morton
  2018-06-20 10:47   ` Michal Suchánek
  6 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2018-06-19 23:36 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: Jonathan Corbet, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
	Aaron Wu, Tony Luck, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

On Tue,  5 Jun 2018 18:43:07 +0200 Michal Suchanek <msuchanek@suse.de> wrote:

> due to work on the fadump_extra_args I looked at the kernel parameter parser
> and found its grammar rather curious.
> 
> It supports double quotes but not any other quoting characters so double quotes
> cannot be quoted. What's more, the quotes can be anywhere in the parameter
> name or value and are interpteted but are removed only from start and end of
> the parameter value.
> 
> These are the patches not specific to fadump which somewhat straighten the
> qouting grammar to make it on par with common shell interpreters.
> 
> Specifically double and single quotes can be used for quoting as well as
> backslashes with the usual shell semantic. All quoting characters are removed
> while the parameters are parsed.

Well.  It's nice.  I guess.  Is there any demand for these
capabilities?  I don't recall ever having seen a complaint - kernel
parameters tend to be pretty simple things.


Also, the break_arg_end() and squash_char() macros make me want to cry.
A macro which changes control flow hidden inside another macro!  Are
they reeeealy necessary?  Can't be done with some C helpers?  Maybe put
inquote, backslash, args, i into a new struct parser_state and pass a
pointer to that around the place?  At the very least, those macros
should be apologetically documented :(



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
  2018-06-19 23:36 ` Andrew Morton
@ 2018-06-20 10:47   ` Michal Suchánek
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Suchánek @ 2018-06-20 10:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jonathan Corbet, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
	Aaron Wu, Tony Luck, Thomas Gleixner, Steven Rostedt,,
	Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
	Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
	linux-kernel

On Tue, 19 Jun 2018 16:36:47 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue,  5 Jun 2018 18:43:07 +0200 Michal Suchanek
> <msuchanek@suse.de> wrote:
> 
> > due to work on the fadump_extra_args I looked at the kernel
> > parameter parser and found its grammar rather curious.
> > 
> > It supports double quotes but not any other quoting characters so
> > double quotes cannot be quoted. What's more, the quotes can be
> > anywhere in the parameter name or value and are interpteted but are
> > removed only from start and end of the parameter value.
> > 
> > These are the patches not specific to fadump which somewhat
> > straighten the qouting grammar to make it on par with common shell
> > interpreters.
> > 
> > Specifically double and single quotes can be used for quoting as
> > well as backslashes with the usual shell semantic. All quoting
> > characters are removed while the parameters are parsed.  
> 
> Well.  It's nice.  I guess.  Is there any demand for these
> capabilities?  I don't recall ever having seen a complaint - kernel
> parameters tend to be pretty simple things.

Yes, the complaint came with the nested arguments which are now not
pursued anymore. The grammar is really not very nice as it is, though.

> Also, the break_arg_end() and squash_char() macros make me want to
> cry. A macro which changes control flow hidden inside another macro!
> Are they reeeealy necessary?  

Seems better than repeating the same code 3 times.

> Can't be done with some C helpers?

You could not change the control flow then, could you?

Technically you could return something and decide based on that I
suppose.

> Maybe put inquote, backslash, args, i into a new struct parser_state
> and pass a pointer to that around the place?  At the very least,
> those macros should be apologetically documented :(

Yes, some description can be added, too.

Thanks

Michal

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-06-20 10:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-05 16:43 [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Michal Suchanek
2018-06-05 16:43 ` [PATCH v10 1/5] lib/cmdline.c: Add backslash support to kernel commandline parsing Michal Suchanek
2018-06-05 16:43 ` [PATCH v10 2/5] Documentation/admin-guide: backslash support in kernel arguments Michal Suchanek
2018-06-05 16:43 ` [PATCH v10 3/5] init/main.c: simplify repair_env_string Michal Suchanek
2018-06-05 16:43 ` [PATCH v10 4/5] lib/cmdline.c: Implement single quotes in commandline argument parsing Michal Suchanek
2018-06-05 16:43 ` [PATCH v10 5/5] Documentation/admin-guide: single quotes in kernel arguments Michal Suchanek
2018-06-05 17:05 ` [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement Andy Shevchenko
2018-06-05 17:25   ` Michal Suchánek
2018-06-19 23:36 ` Andrew Morton
2018-06-20 10:47   ` Michal Suchánek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).