| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 May 2018 14:18:06 +0900
From: Masahiro Yamada <yamada.masahiro@...ionext.com>
To: Sam Ravnborg <sam@...nborg.org>
Cc: Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Ulf Magnusson <ulfalizer@...il.com>,
"Luis R . Rodriguez" <mcgrof@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Nicholas Piggin <npiggin@...il.com>,
Kees Cook <keescook@...omium.org>,
Emese Revfy <re.emese@...il.com>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH v4 07/31] kconfig: add built-in function support
Sam,
2018-05-20 23:50 GMT+09:00 Sam Ravnborg <sam@...nborg.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@...ionext.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@...ionext.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.
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists