Received: by 10.223.185.116 with SMTP id b49csp4032854wrg; Mon, 19 Feb 2018 09:53:29 -0800 (PST) X-Google-Smtp-Source: AH8x225QBpGZTIse8nSTA6rysetF1/HFimu5sMboH3249PT5KB5bMHeeXuLyz34LrUHy9CFNVcoo X-Received: by 10.98.159.200 with SMTP id v69mr15305580pfk.236.1519062809729; Mon, 19 Feb 2018 09:53:29 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519062809; cv=none; d=google.com; s=arc-20160816; b=JKp+bV/PyjP1MKF8IhfQUTJUokJDjLO+BaLozQQUHnNleld8GhIGM+SyJ1lrtCjAds 6n+0zZRmAAEk1nbDoQAlSU30oS27VDymdzIX7attjZjrpnmqerDGibnyv7KI1XrIOzaU z9ZmroH9Y4h7bQJ/+ayN8x3YCMaVh6++Rfl+9jJEGbDLh8B3dniFFYaaBqIRcUKGJ1xv mqIp+colOYmMRQ69RQu3dkLblmbJplkBbwh7tuVIz0+6RoKWC3OMay2+rzG66QqBeVYZ FFltlDS89VNMRy4BMNVa8PXRQe5m25KMwDAAgkhwnEW2qpnEG87VduTLbCJHscVj+s+3 6AMw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature:arc-authentication-results; bh=cg2L8UmH8kH+EMdfN86dzx2uOR9WGXDndIh0kccVaXc=; b=xSDr228eTvEINLF3mzCnnApURRXmydAJ3lbr2X1OxEd79tSH4ToSfD91O/YoWsUnkM h1gBEDoiUEqHKwzyWk52nXqg989WF+pLWC8FjWJa88jV70KCovgWrIU9/33I7SHDc3dS mGJQoRt9iT+Br6YpcXpJfxWCZtIlSIKkSL2KTr7DvYmuS9N81kRDAgYpw4jSFAKEKnDu DZfGAI6aOGAjEwmj9rRpT0uNoyeRs/Tsw033YZ2ObwrkSTUvP3khGFDDHtSs224Myrq7 scgR0b1BqkUiI92APfvoODptrfYTK2R7C4mQNYhU9oOQmAt3NE0SSZZjjMFYt4vBnieb 7Taw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=GGR9LheY; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t25si6896200pge.368.2018.02.19.09.53.15; Mon, 19 Feb 2018 09:53:29 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=GGR9LheY; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753440AbeBSRvH (ORCPT + 99 others); Mon, 19 Feb 2018 12:51:07 -0500 Received: from mail-lf0-f47.google.com ([209.85.215.47]:46756 "EHLO mail-lf0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753342AbeBSRvE (ORCPT ); Mon, 19 Feb 2018 12:51:04 -0500 Received: by mail-lf0-f47.google.com with SMTP id r80so575916lfe.13; Mon, 19 Feb 2018 09:51:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=cg2L8UmH8kH+EMdfN86dzx2uOR9WGXDndIh0kccVaXc=; b=GGR9LheYLvOYDbhkzLWefbYlNe3VLjWJGzPAgv+ym0ocUvq+FkSi/byxH3pibPdVpC LZUJfMzKeZZ5/ElGVEwV8EPubVvIWv3WmPtOo4mWcWItJ/y+Qm4E+fI/oveoJffhatnv CXNmqeojBEPhfK4EVowTP5kMXlGqlafGlqQap/CQKy87crG4LY+IQWiKceM+D5gxipEy C+YClyjV61o4baRRAs0FiXF8FkBrns/OIDI14s8KpjJ8TcM7MZMVVOFJPspVDicKbUew JJnnOmdGiyyvjpMCZO9r1Mti/eidQ+s7DZnDDpXOjShl1D07ths5PZf63/kNFqjf5o6q swvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=cg2L8UmH8kH+EMdfN86dzx2uOR9WGXDndIh0kccVaXc=; b=fIkFMdNi3pNU3I5W+juX4dZwBGHZo21o4AyxRHautIU6v8SG8LvToTjCKc0r6utScB 280+kWj2CMLFSGcJxtdX482ADSupDdb6lVVyN7cL0huis2tlwCB4lxbB9lUfQcC1JdMn bJOo/53/75DQTE+ycRUKo2tKKo2SuGlUD7tR8DtUgSFES9c/q1cXKsAIE6nuDFVugC7O 22PxBM4zAnbt4vOWhO8FGCgLQEOiXhPpA9xLcNLROjEIEGuu1BTfjLwZ6LdJ+p/y8uf8 hXAbzOT0BWoGRnZA2kyLFEtLn3t8IpRLBedB0L6fDYDlsqA3Cz5YZUAU6mtQrBekAiOE Bqbg== X-Gm-Message-State: APf1xPCn1j9/VUDLvn7TcTT+zGNfbVIOmT1KuM5Up/IZmSG/a0wRo/Jh +IKglL9+VFlSWb0b3e8DQ+w= X-Received: by 10.25.232.17 with SMTP id f17mr11154722lfh.55.1519062662263; Mon, 19 Feb 2018 09:51:02 -0800 (PST) Received: from huvuddator (ua-213-113-106-221.cust.bredbandsbolaget.se. [213.113.106.221]) by smtp.gmail.com with ESMTPSA id x69sm2317075ljb.50.2018.02.19.09.51.01 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 19 Feb 2018 09:51:01 -0800 (PST) Date: Mon, 19 Feb 2018 18:50:59 +0100 From: Ulf Magnusson To: Masahiro Yamada Cc: Linux Kbuild mailing list , Linus Torvalds , Greg Kroah-Hartman , Arnd Bergmann , Kees Cook , Randy Dunlap , Sam Ravnborg , Michal Marek , Linux Kernel Mailing List Subject: Re: [PATCH 07/23] kconfig: add function support and implement 'shell' function Message-ID: <20180219175059.iirtcd2pugmbje7f@huvuddator> References: <1518806331-7101-1-git-send-email-yamada.masahiro@socionext.com> <1518806331-7101-8-git-send-email-yamada.masahiro@socionext.com> <20180217161636.h6miw4xhu2mfjzvd@huvuddator> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 20, 2018 at 12:57:13AM +0900, Masahiro Yamada wrote: > 2018-02-18 1:16 GMT+09:00 Ulf Magnusson : > > On Sat, Feb 17, 2018 at 03:38:35AM +0900, Masahiro Yamada wrote: > >> This commit adds a new concept 'function' to Kconfig. A function call > >> resembles a variable reference with arguments, and looks like this: > >> > >> $(function arg1, arg2, arg3, ...) > >> > >> (Actually, this syntax was inspired by Makefile.) > >> > >> Real examples will look like this: > >> > >> $(shell true) > >> $(cc-option -fstackprotector) > >> > >> This commit adds the basic infrastructure to add, delete, evaluate > >> functions. > >> > >> Also, add the first built-in function $(shell ...). This evaluates > >> to 'y' if the given command exits with 0, 'n' otherwise. > >> > >> I am also planning to support user-defined functions (a.k.a 'macro') > >> for cases where hard-coding is not preferred. > >> > >> If you want to try this feature, the hello-world code is someting below. > >> > >> Example code: > >> > >> config CC_IS_GCC > >> bool > >> default $(shell $CC --version | grep -q gcc) > >> > >> config CC_IS_CLANG > >> bool > >> default $(shell $CC --version | grep -q clang) > >> > >> config CC_HAS_OZ > >> bool > >> default $(shell $CC -Werror -Oz -c -x c /dev/null -o /dev/null) > >> > >> Result: > >> > >> $ make -s alldefconfig && tail -n 3 .config > >> CONFIG_CC_IS_GCC=y > >> # CONFIG_CC_IS_CLANG is not set > >> # CONFIG_CC_HAS_OZ is not set > >> > >> $ make CC=clang -s alldefconfig && tail -n 3 .config > >> # CONFIG_CC_IS_GCC is not set > >> CONFIG_CC_IS_CLANG=y > >> CONFIG_CC_HAS_OZ=y > >> > >> A function call can appear anywhere a symbol reference can appear. > >> So, the following code is possible. > >> > >> Example code: > >> > >> config CC_NAME > >> string > >> default "gcc" if $(shell $CC --version | grep -q gcc) > >> default "clang" if $(shell $CC --version | grep -q clang) > >> default "unknown compiler" > >> > >> Result: > >> > >> $ make -s alldefconfig && tail -n 1 .config > >> CONFIG_CC_NAME="gcc" > >> > >> $ make CC=clang -s alldefconfig && tail -n 1 .config > >> CONFIG_CC_NAME="clang" > >> > >> Signed-off-by: Masahiro Yamada > >> --- > >> > >> Reminder for myself: > >> Update Documentation/kbuild/kconfig-language.txt > >> > >> > >> scripts/kconfig/function.c | 149 ++++++++++++++++++++++++++++++++++++++++++++ > >> scripts/kconfig/lkc_proto.h | 5 ++ > >> scripts/kconfig/util.c | 46 +++++++++++--- > >> scripts/kconfig/zconf.l | 38 ++++++++++- > >> scripts/kconfig/zconf.y | 9 +++ > >> 5 files changed, 238 insertions(+), 9 deletions(-) > >> create mode 100644 scripts/kconfig/function.c > >> > >> diff --git a/scripts/kconfig/function.c b/scripts/kconfig/function.c > >> new file mode 100644 > >> index 0000000..60e59be > >> --- /dev/null > >> +++ b/scripts/kconfig/function.c > >> @@ -0,0 +1,149 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +// > >> +// Copyright (C) 2018 Masahiro Yamada > >> + > >> +#include > >> +#include > >> +#include > >> + > >> +#include "list.h" > >> + > >> +#define FUNCTION_MAX_ARGS 10 > >> + > >> +static LIST_HEAD(function_list); > >> + > >> +struct function { > >> + const char *name; > >> + char *(*func)(int argc, char *argv[]); > >> + struct list_head node; > >> +}; > >> + > >> +static struct function *func_lookup(const char *name) > >> +{ > >> + struct function *f; > >> + > >> + list_for_each_entry(f, &function_list, node) { > >> + if (!strcmp(name, f->name)) > >> + return f; > >> + } > >> + > >> + return NULL; > >> +} > >> + > >> +static void func_add(const char *name, char *(*func)(int argc, char *argv[])) > >> +{ > >> + struct function *f; > >> + > >> + f = func_lookup(name); > >> + if (f) { > >> + fprintf(stderr, "%s: function already exists. ignored.\n", name); > >> + return; > >> + } > >> + > >> + f = xmalloc(sizeof(*f)); > >> + f->name = name; > >> + f->func = func; > >> + > >> + list_add_tail(&f->node, &function_list); > >> +} > >> + > >> +static void func_del(struct function *f) > >> +{ > >> + list_del(&f->node); > >> + free(f); > >> +} > >> + > >> +static char *func_call(int argc, char *argv[]) > >> +{ > >> + struct function *f; > >> + > >> + f = func_lookup(argv[0]); > >> + if (!f) { > >> + fprintf(stderr, "%s: function not found\n", argv[0]); > >> + return NULL; > >> + } > >> + > >> + return f->func(argc, argv); > >> +} > >> + > >> +static char *func_eval(const char *func) > >> +{ > >> + char *expanded, *saveptr, *str, *token, *res; > >> + const char *delim; > >> + int argc = 0; > >> + char *argv[FUNCTION_MAX_ARGS]; > >> + > >> + expanded = expand_string_value(func); > >> + > >> + str = expanded; > >> + delim = " "; > >> + > >> + while ((token = strtok_r(str, delim, &saveptr))) { > >> + argv[argc++] = token; > >> + str = NULL; > >> + delim = ","; > >> + } > >> + > >> + res = func_call(argc, argv); > >> + > >> + free(expanded); > >> + > >> + return res ?: xstrdup(""); > >> +} > >> + > >> +char *func_eval_n(const char *func, size_t n) > >> +{ > >> + char *tmp, *res; > >> + > >> + tmp = xmalloc(n + 1); > >> + memcpy(tmp, func, n); > >> + *(tmp + n) = '\0'; > >> + > >> + res = func_eval(tmp); > >> + > >> + free(tmp); > >> + > >> + return res; > >> +} > >> + > >> +/* built-in functions */ > >> +static char *do_shell(int argc, char *argv[]) > >> +{ > >> + static const char *pre = "("; > >> + static const char *post = ") >/dev/null 2>&1"; > >> + char *cmd; > >> + int ret; > >> + > >> + if (argc != 2) > >> + return NULL; > >> + > >> + /* > >> + * Surround the command with ( ) in case it is piped commands. > >> + * Also, redirect stdout and stderr to /dev/null. > >> + */ > >> + cmd = xmalloc(strlen(pre) + strlen(argv[1]) + strlen(post) + 1); > >> + strcpy(cmd, pre); > >> + strcat(cmd, argv[1]); > >> + strcat(cmd, post); > >> + > >> + ret = system(cmd); > >> + > >> + free(cmd); > >> + > >> + return xstrdup(ret == 0 ? "y" : "n"); > >> +} > >> + > >> +void func_init(void) > >> +{ > >> + /* register built-in functions */ > >> + func_add("shell", do_shell); > >> +} > >> + > >> +void func_exit(void) > >> +{ > >> + struct function *f, *tmp; > >> + > >> + /* unregister all functions */ > >> + list_for_each_entry_safe(f, tmp, &function_list, node) > >> + func_del(f); > >> +} > >> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h > >> index 9884adc..09a4f53 100644 > >> --- a/scripts/kconfig/lkc_proto.h > >> +++ b/scripts/kconfig/lkc_proto.h > >> @@ -48,5 +48,10 @@ const char * sym_get_string_value(struct symbol *sym); > >> > >> const char * prop_get_type_name(enum prop_type type); > >> > >> +/* function.c */ > >> +char *func_eval_n(const char *func, size_t n); > >> +void func_init(void); > >> +void func_exit(void); > >> + > >> /* expr.c */ > >> void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken); > >> diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c > >> index dddf85b..ed89fb9 100644 > >> --- a/scripts/kconfig/util.c > >> +++ b/scripts/kconfig/util.c > >> @@ -93,9 +93,10 @@ static char *env_expand_n(const char *name, size_t n) > >> } > >> > >> /* > >> - * Expand environments embedded in the string given in argument. Environments > >> - * to be expanded shall be prefixed by a '$'. Unknown environment expands to > >> - * the empty string. > >> + * Expand environments and functions embedded in the string given in argument. > >> + * Environments to be expanded shall be prefixed by a '$'. Functions to be > >> + * evaluated shall be surrounded by $(). Unknown environment/function expands > >> + * to the empty string. > >> */ > >> char *expand_string_value(const char *in) > >> { > >> @@ -113,11 +114,40 @@ char *expand_string_value(const char *in) > >> while ((p = strchr(in, '$'))) { > >> char *new; > >> > >> - q = p + 1; > >> - while (isalnum(*q) || *q == '_') > >> - q++; > >> - > >> - new = env_expand_n(p + 1, q - p - 1); > >> + /* > >> + * If the next character is '(', it is a function. > >> + * Otherwise, environment. > >> + */ > >> + if (*(p + 1) == '(') { > >> + int nest = 0; > >> + > >> + q = p + 2; > >> + while (1) { > >> + if (*q == '\0') { > >> + fprintf(stderr, > >> + "unterminated function: %s\n", > >> + p); > >> + new = xstrdup(""); > >> + break; > >> + } else if (*q == '(') { > >> + nest++; > >> + } else if (*q == ')') { > >> + if (nest-- == 0) { > >> + new = func_eval_n(p + 2, > >> + q - p - 2); > >> + q++; > >> + break; > >> + } > >> + } > >> + q++; > >> + } > >> + } else { > >> + q = p + 1; > >> + while (isalnum(*q) || *q == '_') > >> + q++; > >> + > >> + new = env_expand_n(p + 1, q - p - 1); > >> + } > >> > >> reslen = strlen(res) + (p - in) + strlen(new) + 1; > >> res = xrealloc(res, reslen); > >> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l > >> index 0d89ea6..f433ab0 100644 > >> --- a/scripts/kconfig/zconf.l > >> +++ b/scripts/kconfig/zconf.l > >> @@ -1,7 +1,7 @@ > >> %option nostdinit noyywrap never-interactive full ecs > >> %option 8bit nodefault perf-report perf-report > >> %option noinput > >> -%x COMMAND HELP STRING PARAM > >> +%x COMMAND HELP STRING PARAM FUNCTION > >> %{ > >> /* > >> * Copyright (C) 2002 Roman Zippel > >> @@ -25,6 +25,7 @@ static struct { > >> > >> static char *text; > >> static int text_size, text_asize; > >> +static int function_nest; > >> > >> struct buffer { > >> struct buffer *parent; > >> @@ -138,6 +139,12 @@ n [$A-Za-z0-9_-] > >> new_string(); > >> BEGIN(STRING); > >> } > >> + "$(" { > >> + new_string(); > >> + append_string(yytext, yyleng); > >> + function_nest = 0; > >> + BEGIN(FUNCTION); > >> + } > >> \n BEGIN(INITIAL); current_file->lineno++; return T_EOL; > >> ({n}|[/.])+ { > >> const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); > >> @@ -196,6 +203,35 @@ n [$A-Za-z0-9_-] > >> } > >> } > >> > >> +{ > >> + [^()\n]* { > >> + append_string(yytext, yyleng); > >> + } > >> + "(" { > >> + append_string(yytext, yyleng); > >> + function_nest++; > >> + } > >> + ")" { > >> + append_string(yytext, yyleng); > >> + if (function_nest-- == 0) { > >> + BEGIN(PARAM); > >> + yylval.string = text; > >> + return T_WORD; > > > > T_WORD_QUOTE (which would turn into a constant symbol in most contexts) > > would be better here, IMO. That would turn $(foo) into just an alias for > > "$(foo)". > > > > See below. > > > >> + } > >> + } > >> + \n { > >> + fprintf(stderr, > >> + "%s:%d:warning: multi-line function not supported\n", > >> + zconf_curname(), zconf_lineno()); > >> + current_file->lineno++; > >> + BEGIN(INITIAL); > >> + return T_EOL; > >> + } > >> + <> { > >> + BEGIN(INITIAL); > >> + } > >> +} > >> + > >> { > >> [ \t]+ { > >> ts = 0; > >> diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y > >> index 784083d..d9977de 100644 > >> --- a/scripts/kconfig/zconf.y > >> +++ b/scripts/kconfig/zconf.y > >> @@ -534,11 +534,19 @@ void conf_parse(const char *name) > >> > >> zconf_initscan(name); > >> > >> + func_init(); > >> _menu_init(); > >> > >> if (getenv("ZCONF_DEBUG")) > >> yydebug = 1; > >> yyparse(); > >> + > >> + /* > >> + * Currently, functions are evaluated only when Kconfig files are > >> + * parsed. We can free functions here. > >> + */ > >> + func_exit(); > >> + > >> if (yynerrs) > >> exit(1); > >> if (!modules_sym) > >> @@ -778,4 +786,5 @@ void zconfdump(FILE *out) > >> #include "confdata.c" > >> #include "expr.c" > >> #include "symbol.c" > >> +#include "function.c" > >> #include "menu.c" > >> -- > >> 2.7.4 > >> > > > > Here's a simplification idea I'm throwing out there: > > > > What about only allowing $ENV and $() within quotes, and just having > > them always do simple text substitution (so that they'd indirectly > > always generate T_WORD_QUOTE tokens)? > > > I ended up with a new state , > but the real problem is the lexer is too ugly. > > So, maybe, the solution is not to make it into a string, > but to re-write the lexer. > > > and are almost the same in the sense > that whitespaces do not split tokens. > > The difference is the characters to start/end with. > > ' ... ' > " ... " > ( ... ) > > If we encounter with the 3 special characters, ' , '', (, > then we enter into state, > then it ends with ', ", ), respectively. > (but we need to take care of nesting, escaping) > > Double-surrounding like "$(cc-option -Oz)" > looks ugly to me. It makes it clear that all we're doing is string interpolation, which is the important thing. The simplest and to me sanest way to implement $(cc-option -Oz) is as "$(cc-option -Oz)" (a SYMBOL_CONST symbol, which lives in a separate namespace -- see below), and at that point $(cc-option -Oz) would just be syntactic sugar for "$(cc-option -Oz)". IMO, that syntactic sugar just hides how simple the behavior really is and makes Kconfig more confusing. Plus it complicates the implementation. > > > > > > > Pros: > > > > - Zero new syntax outside of strings (until the macro stuff). > > > > - Makes the behavior and limitations of the syntax obvious: You can't > > have $(foo) expand to a full expression, only to (possibly part of) a > > value. This is a good limitation, IMO, and it's already there. > > > > - Super simple and straightforward Kconfig implementation. All the new > > magic would happen in expand_string_value(). > > > > Neutral: > > > > - Just as general where it matters. Take something like > > > > default "$(foo)" > > > > If $(foo) happens to expand to "y", then that will do its usual thing > > for a bool/tristate symbol. Same for string symbols, etc. It'd just > > be a thin preprocessing step on constant symbol values. > > > > - The only loss in generality would be that you can no longer have > > a function expand to the name of non-constant symbol. For example, > > take the following: > > > > default $(foo) > > > > If $(foo) expands to MY_SYMBOL, then that would work as > > > > default MY_SYMBOL > > > Probably, this is a "do not do this". > > > If the symbol is 'int' type, > and $MY_NUMBER is expanded to 1, > it is our intention. Just to summarize the tradeoffs in each approach: Approach 1: Let $(fn ...) expand to FOO and reference the symbol FOO if it exists Approach 2: Let $(fn ...) expand to "FOO", which could never reference existing symbols Pros of approach 1: - If someone legitimately wants to do indirection by generating Kconfig symbol names, they can. Cons of approach 1: - If $(fn ...) happens to expand to the name of an existing symbol, it will reference it, even if it wasn't intended. This might get really tricky to debug, as it probably won't be obvious that this is how it works. Pros of approach 2: - You can never accidentally reference an existing symbol when it wasn't intended. You always generate a "value". Constant symbols live in a separate namespace. - The behavior is simpler to understand, IMO. $(fn ...) and $FOO consistently deal with values and just do string interpolation. You don't get "either a symbol or a value" depending on what's already defined. Cons of approach 2: - You can't do indirection by generating Kconfig symbol names. I suspect there'd always be cleaner ways to do that in practice, but I'm showing my bias here. Approach 2 seems much simpler and less magical to me. > > > > > > > (I.e., it would default to the value of the symbol MY_SYMBOL.) > > > > With the quotes, it would instead work as > > > > default "MY_SYMBOL" > > > > IMO, the second version is less confusing, and deliberately using the > > first version seems like a bad idea (there's likely to be cleaner ways > > to do the indirection in plain Kconfig). > > > > This is also why I think T_WORD_QUOTE is better in the code above. It > > would make $(foo) work more like a shorthand for "$(foo)". > > > > > > Cons: > > > > - Bit more typing to add the quotes > > > > - Maybe it isn't widely known that "n", "m", "y" work just like n, m, y > > for bool and tristate symbols (the latter automatically get converted > > to the former), though you see them quite a lot in Kconfig files. > > Not only bool/tristate, but also int/hex. > "1" is equivalent to 1. > > > Kconfig is screwed up in handling of literals. I don't actually think the design is horrible here (though it has flaws). The biggest problem is that it's poorly documented and understood. Expressions are composed of symbols and operators. You have ordinary symbols and constant symbols (quoted stuff, "values" if you will). All symbols have a tristate value and a string value. Which value gets used, and how, depends on the operator. Note that constant symbols are actually stored separately from nonconstant symbols internally (see SYMBOL_CONST). 1 and "1" are not the same symbol (this is easy to miss unless you've spent too much deciphering Kconfig's code). The reason a plain 1 works is that undefined symbols share many behaviors with constant symbols. The biggest mess-up in Kconfig I think is not making the constant vs. undefined distinction clear. It would have been neater if references to undefined symbols simply threw an error, though that gets a bit icky with shared Kconfig files, like in the kernel (would need to make sure that referenced symbols are always defined or predeclare them as undefined). Some people might object to "1" as well, but it would've been a lesser evil I think, once the symbol/value distinction would be clear. The other option would be to tack some kind of complicated type system on top of Kconfig's evaluation semantics. That's overkilling it, IMO. > > > > ("n", "foo bar", etc., are all just constant symbols. Kconfig keeps > > track of them separately from non-constant symbols. The constant > > symbols "n", "m", and "y" are predefined.) > > > > If we go with obligatory quotes, I volunteer to explain things in > > kconfig-language.txt at least, to make it clear why you'd see quotes > > in bool/tristate symbols using $(shell). I suspect it wouldn't be > > that tricky to figure out anyway. > > > I think the right thing to do is > to fix the code instead of advertising it. I don't think the behavior is all bad, just obscure. It could easily be explained. > > > > > > > > What do you think? > > > > 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 > > > > -- > Best Regards > Masahiro Yamada Cheers, Ulf