Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement

[Michal SuchÃnek]

On Tue, 19 Jun 2018 16:36:47 -0700
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Tue,  5 Jun 2018 18:43:07 +0200 Michal Suchanek
> <msuchanek@xxxxxxx> 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