public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v1 1/4] Kconfig: Enable usage of escape char '\' in string values
Date: Fri, 15 May 2015 09:13:59 +0200	[thread overview]
Message-ID: <55559CB7.1050807@denx.de> (raw)
In-Reply-To: <CAK7LNATDp+2dRZM4sy91ZqikROz6kMcFTVxE=55QFE-yZMSGkQ@mail.gmail.com>

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@chromium.org>:
>> Hi Stefan,
>>
>> On 11 May 2015 at 07:27, Stefan Roese <mail@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@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@denx.de>
>>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>>> Cc: Simon Glass <sjg@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();

I'm inclined to just add this additional backslash to the default value 
in Kconfig.

Thanks,
Stefan

  reply	other threads:[~2015-05-15  7:13 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07 12:13 [U-Boot] [PATCH v1 0/4] Add SHA256 encrypted stop string for autobooting Stefan Roese
2015-05-07 12:13 ` [U-Boot] [PATCH v1 1/4] Kconfig: Enable usage of escape char '\' in string values Stefan Roese
2015-05-07 12:41   ` Masahiro Yamada
2015-05-07 12:46     ` Stefan Roese
2015-05-07 12:53       ` Masahiro Yamada
2015-05-10 14:48   ` Simon Glass
2015-05-11  7:58     ` Stefan Roese
2015-05-11 13:27       ` Stefan Roese
2015-05-11 22:41         ` Simon Glass
2015-05-13  1:34           ` Masahiro Yamada
2015-05-15  7:13             ` Stefan Roese [this message]
2015-05-15  7:49               ` Masahiro Yamada
2015-05-07 12:13 ` [U-Boot] [PATCH v1 2/4] autoboot.c: Remove CONFIG_AUTOBOOT_STOP_STR2 and CONFIG_AUTOBOOT_DELAY_STR2 Stefan Roese
2015-05-07 23:47   ` Simon Glass
2015-05-11 12:38   ` Tom Rini
2015-05-07 12:13 ` [U-Boot] [PATCH v1 3/4] autoboot.c: Move config options to Kconfig Stefan Roese
2015-05-07 23:51   ` Simon Glass
2015-05-08  3:30     ` Masahiro Yamada
2015-05-08  6:00       ` Stefan Roese
2015-05-08  5:55     ` Stefan Roese
2015-05-10 14:49       ` Simon Glass
2015-05-07 12:13 ` [U-Boot] [PATCH v1 4/4] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password Stefan Roese
2015-05-07 20:56   ` Magnus Lilja
2015-05-08  7:52   ` [U-Boot] [PATCH v2 " Stefan Roese
2015-05-10 14:49     ` Simon Glass
2015-05-11  7:16     ` Andreas Bießmann
2015-05-11  7:44       ` Stefan Roese
2015-05-15  7:44     ` Magnus Lilja
2015-05-15  8:44       ` Stefan Roese

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55559CB7.1050807@denx.de \
    --to=sr@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox