2007-12-19 20:01:36

by Remy Bohmer

[permalink] [raw]
Subject: [patch 1/3] Add generic routine for parsing map-like options on kernel cmd-line (repost:CC to LKML)

This patch adds a generic routine to the kernel, so that a map of
settings can be entered on the kernel commandline.

Signed-off-by: Remy Bohmer <[email protected]>

---
---
include/linux/kernel.h | 1
lib/cmdline.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)

Index: linux-2.6.24-rc5-rt1/include/linux/kernel.h
===================================================================
--- linux-2.6.24-rc5-rt1.orig/include/linux/kernel.h 2007-12-18 21:32:09.000000000 +0100
+++ linux-2.6.24-rc5-rt1/include/linux/kernel.h 2007-12-18 21:33:58.000000000 +0100
@@ -164,6 +164,7 @@ extern int vsscanf(const char *, const c

extern int get_option(char **str, int *pint);
extern char *get_options(const char *str, int nints, int *ints);
+extern int get_map_option(const char *str, const char *key, int *pint);
extern unsigned long long memparse(char *ptr, char **retptr);

extern int core_kernel_text(unsigned long addr);
Index: linux-2.6.24-rc5-rt1/lib/cmdline.c
===================================================================
--- linux-2.6.24-rc5-rt1.orig/lib/cmdline.c 2007-10-09 22:31:38.000000000 +0200
+++ linux-2.6.24-rc5-rt1/lib/cmdline.c 2007-12-18 21:33:58.000000000 +0100
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/string.h>
+#include <asm/setup.h>

/*
* If a hyphen was found in get_option, this will handle the
@@ -114,6 +115,63 @@ char *get_options(const char *str, int n
}

/**
+ * get_map_option - Parse integer from an option map
+ *
+ * This function parses an integer from an option map like
+ * some_map=key1:99,key2:98,...,keyN:NN,NN
+ * Only the value of the first match is returned, or if no
+ * key option is given (key = NULL) the value of the first
+ * field without a ':' is returned.
+ *
+ * @str: option string
+ * @key: The key inside the map, can be NULL
+ * @pint: (output) integer value parsed from the map @str
+ *
+ * Return values:
+ * 0 - no int in string
+ * 1 - int found
+ */
+int get_map_option(const char *str, const char *key, int *pint)
+{
+ char buf[COMMAND_LINE_SIZE];
+ char *p, *substr;
+ int found = 0;
+
+ /* We must copy the string to the stack, because strsep()
+ changes it.*/
+ strncpy(buf, str, COMMAND_LINE_SIZE);
+ buf[COMMAND_LINE_SIZE-1] = '\0';
+
+ p = buf;
+ substr = strsep(&p, ",");
+ while ((!found) && (substr != NULL)) {
+ if (strlen(substr) != 0) {
+ if (key == NULL) {
+ /* Check for the absence of any ':' */
+ if (strchr(substr, ':') == NULL) {
+ sscanf(substr, "%d", pint);
+ found = 1;
+ }
+ } else {
+ /* check if the first part of the key matches */
+ if (!strncmp(substr, key, strlen(key))) {
+ substr += strlen(key);
+ /* Now the next char must be a ':',
+ if not, search for the next match */
+ if (*substr == ':') {
+ substr++;
+ sscanf(substr, "%d", pint);
+ found = 1;
+ }
+ }
+ }
+ }
+ substr = strsep(&p, ",");
+ }
+ return found;
+}
+
+/**
* memparse - parse a string with mem suffixes into a number
* @ptr: Where parse begins
* @retptr: (output) Pointer to next char after parse completes

--


2007-12-19 21:09:50

by Randy Dunlap

[permalink] [raw]
Subject: Re: [patch 1/3] Add generic routine for parsing map-like options on kernel cmd-line (repost:CC to LKML)

On Wed, 19 Dec 2007 20:45:52 +0100 Remy Bohmer wrote:

> This patch adds a generic routine to the kernel, so that a map of
> settings can be entered on the kernel commandline.
>
> Signed-off-by: Remy Bohmer <[email protected]>
>
> ---
> ---
> include/linux/kernel.h | 1
> lib/cmdline.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+)
>
> Index: linux-2.6.24-rc5-rt1/lib/cmdline.c
> ===================================================================
> --- linux-2.6.24-rc5-rt1.orig/lib/cmdline.c 2007-10-09 22:31:38.000000000 +0200
> +++ linux-2.6.24-rc5-rt1/lib/cmdline.c 2007-12-18 21:33:58.000000000 +0100
> @@ -114,6 +115,63 @@ char *get_options(const char *str, int n
> }
>
> /**
> + * get_map_option - Parse integer from an option map

The @param lines (below) need to go here, immediately following the
function short description (the line above). No intervening blank
lines.

> + *
> + * This function parses an integer from an option map like
> + * some_map=key1:99,key2:98,...,keyN:NN,NN
> + * Only the value of the first match is returned, or if no
> + * key option is given (key = NULL) the value of the first
> + * field without a ':' is returned.
> + *
> + * @str: option string
> + * @key: The key inside the map, can be NULL
> + * @pint: (output) integer value parsed from the map @str
> + *
> + * Return values:
> + * 0 - no int in string
> + * 1 - int found
> + */
> +int get_map_option(const char *str, const char *key, int *pint)
> +{
> + char buf[COMMAND_LINE_SIZE];

COMMAND_LINE_SIZE varies from 256 to 2048, depending on $ARCH.
That's a bit too much to declare on a function's local stack --
unless you are very certain of the call tree to this point and
that the total stack size is safe. Can you just kmalloc() this
buf?

> + char *p, *substr;
> + int found = 0;
> +
> + /* We must copy the string to the stack, because strsep()
> + changes it.*/
> + strncpy(buf, str, COMMAND_LINE_SIZE);
> + buf[COMMAND_LINE_SIZE-1] = '\0';
> +
> + p = buf;
> + substr = strsep(&p, ",");
> + while ((!found) && (substr != NULL)) {
> + if (strlen(substr) != 0) {
> + if (key == NULL) {
> + /* Check for the absence of any ':' */
> + if (strchr(substr, ':') == NULL) {
> + sscanf(substr, "%d", pint);
> + found = 1;
> + }
> + } else {
> + /* check if the first part of the key matches */
> + if (!strncmp(substr, key, strlen(key))) {
> + substr += strlen(key);
> + /* Now the next char must be a ':',
> + if not, search for the next match */
> + if (*substr == ':') {
> + substr++;
> + sscanf(substr, "%d", pint);
> + found = 1;
> + }
> + }
> + }
> + }
> + substr = strsep(&p, ",");
> + }
> + return found;
> +}

---
~Randy

2007-12-19 21:44:22

by Remy Bohmer

[permalink] [raw]
Subject: Re: [patch 1/3] Add generic routine for parsing map-like options on kernel cmd-line (repost:CC to LKML)

Hello Randy,

Sorry for the language errors, English is not my Native language, so I
make these stupid errors...

> > + * get_map_option - Parse integer from an option map
> The @param lines (below) need to go here, immediately following the
> function short description (the line above). No intervening blank
> lines.

OK, I will adapt that.

> > +int get_map_option(const char *str, const char *key, int *pint)
> > +{
> > + char buf[COMMAND_LINE_SIZE];
>
> COMMAND_LINE_SIZE varies from 256 to 2048, depending on $ARCH.
> That's a bit too much to declare on a function's local stack --
> unless you are very certain of the call tree to this point and
> that the total stack size is safe. Can you just kmalloc() this
> buf?

I know it is big on a 4k stack, and I also think it is not very nice...
But kmalloc() panics the kernel if I do it as soon as in the __setup() code.

The problem is that the original string may not be modified by the
cmdline-parser, and I do not know the length of the command up front,
(except that it cannot be longer than this define). Allocating a
global buffer is not safe and needs locking. So actually only what
left for me was the stack... or rewrite the used C-library-routines
completely myself, for this purpose only, which is also not nice.

I hope someone could point to me to another possibilty, that I did not
think of yet.


Kind Regards,

Remy

>
> > + char *p, *substr;
> > + int found = 0;
> > +
> > + /* We must copy the string to the stack, because strsep()
> > + changes it.*/
> > + strncpy(buf, str, COMMAND_LINE_SIZE);
> > + buf[COMMAND_LINE_SIZE-1] = '\0';
> > +
> > + p = buf;
> > + substr = strsep(&p, ",");
> > + while ((!found) && (substr != NULL)) {
> > + if (strlen(substr) != 0) {
> > + if (key == NULL) {
> > + /* Check for the absence of any ':' */
> > + if (strchr(substr, ':') == NULL) {
> > + sscanf(substr, "%d", pint);
> > + found = 1;
> > + }
> > + } else {
> > + /* check if the first part of the key matches */
> > + if (!strncmp(substr, key, strlen(key))) {
> > + substr += strlen(key);
> > + /* Now the next char must be a ':',
> > + if not, search for the next match */
> > + if (*substr == ':') {
> > + substr++;
> > + sscanf(substr, "%d", pint);
> > + found = 1;
> > + }
> > + }
> > + }
> > + }
> > + substr = strsep(&p, ",");
> > + }
> > + return found;
> > +}
>
> ---
> ~Randy
>