Sam, 2018-05-20 23:50 GMT+09:00 Sam Ravnborg <sam@xxxxxxxxxxxx>: > 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@xxxxxxxxxxxxx> >> --- >> >> 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@xxxxxxxxxxxxx> >> >> #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. -- 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