2018-01-29 4:21 GMT+09:00 Philip Prindeville
<philipp_subx@xxxxxxxxxxxxxxxxxxxxx>:
> Inline
>
>> On Jan 28, 2018, at 4:11 AM, Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>>
>> 2018-01-22 13:58 GMT+09:00 Philip Prindeville <philipp@xxxxxxxxxxxxxxxxxxxxx>:
>>> From: Philip Prindeville <philipp@xxxxxxxxxxxxxxxxxxxxx>
>>>
>>> Since kconfig reads and writes makefiles, reflect that dollars need
>>> to be doubled up in make syntax so that they're not interpolated.
>>>
>>> Example: if we have a CONFIG_VAR of type string that we want to have
>>> the value of "abc$def" then it must be written to the file .config
>>> file as:
>>>
>>> CONFIG_VAR="abc$$def"
>>>
>>> to avoid the $d from being interpolated as the $(d).
>>>
>>> Signed-off-by: Philip Prindeville <philipp@xxxxxxxxxxxxxxxxxxxxx>
>>
>>
>> Question:
>> If you want to interpolate $d, how can you do that?
>> Is it possible to make CONFIG_VAR="abc${d}ef” ?
>
>
> That would be an entirely new requirement of Kbuild, then.
I would not say this is requirement, but
looks like xtensa is already doing interpolation. [1]
[1] https://github.com/torvalds/linux/blob/v4.15/arch/xtensa/configs/cadence_csp_defconfig#L18
> No type that we enter manually has any form of interpretation or interpolation: not string, not bool, not hex, and not int… These values are always literals.
>
> If there’s to be interpolation, then this would be done in the Makefile itself as:
>
> $(LEFT_PART)$($(MIDDLE_VAR_NAME))$(RIGHT_PART)
>
> where MIDDLE_VAR_NAME:=d or some other similar construct.
>
> That would be done in the Makefile which includes the .config, not the .config itself. Witness:
>
> obj-$(CONFIG_BONDING) += bonding/
>
> in drivers/net/Makefile —- this is how (and more importantly WHERE) we handle interpolation.
>
>>
>> Currently,
>> User input: abc$$def -> Output: CONFIG_FOO="abc$$def"
>>
>> User input: abc${d}ef -> Output: CONFIG_FOO="abc${d}ef"
>>
>> Both are possible by inputting the string verbatim.
>
>
> Then that would be the result of abusing Kbuild. The values that we enter are meant to be literal values. Not macros to be interpolated.
>
> Otherwise, we’d have to do all sorts of type checking like (a) is $(d) defined? (b) is $(d) also of type STRING? etc. Way out of scope of the current language definition.
There is some room for discussion if we should ban interpolation or not.
Even if this is abuse, it is matter of Kbuild,
not in the scope of Kconfig.
My point is,
'$' is not a special character for Kconfig.
So, Kconfig is agnostic about it.
>
>>
>> With your patch, the conversion is forced as follows:
>>
>> User Input: abc$def -> Output: CONFIG_FOO="abc$$def
>>
>> So, having a single $ is banned.
>>
>>
>> I am wondering what value is added by this patch…
>
>
> It (a) prevents the user who is potentially naive user (who is oblivious about Makefile syntax and the fact that dollars need to be doubled up) from accidentally entering a single one where two would [currently] be required…
There is one more thing you are missing.
You are only talking about the use in Makefile, but
string type options are used in C as well.
For example, CONFIG_DEFAULT_HOSTNAME [2], CONFIG_PANEL_BOOT_MESSAGE [3].
[2] https://github.com/torvalds/linux/blob/v4.15/include/linux/uts.h#L13
[3] https://github.com/torvalds/linux/blob/v4.15/drivers/auxdisplay/charlcd.c#L730
With your patch,
User input: abc$def ->
Output of include/generated/autoconf.h: #define CONFIG_FOO "ab$$def"
This is breakage since $ is doubled up in C language,
where it should not happen.
> (we do, after all, handle backslash, quote, and dquote without requiring the user to be aware of their special properties… why be inconsistent?)
I am not sure about this background,
but I guess escaping is C-oriented.
User input: abc"def -> #define CONFIG_FOO "abc\"def"
User input: abc\def -> #define CONFIG_FOO "abc\\def"
Single-quote is not escaped. Kconfig handles it as-is.
User input: abc'def -> #define CONFIG_FOO "abc'def"
So, one down-side is, CONFIG option cannot contain escape sequence like '\n'
I am thinking of dropping escaping.
U-Boot already did this. [4]
[4] https://github.com/u-boot/u-boot/commit/20c20826efabf9ed64f5555bc8739bdbb89c1edd
> and (b) it provides additional error checking and safety by affirming that a STRING really is a literal value and not a MACRO open to some forms of interpolation but not others.
Why does Kconfig need to check this?
> I guess the question boils down to: should the casual Kbuild user have to be familiar with the intricacies of Makefile syntax? I don’t think so. The whole point of Kbuild is to provide a layer of abstraction so that he doesn’t need to
> hand-edit Makefiles. This fix just makes that abstraction more complete.
>
> -Philip
>
>
>>
>>
>>> scripts/kconfig/confdata.c | 6 +++++-
>>> scripts/kconfig/symbol.c | 9 ++++++---
>>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>>> index 87f723804079ed3b6c1fbb0d1470f717582c4a28..e45f23dd04a7ceede2533d80d3ec8fc993685112 100644
>>> --- a/scripts/kconfig/confdata.c
>>> +++ b/scripts/kconfig/confdata.c
>>> @@ -155,11 +155,15 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
>>> case S_STRING:
>>> if (*p++ != '"')
>>> break;
>>> - for (p2 = p; (p2 = strpbrk(p2, "\"\\")); p2++) {
>>> + for (p2 = p; (p2 = strpbrk(p2, "\"\\$")); p2++) {
>>> if (*p2 == '"') {
>>> *p2 = 0;
>>> break;
>>> }
>>> + else if (*p2 == '$' && p2[1] != '$') {
>>> + conf_warning("invalid string found");
>>> + break;
>>> + }
>>> memmove(p2, p2 + 1, strlen(p2));
>>> }
>>> if (!p2) {
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index 7caabdb51c647e12e35b500bdebd262ba1545eb0..2458da4e2b203e5213e4ea0cb380f8f88af566ab 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -923,7 +923,7 @@ const char *sym_escape_string_value(const char *in)
>>>
>>> p = in;
>>> for (;;) {
>>> - l = strcspn(p, "\"\\");
>>> + l = strcspn(p, "\"\\$");
>>> p += l;
>>>
>>> if (p[0] == '\0')
>>> @@ -940,14 +940,17 @@ const char *sym_escape_string_value(const char *in)
>>>
>>> p = in;
>>> for (;;) {
>>> - l = strcspn(p, "\"\\");
>>> + l = strcspn(p, "\"\\$");
>>> strncat(res, p, l);
>>> p += l;
>>>
>>> if (p[0] == '\0')
>>> break;
>>>
>>> - strcat(res, "\\");
>>> + if (p[0] == '$')
>>> + strcat(res, "$");
>>> + else
>>> + strcat(res, "\\");
>>> strncat(res, p++, 1);
>>> }
>>>
>>> --
>>> 2.7.4
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
