[U-Boot] [PATCH v1 1/4] Kconfig: Enable usage of escape char '\' in string values

[Masahiro Yamada]

Hi Stefan,




2015-05-15 16:13 GMT+09:00 Stefan Roese <sr at denx.de>:
> Hi Masahiro,
>
>
> On 13.05.2015 03:34, Masahiro Yamada wrote:
>>
>> Hi, Simon, Stefan,
>>
>>
>> 2015-05-12 7:41 GMT+09:00 Simon Glass <sjg at chromium.org>:
>>>
>>> Hi Stefan,
>>>
>>> On 11 May 2015 at 07:27, Stefan Roese <mail at roese.nl> wrote:
>>>>
>>>> Hi Simon, Hi Masahiro,
>>>>
>>>>
>>>> On 11.05.2015 09:58, Stefan Roese wrote:
>>>>>
>>>>>
>>>>> On 10.05.2015 16:48, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>> On 7 May 2015 at 06:13, Stefan Roese <sr at denx.de> wrote:
>>>>>>>
>>>>>>>
>>>>>>> I might have missed something, but I failed to use the escape char
>>>>>>> '\'
>>>>>>> in strings. To pass a printf format string like "foo %d bar\n" via
>>>>>>> Kconfig to the code.
>>>>>>>
>>>>>>> Right now its not possible to use the escape character '\' in Kconfig
>>>>>>> string values correctly to e.g. set this string value "test
>>>>>>> output\n".
>>>>>>> The '\n' will be converted to 'n'.
>>>>>>>
>>>>>>> The current implementation removes some of the '\' chars from the
>>>>>>> input
>>>>>>> string in conf_set_sym_val(). Examples:
>>>>>>>
>>>>>>> '\'     -> ''
>>>>>>> '\\'    -> '\'
>>>>>>> '\\\'   -> '\'
>>>>>>> '\\\\'  -> '\\'
>>>>>>> ...
>>>>>>>
>>>>>>> And then doubles the backslash chars in the output string in
>>>>>>> sym_escape_string_value(). Example:
>>>>>>>
>>>>>>> '\'     -> ''   -> ''
>>>>>>> '\\'    -> '\'  -> '\\'
>>>>>>> '\\\'   -> '\'  -> '\\'
>>>>>>> '\\\\'  -> '\\' -> '\\\\'
>>>>>>> ...
>>>>>>>
>>>>>>> As you see in these examples, its impossible to generate a single '\'
>>>>>>> charater in the output string as its needed for something like '\n'.
>>>>>>>
>>>>>>> This patch now changes this behavior to not drop some backslashes in
>>>>>>> conf_set_sym_val() and to not add new backslashes in the resulting
>>>>>>> output string. Removing the function sym_escape_string_value()
>>>>>>> completely as its not needed anymore.
>>>>>>>
>>>>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>>>>> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>> ---
>>>>>>>
>>>>>>>    scripts/kconfig/confdata.c | 20 +++++++++-----------
>>>>>>>    scripts/kconfig/symbol.c   | 43
>>>>>>> -------------------------------------------
>>>>>>>    2 files changed, 9 insertions(+), 54 deletions(-)
>>>>>>
>>>>>>
>>>>>>
>>>>>> This looks right to me. But I do see one problem - the default string
>>>>>> for CONFIG_AUTOBOOT_PROMPT appears as:
>>>>>>
>>>>>> "Autoboot in %d secondsn"
>>>>>>
>>>>>> so something is still removing the \ in the Kconfig default;
>>>>>
>>>>>
>>>>>
>>>>> Right. Thanks for spotting. I'll fix this in v3.
>>>>
>>>>
>>>>
>>>> I could easily change the default string in the Kconfig file to
>>>> "Autoboot in
>>>> %d seconds\\n". This works. But its a different syntax regarding using
>>>> the
>>>> escape character backslash compared to editing the .config file or
>>>> editing
>>>> the string in "make menuconfig etc...". So I hesitate to "fix" it this
>>>> way.
>>>>
>>>> Unfortunately fixing this issue in the code is not that easy. At least
>>>> not
>>>> for me. As the default values of the "string values" are set in the
>>>> conf_parse() function (in scripts/kconfig/zconf.y). And I really have
>>>> absolutely no experience with yacc / bison. Perhaps one of you guys has
>>>> a
>>>> quick fix to make this default value of strings compatible again so that
>>>> this additional '\' is not needed in the Kconfig file?
>>>
>>>
>>> Well I am familiar with those tools but I think Masahiro probably
>>> knows a lot more here.
>>
>>
>>
>> I am not so familiar with Bison, and I am getting a bit busy these days.
>> So, I cannot find time to take a close look.  Sorry.
>>
>> If Simon (or someone else) could follow it up, that'd be nice.
>>
>> BTW, if you have already figured out that conf_parse() is the cause of
>> the problem,
>> you do not have to invoke Bison.
>>
>> Bison does not touch the C part.
>> conf_parse() is just copied verbatim from zconf.y to zconf.tab.c_shipped.
>>
>> You can modify conf_parse() exactly in the same way in both of them.
>
>
> Thanks. Did it and found that zconfparse() is responsible for this default
> value configuration / parsing. I must be missing something, but I fail to
> see where this function is really implemented:
>
> $ git grep zconfparse
> scripts/kconfig/lkc.h:int zconfparse(void);
> scripts/kconfig/zconf.tab.c_shipped:#define yyparse         zconfparse
> scripts/kconfig/zconf.tab.c_shipped:    zconfparse();
> scripts/kconfig/zconf.y:        zconfparse();





Bison converts zconf.y into zconf.tab.c_shipped

You will notice the following part.

/* Substitute the variable and function names.  */
#define yyparse         zconfparse
#define yylex           zconflex



I think zconfparse was taken from the basename of zconf.y


And then, you will also notice the following part:



/*----------.
| yyparse.  |
`----------*/

#ifdef YYPARSE_PARAM
#if (defined __STDC__ || defined __C99__FUNC__ \
     || defined __cplusplus || defined _MSC_VER)
int
yyparse (void *YYPARSE_PARAM)
#else
int
yyparse (YYPARSE_PARAM)
    void *YYPARSE_PARAM;
#endif
#else /* ! YYPARSE_PARAM */
#if (defined __STDC__ || defined __C99__FUNC__ \
     || defined __cplusplus || defined _MSC_VER)
int
yyparse (void)
#else
int
yyparse ()

#endif
#endif
{
    int yystate;
    /* Number of tokens to shift before error messages enabled.  */




"yyparse" is replaced with "zconfparse" by the prepreocessor,
so this is the implementation of zconfparse(),
and it is the parser generated by Bison.

I think it is almost impossible to read the generated parser directly.
Instead, you should read zconf.y, but you need to understand Bison
grammer for that...





-- 
Best Regards
Masahiro Yamada