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

[Remy Bohmer]

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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/