Re: [RFC] Re: Parsing kernel parameters and escaping "

[Rusty Russell <rusty@rustcorp.com.au>]

On Mon, 13 Jul 2009 03:29:39 am Daniel Mierswa wrote:
> Tested:
> |param| => [param[(none)]]
> |param=| => [param[]]
> |param=value| => [param[value]]
> |param=value=withequal | => [param[value=withequal]]
> |param="value with spaces" | => [param[value with spaces]]
> |param="value with spaces and quotes \"" | => [param[value with spaces and
> | quotes "]] param=\"foo\" | => [param[\"foo" ]]
> |"param = value" | => [param = value[(none)]]

Hi Daniel!

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.

> Thanks for your kind feedback, I'm willing to put more effort into this
> when needed. I really first wanted to check if patches for this are
> welcomed.

Well, IMO it's a maintainer's job to give feedback, and patches should always be welcomed (even if not applied!).

Now to the details:

> +static size_t pull_token(char *args, char const *delim)
>  {
> -       unsigned int i, equals = 0;
> -       int in_quote = 0, quoted = 0;
> -       char *next;
> +       size_t length = 0;
> +       char *iterator = NULL, *last_quote = NULL;
> +
> +       for (iterator = args; *iterator; iterator++, length++) {

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.

> +               if (*iterator == '"') {
> +                       if (last_quote) {
> +                               char *mover = last_quote;
> +
> +                               /* move whole string back until current " is reached */ 
> +                               while (mover != iterator - 1) {
> +                                       *mover = *(mover + 1);
> +                                       mover++;
> +                               }

memmove?

> +               {
> +                       /* check for delimiter */
> +                       char const *delim_iterator = NULL;
> +                       for (delim_iterator = delim; *delim_iterator; delim_iterator++) {
> +                               if (*iterator == *delim_iterator) {
> +                                       return length;
> +                               }
> +                       }

How about:
	if (strchr(delim, *iterator))
		return length;

> +static char *next_arg(char *args, char **param, char **val)
> +{
> +       size_t len;
> +
> +       /* Chew leading spaces */
> +       while (*args == ' ')
> +               args++;

Note that this will undo another pending patch, which changes this to
isspace() to handle tabs et al.

Thanks!
Rusty.