Re: [PATCH v2 09/21] kconfig: add 'macro' keyword to support user-defined function
From: Masahiro Yamada
Date: Fri Apr 13 2018 - 01:45:14 EST
2018-04-01 15:05 GMT+09:00 Ulf Magnusson <ulfalizer@xxxxxxxxx>:
> On Tue, Mar 27, 2018 at 7:29 AM, Masahiro Yamada
> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>> Now, we got a basic ability to test compiler capability in Kconfig.
>>
>> config CC_HAS_STACKPROTECTOR
>> def_bool $(shell (($CC -Werror -fstack-protector -c -x c /dev/null -o /dev/null) && echo y) || echo n)
>>
>> This works, but it is ugly to repeat this long boilerplate.
>>
>> We want to describe like this:
>>
>> config CC_HAS_STACKPROTECTOR
>> bool
>> default $(cc-option -fstack-protector)
>>
>> It is straight-forward to add a new function, but I do not like to
>> hard-code specialized functions like this. Hence, here is another
>> feature to add functions from Kconfig files.
>>
>> A user-defined function is defined with a special keyword 'macro'.
>> It can be referenced in the same way as built-in functions. This
>> feature was also inspired by Makefile where user-defined functions
>> are referenced by $(call func-name, args...), but I omitted the 'call'
>> to makes it shorter.
>>
>> The macro definition can contain $(1), $(2), ... which will be replaced
>> with arguments from the caller. The macro works just as a textual
>> shorthand, which is also expanded in the lexer phase.
>>
>> [Example Code]
>>
>> macro success $(shell ($(1) && echo y) || echo n)
>>
>> config TRUE
>> bool "true"
>> default $(success true)
>>
>> config FALSE
>> bool "false"
>> default $(success false)
>>
>> [Result]
>>
>> $ make -s alldefconfig
>> $ tail -n 2 .config
>> CONFIG_TRUE=y
>> # CONFIG_FALSE is not set
>>
>> [Example Code]
>>
>> macro success $(shell ($(1) && echo y) || echo n)
>>
>> macro cc-option $(success $CC -Werror $(1) -c -x c /dev/null -o /dev/null)
>>
>> config CC_HAS_STACKPROTECTOR
>> def_bool $(cc-option -fstack-protector)
>>
>> [Result]
>> $ make -s alldefconfig
>> $ tail -n 1 .config
>> CONFIG_CC_HAS_STACKPROTECTOR=y
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>> ---
>>
>> Reminder for myself:
>> Update Documentation/kbuild/kconfig-language.txt
>>
>> Changes in v2:
>> - Use 'macro' directly instead of inside the string type symbol.
>>
>> scripts/kconfig/function.c | 59 +++++++++++++++++++++++++++++++++++++++++++--
>> scripts/kconfig/lkc_proto.h | 1 +
>> scripts/kconfig/zconf.l | 31 ++++++++++++++++++++++++
>> 3 files changed, 89 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/function.c b/scripts/kconfig/function.c
>> index 913685f..389bb44 100644
>> --- a/scripts/kconfig/function.c
>> +++ b/scripts/kconfig/function.c
>> @@ -15,6 +15,7 @@ static LIST_HEAD(function_list);
>> struct function {
>> char *name;
>> char *(*func)(struct function *f, int argc, char *argv[]);
>> + char *macro;
>> struct list_head node;
>> };
>>
>> @@ -31,7 +32,8 @@ static struct function *func_lookup(const char *name)
>> }
>>
>> static void func_add(const char *name,
>> - char *(*func)(struct function *f, int argc, char *argv[]))
>> + char *(*func)(struct function *f, int argc, char *argv[]),
>> + const char *macro)
>> {
>> struct function *f;
>>
>> @@ -44,6 +46,7 @@ static void func_add(const char *name,
>> f = xmalloc(sizeof(*f));
>> f->name = xstrdup(name);
>> f->func = func;
>> + f->macro = macro ? xstrdup(macro) : NULL;
>>
>> list_add_tail(&f->node, &function_list);
>> }
>> @@ -51,6 +54,7 @@ static void func_add(const char *name,
>> static void func_del(struct function *f)
>> {
>> list_del(&f->node);
>> + free(f->macro);
>> free(f->name);
>> free(f);
>> }
>> @@ -108,6 +112,57 @@ char *func_eval_n(const char *func, size_t n)
>> return res;
>> }
>>
>> +/* run user-defined function */
>> +static char *do_macro(struct function *f, int argc, char *argv[])
>> +{
>> + char *new;
>> + char *src, *p, *res;
>> + size_t newlen;
>> + int n;
>> +
>> + new = xmalloc(1);
>> + *new = 0;
>
> new = '\0' would be consistent with the rest of the code.
>
>> +
>> + /*
>> + * This is a format string. $(1), $(2), ... must be replaced with
>> + * function arguments.
>> + */
>> + src = f->macro;
>> + p = src;
>> +
>> + while ((p = strstr(p, "$("))) {
>> + if (isdigit(p[2]) && p[3] == ')') {
>> + n = p[2] - '0';
>> + if (n < argc) {
>> + newlen = strlen(new) + (p - src) +
>> + strlen(argv[n]) + 1;
>> + new = xrealloc(new, newlen);
>> + strncat(new, src, p - src);
>> + strcat(new, argv[n]);
>> + src = p + 4;
>> + }
>
> Might be nice to warn when a macro call has missing arguments.
I made this a feature as we see in Make.
In Makefile, it is allowed to pass more or less arguments.
cc-option is often used with one flag.
$(call cc-option,-fno-PIE)
But, it can take one more as a fall-back compiler option.
$(call cc-option,-mtune=marvell-f,-mtune=xscale)
Of course, we can describe the first one like follows
just to pass a blank parameter as the last parameter.
$(call cc-option,-fno-PIE,)
>> + p += 2;
>> + }
>> + p += 2;
>> + }
>
> I had to stare at this for a while to see how it worked. What do you
> think of this tweak?
>
> while ((p = strstr(p, "$("))) {
> if (isdigit(p[2]) && p[3] == ')') {
> n = p[2] - '0';
> if (n < argc) {
> newlen = strlen(new) + (p - src) +
> strlen(argv[n]) + 1;
> new = xrealloc(new, newlen);
> strncat(new, src, p - src);
> strcat(new, argv[n]);
>
> /*
> * Jump past macro parameter ("$(n)") and remember the
> * position
> */
> p += 4;
> src = p;
>
> continue;
> }
> }
>
> /* Jump past "$(" that isn't from a macro parameter */
> p += 2;
> }
Will work, but I re-implemented the parser in v3.
>> +
>> + newlen = strlen(new) + strlen(src) + 1;
>> + new = xrealloc(new, newlen);
>> + strcat(new, src);
>> +
>> + res = expand_string_value(new);
>> +
>> + free(new);
>> +
>> + return res;
>> +}
>> +
>> +/* add user-defined function (macro) */
>> +void func_add_macro(const char *name, const char *macro)
>> +{
>> + func_add(name, do_macro, macro);
>> +}
>> +
>> /* built-in functions */
>> static char *do_shell(struct function *f, int argc, char *argv[])
>> {
>> @@ -157,7 +212,7 @@ static char *do_shell(struct function *f, int argc, char *argv[])
>> void func_init(void)
>> {
>> /* register built-in functions */
>> - func_add("shell", do_shell);
>> + func_add("shell", do_shell, NULL);
>> }
>>
>> void func_exit(void)
>> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
>> index 09a4f53..48699c0 100644
>> --- a/scripts/kconfig/lkc_proto.h
>> +++ b/scripts/kconfig/lkc_proto.h
>> @@ -50,6 +50,7 @@ const char * prop_get_type_name(enum prop_type type);
>>
>> /* function.c */
>> char *func_eval_n(const char *func, size_t n);
>> +void func_add_macro(const char *name, const char *macro);
>> void func_init(void);
>> void func_exit(void);
>>
>> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
>> index 551ca47..6a18c68 100644
>> --- a/scripts/kconfig/zconf.l
>> +++ b/scripts/kconfig/zconf.l
>> @@ -74,6 +74,36 @@ static void warn_ignored_character(char chr)
>> "%s:%d:warning: ignoring unsupported character '%c'\n",
>> zconf_curname(), zconf_lineno(), chr);
>> }
>> +
>> +static void handle_macro(const char *text)
>> +{
>> + char *p, *q;
>> +
>> + while (isspace(*text))
>> + text++;
>> +
>> + p = xstrdup(text);
>> +
>> + q = p;
>> + while (isalnum(*q) || *q == '_' || *q == '-')
>> + q++;
>> +
>> + if (q == p || !*q) {
>> + yyerror("invalid\n");
>> + goto free;
>> + }
>> +
>> + *q = '\0';
>> +
>> + q++;
>> + while (isspace(*q))
>> + q++;
>> +
>> + func_add_macro(p, q);
>> +free:
>> + free(p);
>> +}
>> +
>> %}
>>
>> n [A-Za-z0-9_-]
>> @@ -82,6 +112,7 @@ n [A-Za-z0-9_-]
>> int str = 0;
>> int ts, i;
>>
>> +"macro"[ \t].* handle_macro(yytext + 6);
>> [ \t]*#.*\n |
>> [ \t]*\n {
>> return T_EOL;
>> --
>> 2.7.4
>>
>
> Cheers,
> Ulf
> --
> 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