| Message ID | 1526537830-22606-8-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote: > This commit adds a new concept 'function' to do more text processing > in Kconfig. > > A function call looks like this: > > $(function,arg1,arg2,arg3,...) > > This commit adds the basic infrastructure to expand functions. > Change the text expansion helpers to take arguments. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v4: > - Error out if arguments more than FUNCTION_MAX_ARGS are passed > - Use a comma as a delimiter between the function name and the > first argument > - Check the number of arguments accepted by each function > - Support delayed expansion of arguments. > This will be needed to implement 'if' function > > Changes in v3: > - Split base infrastructure and 'shell' function > into separate patches. > > Changes in v2: > - Use 'shell' for getting stdout from the comment. > It was 'shell-stdout' in the previous version. > - Simplify the implementation since the expansion has been moved to > lexer. > > scripts/kconfig/preprocess.c | 168 ++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 159 insertions(+), 9 deletions(-) > > diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c > index 1bf506c..5be28ec 100644 > --- a/scripts/kconfig/preprocess.c > +++ b/scripts/kconfig/preprocess.c > @@ -3,12 +3,17 @@ > // Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com> > > #include <stdarg.h> > +#include <stdbool.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > > #include "list.h" > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > + > +static char *expand_string_with_args(const char *in, int argc, char *argv[]); > + > static void __attribute__((noreturn)) pperror(const char *format, ...) > { > va_list ap; > @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name) > } > } > > -static char *eval_clause(const char *in) > +/* > + * Built-in functions > + */ > +struct function { > + const char *name; > + unsigned int min_args; > + unsigned int max_args; > + bool expand_args; > + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]); > +}; If a typedef was provided for the function then ... > + > +static const struct function function_table[] = { > + /* Name MIN MAX EXP? Function */ > +}; > + > +#define FUNCTION_MAX_ARGS 16 > + > +static char *function_expand_arg_and_call(char *(*func)(int argc, char *argv[], > + int old_argc, > + char *old_argv[]), > + int argc, char *argv[], > + int old_argc, char *old_argv[]) this could be much easier to read. > +{ > + char *expanded_argv[FUNCTION_MAX_ARGS], *res; > + int i; > + > + for (i = 0; i < argc; i++) > + expanded_argv[i] = expand_string_with_args(argv[i], > + old_argc, old_argv); No check for too many arguments here - maybe it is done in some other place. > + > + res = func(argc, expanded_argv, 0, NULL); > + > + for (i = 0; i < argc; i++) > + free(expanded_argv[i]); > + > + return res; > +} > + > +static char *function_call(const char *name, int argc, char *argv[], > + int old_argc, char *old_argv[]) > +{ > + const struct function *f; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(function_table); i++) { > + f = &function_table[i]; > + if (strcmp(f->name, name)) > + continue; > + > + if (argc < f->min_args) > + pperror("too few function arguments passed to '%s'", > + name); > + > + if (argc > f->max_args) > + pperror("too many function arguments passed to '%s'", > + name); Number of arguments checked here, but max_args is not assiged in this patch. > + > + if (f->expand_args) > + return function_expand_arg_and_call(f->func, argc, argv, > + old_argc, old_argv); > + return f->func(argc, argv, old_argc, old_argv); > + } > + > + return NULL; > +} > + > +/* > + * Evaluate a clause with arguments. argc/argv are arguments from the upper > + * function call. > + * > + * Returned string must be freed when done > + */ > +static char *eval_clause(const char *in, int argc, char *argv[]) > { > - char *res, *name; > + char *tmp, *prev, *p, *res, *name; > + int new_argc = 0; > + char *new_argv[FUNCTION_MAX_ARGS]; > + int nest = 0; > + int i; > > /* > * Returns an empty string because '$()' should be evaluated > @@ -99,10 +180,69 @@ static char *eval_clause(const char *in) > if (!*in) > return xstrdup(""); > > - name = expand_string(in); > + tmp = xstrdup(in); > + > + prev = p = tmp; > + > + /* > + * Split into tokens > + * The function name and arguments are separated by a comma. > + * For example, if the function call is like this: > + * $(foo,abc,$(x),$(y)) > + * > + * The input string for this helper should be: > + * foo,abc,$(x),$(y) > + * > + * and split into: > + * new_argv[0] = 'foo' > + * new_argv[1] = 'abc' > + * new_argv[2] = '$(x)' > + * new_argv[3] = '$(y)' > + */ > + while (*p) { > + if (nest == 0 && *p == ',') { > + *p = 0; > + if (new_argc >= FUNCTION_MAX_ARGS) > + pperror("too many function arguments"); > + new_argv[new_argc++] = prev; > + prev = p + 1; > + } else if (*p == '(') { > + nest++; > + } else if (*p == ')') { > + nest--; > + } > + > + p++; > + } > + new_argv[new_argc++] = prev; Will the following be equal: $(foo,abc,$(x),$(y)) $(foo, abc, $(x), $(y)) make is rather annoying as space is significant, but there seems no good reason for kconfig to inheritate this. So unless there are good arguments consider alloing the spaces. If the current implmentation already supports optional spaces then I just missed it whie reviewing. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sam, 2018-05-20 23:50 GMT+09:00 Sam Ravnborg <sam@ravnborg.org>: > On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote: >> This commit adds a new concept 'function' to do more text processing >> in Kconfig. >> >> A function call looks like this: >> >> $(function,arg1,arg2,arg3,...) >> >> This commit adds the basic infrastructure to expand functions. >> Change the text expansion helpers to take arguments. >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> --- >> >> Changes in v4: >> - Error out if arguments more than FUNCTION_MAX_ARGS are passed >> - Use a comma as a delimiter between the function name and the >> first argument >> - Check the number of arguments accepted by each function >> - Support delayed expansion of arguments. >> This will be needed to implement 'if' function >> >> Changes in v3: >> - Split base infrastructure and 'shell' function >> into separate patches. >> >> Changes in v2: >> - Use 'shell' for getting stdout from the comment. >> It was 'shell-stdout' in the previous version. >> - Simplify the implementation since the expansion has been moved to >> lexer. >> >> scripts/kconfig/preprocess.c | 168 ++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 159 insertions(+), 9 deletions(-) >> >> diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c >> index 1bf506c..5be28ec 100644 >> --- a/scripts/kconfig/preprocess.c >> +++ b/scripts/kconfig/preprocess.c >> @@ -3,12 +3,17 @@ >> // Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com> >> >> #include <stdarg.h> >> +#include <stdbool.h> >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> >> #include "list.h" >> >> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) >> + >> +static char *expand_string_with_args(const char *in, int argc, char *argv[]); >> + >> static void __attribute__((noreturn)) pperror(const char *format, ...) >> { >> va_list ap; >> @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name) >> } >> } >> >> -static char *eval_clause(const char *in) >> +/* >> + * Built-in functions >> + */ >> +struct function { >> + const char *name; >> + unsigned int min_args; >> + unsigned int max_args; >> + bool expand_args; >> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]); >> +}; > If a typedef was provided for the function then ... Yes, I can do this, but I may rather consider to simplify the code. >> + >> +static const struct function function_table[] = { >> + /* Name MIN MAX EXP? Function */ >> +}; >> + >> +#define FUNCTION_MAX_ARGS 16 >> + >> +static char *function_expand_arg_and_call(char *(*func)(int argc, char *argv[], >> + int old_argc, >> + char *old_argv[]), >> + int argc, char *argv[], >> + int old_argc, char *old_argv[]) > this could be much easier to read. > >> +{ >> + char *expanded_argv[FUNCTION_MAX_ARGS], *res; >> + int i; >> + >> + for (i = 0; i < argc; i++) >> + expanded_argv[i] = expand_string_with_args(argv[i], >> + old_argc, old_argv); > > No check for too many arguments here - maybe it is done in some other place. Right. This has already been checked by eval_clause(). >> + >> + res = func(argc, expanded_argv, 0, NULL); >> + >> + for (i = 0; i < argc; i++) >> + free(expanded_argv[i]); >> + >> + return res; >> +} >> + >> +static char *function_call(const char *name, int argc, char *argv[], >> + int old_argc, char *old_argv[]) >> +{ >> + const struct function *f; >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(function_table); i++) { >> + f = &function_table[i]; >> + if (strcmp(f->name, name)) >> + continue; >> + >> + if (argc < f->min_args) >> + pperror("too few function arguments passed to '%s'", >> + name); >> + >> + if (argc > f->max_args) >> + pperror("too many function arguments passed to '%s'", >> + name); > Number of arguments checked here, but max_args is not assiged in this patch. This is added to function_table[] by later patches. > >> + >> + if (f->expand_args) >> + return function_expand_arg_and_call(f->func, argc, argv, >> + old_argc, old_argv); >> + return f->func(argc, argv, old_argc, old_argv); >> + } >> + >> + return NULL; >> +} >> + >> +/* >> + * Evaluate a clause with arguments. argc/argv are arguments from the upper >> + * function call. >> + * >> + * Returned string must be freed when done >> + */ >> +static char *eval_clause(const char *in, int argc, char *argv[]) >> { >> - char *res, *name; >> + char *tmp, *prev, *p, *res, *name; >> + int new_argc = 0; >> + char *new_argv[FUNCTION_MAX_ARGS]; >> + int nest = 0; >> + int i; >> >> /* >> * Returns an empty string because '$()' should be evaluated >> @@ -99,10 +180,69 @@ static char *eval_clause(const char *in) >> if (!*in) >> return xstrdup(""); >> >> - name = expand_string(in); >> + tmp = xstrdup(in); >> + >> + prev = p = tmp; >> + >> + /* >> + * Split into tokens >> + * The function name and arguments are separated by a comma. >> + * For example, if the function call is like this: >> + * $(foo,abc,$(x),$(y)) >> + * >> + * The input string for this helper should be: >> + * foo,abc,$(x),$(y) >> + * >> + * and split into: >> + * new_argv[0] = 'foo' >> + * new_argv[1] = 'abc' >> + * new_argv[2] = '$(x)' >> + * new_argv[3] = '$(y)' >> + */ >> + while (*p) { >> + if (nest == 0 && *p == ',') { >> + *p = 0; >> + if (new_argc >= FUNCTION_MAX_ARGS) >> + pperror("too many function arguments"); >> + new_argv[new_argc++] = prev; >> + prev = p + 1; >> + } else if (*p == '(') { >> + nest++; >> + } else if (*p == ')') { >> + nest--; >> + } >> + >> + p++; >> + } >> + new_argv[new_argc++] = prev; > > Will the following be equal: > > $(foo,abc,$(x),$(y)) > $(foo, abc, $(x), $(y)) > > make is rather annoying as space is significant, but there seems no good reason > for kconfig to inheritate this. > So unless there are good arguments consider alloing the spaces. > If the current implmentation already supports optional spaces then I just missed > it whie reviewing. I have been thinking of trimming the leading whitespaces. (https://patchwork.kernel.org/patch/10405549/) This is trade-off vs "how to pass spaces as arguments?" GNU Make trims any leading whitespaces from the first argument. At least, it was tedious to print messages with indentation. $(info Tool information:) $(info CC: $(CC)) $(info LD: $(LD)) will print Tool information: CC: gcc LD: ld To keep the indentation, I need to do like follows: $(info Tool information:) $(info $(space)$(space)CC: $(CC)) $(info $(space)$(space)LD: $(LD)) If this is acceptable limitation, yes, I can do this.
Hi Masahiro > >> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]); > >> +}; > > If a typedef was provided for the function then ... > > > Yes, I can do this, > but I may rather consider to simplify the code. Simplify is better. > > Will the following be equal: > > > > $(foo,abc,$(x),$(y)) > > $(foo, abc, $(x), $(y)) > > > > make is rather annoying as space is significant, but there seems no good reason > > for kconfig to inheritate this. > > So unless there are good arguments consider alloing the spaces. > > If the current implmentation already supports optional spaces then I just missed > > it whie reviewing. > > > I have been thinking of trimming the leading whitespaces. > (https://patchwork.kernel.org/patch/10405549/) > > This is trade-off vs "how to pass spaces as arguments?" Maybe allow strings to be passed enclosed in ""? Then it is simple to add whitespace. But the use of "" should be optional in all other cases. And the "" should be stripped. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sam, 2018-05-21 15:16 GMT+09:00 Sam Ravnborg <sam@ravnborg.org>: > Hi Masahiro > >> >> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]); >> >> +}; >> > If a typedef was provided for the function then ... >> >> >> Yes, I can do this, >> but I may rather consider to simplify the code. > > Simplify is better. > >> > Will the following be equal: >> > >> > $(foo,abc,$(x),$(y)) >> > $(foo, abc, $(x), $(y)) >> > >> > make is rather annoying as space is significant, but there seems no good reason >> > for kconfig to inheritate this. >> > So unless there are good arguments consider alloing the spaces. >> > If the current implmentation already supports optional spaces then I just missed >> > it whie reviewing. >> >> >> I have been thinking of trimming the leading whitespaces. >> (https://patchwork.kernel.org/patch/10405549/) >> >> This is trade-off vs "how to pass spaces as arguments?" > > Maybe allow strings to be passed enclosed in ""? > Then it is simple to add whitespace. > > But the use of "" should be optional in all other cases. > And the "" should be stripped. > Hmm, your suggestion is more shell-oriented parsing. In Make, there is no concept of quoting because it does not touch single-quote, double-quote, whitespaces etc. at all. $(info "'@@"'' '" ' "' ) will print the message as they are. This simplifies both the grammar and the parser implementation. If we expand the quoting by Kconfig, we need more careful consideration. [1] In the following, would "hello world" be expanded by Kconfig or by shell? $(shell, echo "hello world") [2] Is a quoted comma delimiter or not? $(if,",",$(A)) If remember I first examined shell-oriented expansion, but I stopped. Probably, I found it was much more complex, then I chose Make-like simpler one.
Hi Masahiro, > Hmm, your suggestion is more shell-oriented parsing. ... > Probably, I found it was much more complex, > then I chose Make-like simpler one. Agree that the simple one should be the best choice for now. We shall not add anything more complex unless it is really needed. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: > Will the following be equal: > > $(foo,abc,$(x),$(y)) > $(foo, abc, $(x), $(y)) > > make is rather annoying as space is significant, but there seems no good reason > for kconfig to inheritate this. > So unless there are good arguments consider alloing the spaces. > If the current implmentation already supports optional spaces then I just missed > it whie reviewing. > > Sam +1 from me. I also find the rules for whitespace in Make confusing, and always have to look them up when doing trickier stuff. Maybe they're the result of people not considering whitespace initially, and stuff getting tacked on later. GNU Make adds some alternate syntaxes with quotes. I was going to mention shell, but it looks like you already did. :) If we go with Make-like syntax, maybe we could at least have a variant with fewer whitespace gotchas. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: >> Will the following be equal: >> >> $(foo,abc,$(x),$(y)) >> $(foo, abc, $(x), $(y)) >> >> make is rather annoying as space is significant, but there seems no good reason >> for kconfig to inheritate this. >> So unless there are good arguments consider alloing the spaces. >> If the current implmentation already supports optional spaces then I just missed >> it whie reviewing. >> >> Sam > > +1 from me. > > I also find the rules for whitespace in Make confusing, and always > have to look them up when doing trickier stuff. Maybe they're the > result of people not considering whitespace initially, and stuff > getting tacked on later. GNU Make adds some alternate syntaxes with > quotes. > > I was going to mention shell, but it looks like you already did. :) > > If we go with Make-like syntax, maybe we could at least have a variant > with fewer whitespace gotchas. > > Cheers, > Ulf Maybe it'd be a pain to implement, but something like $(foo $(x) "two words" "interpolated $(stuff)") seems pretty nice, with three arguments there. For variables too: x = foo y = "two words" Or have mandatory quotes, but yeah, bit spammy there maybe. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: >>> Will the following be equal: >>> >>> $(foo,abc,$(x),$(y)) >>> $(foo, abc, $(x), $(y)) >>> >>> make is rather annoying as space is significant, but there seems no good reason >>> for kconfig to inheritate this. >>> So unless there are good arguments consider alloing the spaces. >>> If the current implmentation already supports optional spaces then I just missed >>> it whie reviewing. >>> >>> Sam >> >> +1 from me. >> >> I also find the rules for whitespace in Make confusing, and always >> have to look them up when doing trickier stuff. Maybe they're the >> result of people not considering whitespace initially, and stuff >> getting tacked on later. GNU Make adds some alternate syntaxes with >> quotes. >> >> I was going to mention shell, but it looks like you already did. :) >> >> If we go with Make-like syntax, maybe we could at least have a variant >> with fewer whitespace gotchas. >> >> Cheers, >> Ulf > > Maybe it'd be a pain to implement, but something like $(foo $(x) "two > words" "interpolated $(stuff)") seems pretty nice, with three > arguments there. Guess that might interact poorly with $(shell foo "bar baz") though. Kinda nice to have a syntax that doesn't overlap with shell when building shell commands. Still wondering if you could get rid of some of the Make gotchas without losing other stuff... Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-05-22 0:10 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: >>>> Will the following be equal: >>>> >>>> $(foo,abc,$(x),$(y)) >>>> $(foo, abc, $(x), $(y)) >>>> >>>> make is rather annoying as space is significant, but there seems no good reason >>>> for kconfig to inheritate this. >>>> So unless there are good arguments consider alloing the spaces. >>>> If the current implmentation already supports optional spaces then I just missed >>>> it whie reviewing. >>>> >>>> Sam >>> >>> +1 from me. >>> >>> I also find the rules for whitespace in Make confusing, and always >>> have to look them up when doing trickier stuff. Maybe they're the >>> result of people not considering whitespace initially, and stuff >>> getting tacked on later. GNU Make adds some alternate syntaxes with >>> quotes. >>> >>> I was going to mention shell, but it looks like you already did. :) >>> >>> If we go with Make-like syntax, maybe we could at least have a variant >>> with fewer whitespace gotchas. >>> >>> Cheers, >>> Ulf >> >> Maybe it'd be a pain to implement, but something like $(foo $(x) "two >> words" "interpolated $(stuff)") seems pretty nice, with three >> arguments there. > > Guess that might interact poorly with $(shell foo "bar baz") though. > Kinda nice to have a syntax that doesn't overlap with shell when > building shell commands. Right. I can easily imagine that would end up with more gotchas due to quoting and escaping. > Still wondering if you could get rid of some of the Make gotchas > without losing other stuff... > > Cheers, > Ulf
On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-05-22 0:10 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >>>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: >>>>> Will the following be equal: >>>>> >>>>> $(foo,abc,$(x),$(y)) >>>>> $(foo, abc, $(x), $(y)) >>>>> >>>>> make is rather annoying as space is significant, but there seems no good reason >>>>> for kconfig to inheritate this. >>>>> So unless there are good arguments consider alloing the spaces. >>>>> If the current implmentation already supports optional spaces then I just missed >>>>> it whie reviewing. >>>>> >>>>> Sam >>>> >>>> +1 from me. >>>> >>>> I also find the rules for whitespace in Make confusing, and always >>>> have to look them up when doing trickier stuff. Maybe they're the >>>> result of people not considering whitespace initially, and stuff >>>> getting tacked on later. GNU Make adds some alternate syntaxes with >>>> quotes. >>>> >>>> I was going to mention shell, but it looks like you already did. :) >>>> >>>> If we go with Make-like syntax, maybe we could at least have a variant >>>> with fewer whitespace gotchas. >>>> >>>> Cheers, >>>> Ulf >>> >>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two >>> words" "interpolated $(stuff)") seems pretty nice, with three >>> arguments there. >> >> Guess that might interact poorly with $(shell foo "bar baz") though. >> Kinda nice to have a syntax that doesn't overlap with shell when >> building shell commands. > > > Right. I can easily imagine > that would end up with more gotchas due to quoting and escaping. Yeah, you're right. It's probably trying to fix something that isn't broken. Make's syntax really isn't bad there, just slightly non-obvious at first... Think it's fine now. Better to commit to the syntax than trying to be "helpful" by adding a bunch of random exceptions too. That probably gives a bigger mess in the end... Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 22, 2018 at 6:50 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2018-05-22 0:10 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >>> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >>>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >>>>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg <sam@ravnborg.org> wrote: >>>>>> Will the following be equal: >>>>>> >>>>>> $(foo,abc,$(x),$(y)) >>>>>> $(foo, abc, $(x), $(y)) >>>>>> >>>>>> make is rather annoying as space is significant, but there seems no good reason >>>>>> for kconfig to inheritate this. >>>>>> So unless there are good arguments consider alloing the spaces. >>>>>> If the current implmentation already supports optional spaces then I just missed >>>>>> it whie reviewing. >>>>>> >>>>>> Sam >>>>> >>>>> +1 from me. >>>>> >>>>> I also find the rules for whitespace in Make confusing, and always >>>>> have to look them up when doing trickier stuff. Maybe they're the >>>>> result of people not considering whitespace initially, and stuff >>>>> getting tacked on later. GNU Make adds some alternate syntaxes with >>>>> quotes. >>>>> >>>>> I was going to mention shell, but it looks like you already did. :) >>>>> >>>>> If we go with Make-like syntax, maybe we could at least have a variant >>>>> with fewer whitespace gotchas. >>>>> >>>>> Cheers, >>>>> Ulf >>>> >>>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two >>>> words" "interpolated $(stuff)") seems pretty nice, with three >>>> arguments there. >>> >>> Guess that might interact poorly with $(shell foo "bar baz") though. >>> Kinda nice to have a syntax that doesn't overlap with shell when >>> building shell commands. >> >> >> Right. I can easily imagine >> that would end up with more gotchas due to quoting and escaping. > > Yeah, you're right. It's probably trying to fix something that isn't > broken. Make's syntax really isn't bad there, just slightly > non-obvious at first... > > Think it's fine now. Better to commit to the syntax than trying to be > "helpful" by adding a bunch of random exceptions too. That probably > gives a bigger mess in the end... > > Cheers, > Ulf I'm fine with the comma-after-function-name change though. That just makes it more consistent. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c index 1bf506c..5be28ec 100644 --- a/scripts/kconfig/preprocess.c +++ b/scripts/kconfig/preprocess.c @@ -3,12 +3,17 @@ // Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com> #include <stdarg.h> +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include "list.h" +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) + +static char *expand_string_with_args(const char *in, int argc, char *argv[]); + static void __attribute__((noreturn)) pperror(const char *format, ...) { va_list ap; @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name) } } -static char *eval_clause(const char *in) +/* + * Built-in functions + */ +struct function { + const char *name; + unsigned int min_args; + unsigned int max_args; + bool expand_args; + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]); +}; + +static const struct function function_table[] = { + /* Name MIN MAX EXP? Function */ +}; + +#define FUNCTION_MAX_ARGS 16 + +static char *function_expand_arg_and_call(char *(*func)(int argc, char *argv[], + int old_argc, + char *old_argv[]), + int argc, char *argv[], + int old_argc, char *old_argv[]) +{ + char *expanded_argv[FUNCTION_MAX_ARGS], *res; + int i; + + for (i = 0; i < argc; i++) + expanded_argv[i] = expand_string_with_args(argv[i], + old_argc, old_argv); + + res = func(argc, expanded_argv, 0, NULL); + + for (i = 0; i < argc; i++) + free(expanded_argv[i]); + + return res; +} + +static char *function_call(const char *name, int argc, char *argv[], + int old_argc, char *old_argv[]) +{ + const struct function *f; + int i; + + for (i = 0; i < ARRAY_SIZE(function_table); i++) { + f = &function_table[i]; + if (strcmp(f->name, name)) + continue; + + if (argc < f->min_args) + pperror("too few function arguments passed to '%s'", + name); + + if (argc > f->max_args) + pperror("too many function arguments passed to '%s'", + name); + + if (f->expand_args) + return function_expand_arg_and_call(f->func, argc, argv, + old_argc, old_argv); + return f->func(argc, argv, old_argc, old_argv); + } + + return NULL; +} + +/* + * Evaluate a clause with arguments. argc/argv are arguments from the upper + * function call. + * + * Returned string must be freed when done + */ +static char *eval_clause(const char *in, int argc, char *argv[]) { - char *res, *name; + char *tmp, *prev, *p, *res, *name; + int new_argc = 0; + char *new_argv[FUNCTION_MAX_ARGS]; + int nest = 0; + int i; /* * Returns an empty string because '$()' should be evaluated @@ -99,10 +180,69 @@ static char *eval_clause(const char *in) if (!*in) return xstrdup(""); - name = expand_string(in); + tmp = xstrdup(in); + + prev = p = tmp; + + /* + * Split into tokens + * The function name and arguments are separated by a comma. + * For example, if the function call is like this: + * $(foo,abc,$(x),$(y)) + * + * The input string for this helper should be: + * foo,abc,$(x),$(y) + * + * and split into: + * new_argv[0] = 'foo' + * new_argv[1] = 'abc' + * new_argv[2] = '$(x)' + * new_argv[3] = '$(y)' + */ + while (*p) { + if (nest == 0 && *p == ',') { + *p = 0; + if (new_argc >= FUNCTION_MAX_ARGS) + pperror("too many function arguments"); + new_argv[new_argc++] = prev; + prev = p + 1; + } else if (*p == '(') { + nest++; + } else if (*p == ')') { + nest--; + } + + p++; + } + new_argv[new_argc++] = prev; - res = env_expand(name); + /* + * Shift arguments + * new_argv[0] represents a function name or a variable name. Put it + * into 'name', then shift the rest of the arguments. This simplifies + * 'const' handling. + */ + name = expand_string_with_args(new_argv[0], argc, argv); + new_argc--; + for (i = 0; i < new_argc; i++) + new_argv[i] = new_argv[i + 1]; + + /* Look for built-in functions */ + res = function_call(name, new_argc, new_argv, argc, argv); + if (res) + goto out; + + /* Last, try environment variable */ + if (new_argc == 0) { + res = env_expand(name); + if (res) + goto out; + } + + res = xstrdup(""); +out: free(name); + free(tmp); return res; } @@ -119,7 +259,7 @@ static char *eval_clause(const char *in) * will be * $(BAR) */ -char *expand_dollar(const char **str) +static char *expand_dollar_with_args(const char **str, int argc, char *argv[]) { const char *p = *str; const char *q; @@ -158,18 +298,18 @@ char *expand_dollar(const char **str) memcpy(tmp, p, q - p); tmp[q - p] = 0; *str = q + 1; - out = eval_clause(tmp); + out = eval_clause(tmp, argc, argv); free(tmp); return out; } /* - * Expand variables in the given string. Undefined variables + * Expand variables, functions, etc. in the given string. Undefined variables * expand to an empty string. * The returned string must be freed when done. */ -char *expand_string(const char *in) +static char *expand_string_with_args(const char *in, int argc, char *argv[]) { const char *prev_in, *p; char *new, *out; @@ -181,7 +321,7 @@ char *expand_string(const char *in) while ((p = strchr(in, '$'))) { prev_in = in; in = p + 1; - new = expand_dollar(&in); + new = expand_dollar_with_args(&in, argc, argv); outlen = strlen(out) + (p - prev_in) + strlen(new) + 1; out = xrealloc(out, outlen); strncat(out, prev_in, p - prev_in); @@ -196,6 +336,16 @@ char *expand_string(const char *in) return out; } +char *expand_string(const char *in) +{ + return expand_string_with_args(in, 0, NULL); +} + +char *expand_dollar(const char **str) +{ + return expand_dollar_with_args(str, 0, NULL); +} + /* * Expand variables in a token. The parsing stops when a token separater * (in most cases, it is a whitespace) is encountered. 'str' is updated to
This commit adds a new concept 'function' to do more text processing in Kconfig. A function call looks like this: $(function,arg1,arg2,arg3,...) This commit adds the basic infrastructure to expand functions. Change the text expansion helpers to take arguments. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v4: - Error out if arguments more than FUNCTION_MAX_ARGS are passed - Use a comma as a delimiter between the function name and the first argument - Check the number of arguments accepted by each function - Support delayed expansion of arguments. This will be needed to implement 'if' function Changes in v3: - Split base infrastructure and 'shell' function into separate patches. Changes in v2: - Use 'shell' for getting stdout from the comment. It was 'shell-stdout' in the previous version. - Simplify the implementation since the expansion has been moved to lexer. scripts/kconfig/preprocess.c | 168 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 159 insertions(+), 9 deletions(-)