* [RESEND PATCH v2 0/2] kconfig: Remove bad inference rules expr_eliminate_dups2() @ 2014-06-10 14:31 Martin Walch 2014-09-22 17:13 ` [Resending PATCH v2 0/2] kconfig: fix bad syntactic transformation Martin Walch 0 siblings, 1 reply; 10+ messages in thread From: Martin Walch @ 2014-06-10 14:31 UTC (permalink / raw) To: linux-kbuild; +Cc: linux-kernel, Yann E. MORIN, Michal Marek This is the third time I send this patch. As I think it is an obvious improvement, but has not been applied, please tell me what is wrong with this patch and what I can do better. PATCH 1: Remove expr_eliminate_dups2() and other related code. This does not seem to actually affect the behaviour when using Kconfig files from the mainline kernel. But there is a slight possibility that there are configuration files in other branches or in other projects that use the Linux Kernel configuration system that may be affected or may even rely on the behaviour (although I did not find such a case when doing a quick check on busybox, uClibc and openwrt). So, there is a tiny chance to break some very exotic case (on the other hand, it is plausible that it actually fixes any such case). PATCH 2: When touching expr.c anyway, fix some comments and convert C99 style comments to traditional ones. Martin Walch (2): kconfig: completely remove expr_eliminate_dups2() and related code kconfig: trivial - adjust comments scripts/kconfig/expr.c | 146 +++++++------------------------------------------ scripts/kconfig/expr.h | 3 - 2 files changed, 19 insertions(+), 130 deletions(-) -- 1.8.5.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Resending PATCH v2 0/2] kconfig: fix bad syntactic transformation 2014-06-10 14:31 [RESEND PATCH v2 0/2] kconfig: Remove bad inference rules expr_eliminate_dups2() Martin Walch @ 2014-09-22 17:13 ` Martin Walch 2014-09-22 17:13 ` [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c Martin Walch 2014-09-22 17:13 ` [PATCH v2 2/2] kconfig: trivial - adjust comments Martin Walch 0 siblings, 2 replies; 10+ messages in thread From: Martin Walch @ 2014-09-22 17:13 UTC (permalink / raw) To: yann.morin.1998; +Cc: linux-kbuild, linux-kernel, Martin Walch PATCH 1: Remove these two not semantics preserving inference rules: (FOO || BAR) && (!FOO && !BAR) -> n (FOO && BAR) || (!FOO || !BAR) -> y When applied, they silently alter the meaning of symbol dependencies. PATCH 2: When touching expr.c anyway, fix some comments and convert C99 style comments to traditional ones. Martin Walch (2): kconfig: fix bad syntactic transformation in expr.c kconfig: trivial - adjust comments scripts/kconfig/expr.c | 146 +++++++------------------------------------------ scripts/kconfig/expr.h | 3 - 2 files changed, 19 insertions(+), 130 deletions(-) -- 1.8.5.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c 2014-09-22 17:13 ` [Resending PATCH v2 0/2] kconfig: fix bad syntactic transformation Martin Walch @ 2014-09-22 17:13 ` Martin Walch 2014-10-07 8:10 ` Paul Bolle 2014-10-11 19:07 ` Paul Bolle 2014-09-22 17:13 ` [PATCH v2 2/2] kconfig: trivial - adjust comments Martin Walch 1 sibling, 2 replies; 10+ messages in thread From: Martin Walch @ 2014-09-22 17:13 UTC (permalink / raw) To: yann.morin.1998; +Cc: linux-kbuild, linux-kernel, Martin Walch expr_eliminate_dups2() in scripts/kconfig/expr.c applies two bad inference rules: (FOO || BAR) && (!FOO && !BAR) -> n (FOO && BAR) || (!FOO || !BAR) -> y They would be correct in propositional logic, but this is a three-valued logic, and here it is wrong in that it changes semantics. It becomes immediately visible when substituting FOO and BAR with m. Fix it by removing expr_eliminate_dups2() and the functions that have no use anywhere else: expr_extract_eq_and(), expr_extract_eq_or(), and expr_extract_eq() from scripts/kconfig/expr.[ch] Currently the bug is not triggered in mainline, so this patch does not modify the configuration space there. As a side effect, this reduces code size in expr.c by roughly 10% and slightly improves startup time for all configuration frontends. Signed-off-by: Martin Walch <walch.martin@web.de> --- scripts/kconfig/expr.c | 108 ------------------------------------------------- scripts/kconfig/expr.h | 3 -- 2 files changed, 111 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index d662652..4aa171b 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -553,62 +553,6 @@ static void expr_eliminate_dups1(enum expr_type type, struct expr **ep1, struct #undef e2 } -static void expr_eliminate_dups2(enum expr_type type, struct expr **ep1, struct expr **ep2) -{ -#define e1 (*ep1) -#define e2 (*ep2) - struct expr *tmp, *tmp1, *tmp2; - - if (e1->type == type) { - expr_eliminate_dups2(type, &e1->left.expr, &e2); - expr_eliminate_dups2(type, &e1->right.expr, &e2); - return; - } - if (e2->type == type) { - expr_eliminate_dups2(type, &e1, &e2->left.expr); - expr_eliminate_dups2(type, &e1, &e2->right.expr); - } - if (e1 == e2) - return; - - switch (e1->type) { - case E_OR: - expr_eliminate_dups2(e1->type, &e1, &e1); - // (FOO || BAR) && (!FOO && !BAR) -> n - tmp1 = expr_transform(expr_alloc_one(E_NOT, expr_copy(e1))); - tmp2 = expr_copy(e2); - tmp = expr_extract_eq_and(&tmp1, &tmp2); - if (expr_is_yes(tmp1)) { - expr_free(e1); - e1 = expr_alloc_symbol(&symbol_no); - trans_count++; - } - expr_free(tmp2); - expr_free(tmp1); - expr_free(tmp); - break; - case E_AND: - expr_eliminate_dups2(e1->type, &e1, &e1); - // (FOO && BAR) || (!FOO || !BAR) -> y - tmp1 = expr_transform(expr_alloc_one(E_NOT, expr_copy(e1))); - tmp2 = expr_copy(e2); - tmp = expr_extract_eq_or(&tmp1, &tmp2); - if (expr_is_no(tmp1)) { - expr_free(e1); - e1 = expr_alloc_symbol(&symbol_yes); - trans_count++; - } - expr_free(tmp2); - expr_free(tmp1); - expr_free(tmp); - break; - default: - ; - } -#undef e1 -#undef e2 -} - struct expr *expr_eliminate_dups(struct expr *e) { int oldcount; @@ -621,7 +565,6 @@ struct expr *expr_eliminate_dups(struct expr *e) switch (e->type) { case E_OR: case E_AND: expr_eliminate_dups1(e->type, &e, &e); - expr_eliminate_dups2(e->type, &e, &e); default: ; } @@ -823,57 +766,6 @@ bool expr_depends_symbol(struct expr *dep, struct symbol *sym) return false; } -struct expr *expr_extract_eq_and(struct expr **ep1, struct expr **ep2) -{ - struct expr *tmp = NULL; - expr_extract_eq(E_AND, &tmp, ep1, ep2); - if (tmp) { - *ep1 = expr_eliminate_yn(*ep1); - *ep2 = expr_eliminate_yn(*ep2); - } - return tmp; -} - -struct expr *expr_extract_eq_or(struct expr **ep1, struct expr **ep2) -{ - struct expr *tmp = NULL; - expr_extract_eq(E_OR, &tmp, ep1, ep2); - if (tmp) { - *ep1 = expr_eliminate_yn(*ep1); - *ep2 = expr_eliminate_yn(*ep2); - } - return tmp; -} - -void expr_extract_eq(enum expr_type type, struct expr **ep, struct expr **ep1, struct expr **ep2) -{ -#define e1 (*ep1) -#define e2 (*ep2) - if (e1->type == type) { - expr_extract_eq(type, ep, &e1->left.expr, &e2); - expr_extract_eq(type, ep, &e1->right.expr, &e2); - return; - } - if (e2->type == type) { - expr_extract_eq(type, ep, ep1, &e2->left.expr); - expr_extract_eq(type, ep, ep1, &e2->right.expr); - return; - } - if (expr_eq(e1, e2)) { - *ep = *ep ? expr_alloc_two(type, *ep, e1) : e1; - expr_free(e2); - if (type == E_AND) { - e1 = expr_alloc_symbol(&symbol_yes); - e2 = expr_alloc_symbol(&symbol_yes); - } else if (type == E_OR) { - e1 = expr_alloc_symbol(&symbol_no); - e2 = expr_alloc_symbol(&symbol_no); - } - } -#undef e1 -#undef e2 -} - struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symbol *sym) { struct expr *e1, *e2; diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index 412ea8a..9c9fb57 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -214,9 +214,6 @@ struct expr *expr_eliminate_dups(struct expr *e); struct expr *expr_transform(struct expr *e); int expr_contains_symbol(struct expr *dep, struct symbol *sym); bool expr_depends_symbol(struct expr *dep, struct symbol *sym); -struct expr *expr_extract_eq_and(struct expr **ep1, struct expr **ep2); -struct expr *expr_extract_eq_or(struct expr **ep1, struct expr **ep2); -void expr_extract_eq(enum expr_type type, struct expr **ep, struct expr **ep1, struct expr **ep2); struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symbol *sym); struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2); -- 1.8.5.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c 2014-09-22 17:13 ` [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c Martin Walch @ 2014-10-07 8:10 ` Paul Bolle 2014-10-07 9:21 ` Dirk Gouders 2014-10-11 19:07 ` Paul Bolle 1 sibling, 1 reply; 10+ messages in thread From: Paul Bolle @ 2014-10-07 8:10 UTC (permalink / raw) To: Martin Walch; +Cc: yann.morin.1998, linux-kbuild, linux-kernel On Mon, 2014-09-22 at 19:13 +0200, Martin Walch wrote: > expr_eliminate_dups2() in scripts/kconfig/expr.c applies two bad > inference rules: > > (FOO || BAR) && (!FOO && !BAR) -> n > (FOO && BAR) || (!FOO || !BAR) -> y > > They would be correct in propositional logic, but this is a three-valued > logic, and here it is wrong in that it changes semantics. It becomes > immediately visible when substituting FOO and BAR with m. Well, not to me. When I apply the rules under "Menu dependencies" in Documentation/kbuild/kconfig-language.txt, in embarrassingly small steps, I get: (m || m) && (!m && !m) -> (1 || 1) && ((2 - 1) && (2 - 1)) -> (1 || 1) && (1 && 1) -> max(1, 1) && min(1, 1) -> 1 && 1 -> min(1, 1) -> 1 -> m and (m && m) || (!m || !m) -> (1 && 1) || ((2 - 1) || (2 - 1)) -> (1 && 1) || (1 || 1) -> min(1, 1) || max(1, 1) -> 1 || 1 -> max(1, 1) -> 1 -> m Did I do that right? Anyway, perhaps you'd like to add a line or two how these two expressions actually evaluate when FOO and BAR are m. > Fix it by removing expr_eliminate_dups2() and the functions that have no use > anywhere else: expr_extract_eq_and(), expr_extract_eq_or(), > and expr_extract_eq() from scripts/kconfig/expr.[ch] > > Currently the bug is not triggered in mainline, so this patch does not modify > the configuration space there. > > As a side effect, this reduces code size in expr.c by roughly 10% and slightly > improves startup time for all configuration frontends. > > Signed-off-by: Martin Walch <walch.martin@web.de> Does Yann still care about Kconfig? Yann has been rather quiet for a while now. Paul Bolle ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c 2014-10-07 8:10 ` Paul Bolle @ 2014-10-07 9:21 ` Dirk Gouders 2014-10-11 0:20 ` Martin Walch 0 siblings, 1 reply; 10+ messages in thread From: Dirk Gouders @ 2014-10-07 9:21 UTC (permalink / raw) To: Paul Bolle Cc: Martin Walch, yann.morin.1998, Michal Marek, linux-kbuild, linux-kernel CC: Michal added Paul Bolle <pebolle@tiscali.nl> writes: > On Mon, 2014-09-22 at 19:13 +0200, Martin Walch wrote: >> expr_eliminate_dups2() in scripts/kconfig/expr.c applies two bad >> inference rules: >> >> (FOO || BAR) && (!FOO && !BAR) -> n >> (FOO && BAR) || (!FOO || !BAR) -> y >> >> They would be correct in propositional logic, but this is a three-valued >> logic, and here it is wrong in that it changes semantics. It becomes >> immediately visible when substituting FOO and BAR with m. > > Well, not to me. When I apply the rules under "Menu dependencies" in > Documentation/kbuild/kconfig-language.txt, in embarrassingly small > steps, I get: > (m || m) && (!m && !m) -> > (1 || 1) && ((2 - 1) && (2 - 1)) -> > (1 || 1) && (1 && 1) -> > max(1, 1) && min(1, 1) -> > 1 && 1 -> > min(1, 1) -> > 1 -> m > > and > (m && m) || (!m || !m) -> > (1 && 1) || ((2 - 1) || (2 - 1)) -> > (1 && 1) || (1 || 1) -> > min(1, 1) || max(1, 1) -> > 1 || 1 -> > max(1, 1) -> > 1 -> m > > Did I do that right? Anyway, perhaps you'd like to add a line or two how > these two expressions actually evaluate when FOO and BAR are m. Perhaps, it helps to use a Kconfig file that visualizes the problem: mainmenu "Expr Test" config expr-config bool "Expr Config" depends on (m || m) && (!m && !m) # depends on (m && m) || (!m || !m) config dummy bool "Dummy" config modules bool option modules default y Those two depends, according to your calculation and if I understood correctly also to Martin's, should evaluate to 'm' and thus the menu entry should always be visible, because both depends should evaluate to 'm'. But because of expr_eliminate_dups2() it is either invisible (the first depends becomes 'n') or always visible (the second depends becomes 'y'). Dirk >> Fix it by removing expr_eliminate_dups2() and the functions that have no use >> anywhere else: expr_extract_eq_and(), expr_extract_eq_or(), >> and expr_extract_eq() from scripts/kconfig/expr.[ch] >> >> Currently the bug is not triggered in mainline, so this patch does not modify >> the configuration space there. >> >> As a side effect, this reduces code size in expr.c by roughly 10% and slightly >> improves startup time for all configuration frontends. >> >> Signed-off-by: Martin Walch <walch.martin@web.de> > > Does Yann still care about Kconfig? Yann has been rather quiet for a > while now. > > > Paul Bolle > > -- > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c 2014-10-07 9:21 ` Dirk Gouders @ 2014-10-11 0:20 ` Martin Walch 0 siblings, 0 replies; 10+ messages in thread From: Martin Walch @ 2014-10-11 0:20 UTC (permalink / raw) To: Dirk Gouders Cc: Paul Bolle, Martin Walch, yann.morin.1998, Michal Marek, linux-kbuild, linux-kernel On Tuesday 07 October 2014 11:21:51 Dirk Gouders wrote: > Paul Bolle <pebolle@tiscali.nl> writes: > > > On Mon, 2014-09-22 at 19:13 +0200, Martin Walch wrote: > >> expr_eliminate_dups2() in scripts/kconfig/expr.c applies two bad > >> inference rules: > >> > >> (FOO || BAR) && (!FOO && !BAR) -> n > >> (FOO && BAR) || (!FOO || !BAR) -> y > >> > >> They would be correct in propositional logic, but this is a three-valued > >> logic, and here it is wrong in that it changes semantics. It becomes > >> immediately visible when substituting FOO and BAR with m. > > > > Well, not to me. When I apply the rules under "Menu dependencies" in > > Documentation/kbuild/kconfig-language.txt, in embarrassingly small > > steps, I get: > > (m || m) && (!m && !m) -> > > (1 || 1) && ((2 - 1) && (2 - 1)) -> > > (1 || 1) && (1 && 1) -> > > max(1, 1) && min(1, 1) -> > > 1 && 1 -> > > min(1, 1) -> > > 1 -> m > > > > and > > (m && m) || (!m || !m) -> > > (1 && 1) || ((2 - 1) || (2 - 1)) -> > > (1 && 1) || (1 || 1) -> > > min(1, 1) || max(1, 1) -> > > 1 || 1 -> > > max(1, 1) -> > > 1 -> m > > > > Did I do that right? Anyway, perhaps you'd like to add a line or two how > > these two expressions actually evaluate when FOO and BAR are m. Yes, this is a proper evaluation. To explain the problem in more detail, I will describe it a bit more formal. First of all, this is a thing about the formal system of the configuration logic, so I think it is a good idea to distinguish between syntax and semantics. Any expression, e.g. "(FOO || BAR)" is pure syntax. It consists of non-constant symbols (FOO, BAR...), constant symbols (n, m, y), operators (||, &&, !), and parenthesis. Semantics is when evaluating an expression by assigning values to the constant and the non-constant symbols and functions to the operators (overriding operator precedence with parenthesis), and then calculating an actual value. So, this is pure syntax: (FOO || BAR) && (!FOO && !BAR) And semantics is: * an assignment of 0, 1, or 2 to each of the non-constant symbols (arbitrary) * the fixed assignments of 0 to n, 1 to m, and 2 to y * applying the functions to values that correspond to the respective operators So, e.g. when assigning 2 to FOO and 0 to BAR, then the evaluation of (FOO || BAR) && (!FOO && !BAR) is min(max(2, 0), min(2-2, 2-0)) = 0 Above, we saw that assigning 1 to FOO as well as to BAR yields 1 for the whole expression. Now getting back to the problem: the configuration system makes some syntactical transformations according to some rules, so they are easier to read in the user interfaces (e.g. mconf). In formal logic, such rules are called "inference rules". Obviously, inference rules are purely syntactical transformations, e.g. one rule could be to replace "FOO && FOO" with only "FOO". Another rule could replace "FOO || BAR" with "FOO && BAR". The difference between these two example rules is that in semantics, the first one keeps the result of the evaluation for all possible assignments, while the second one changes the results for some of the assignments. Formally, the first rule is "valid" while the second one is not. With the term "valid" it is now easy to summarize the actual problem: The inference rule (FOO || BAR) && (!FOO && !BAR) -> n is not valid. The proof for this is assigning 1 to FOO and also to BAR. As we have already seen, for this assignment, the expression evaluates to 1. In contrast, the expression "n" always evaluates to 0. So, this inference rule is not valid. > Perhaps, it helps to use a Kconfig file that visualizes the problem: > > mainmenu "Expr Test" > > config expr-config > bool "Expr Config" > depends on (m || m) && (!m && !m) > # depends on (m && m) || (!m || !m) > > config dummy > bool "Dummy" > > config modules > bool > option modules > default y > > Those two depends, according to your calculation and if I understood > correctly also to Martin's, should evaluate to 'm' and thus the menu > entry should always be visible, because both depends should evaluate to > 'm'. But because of expr_eliminate_dups2() it is either invisible (the > first depends becomes 'n') or always visible (the second depends becomes > 'y'). This is a good idea. However, I tried to avoid an example that contains the constant symbol "m". The problem with this is that "m" is treated specially and can lead to some confusion, although a very short test case is possible using m: config expr-config-short bool "Expr Config Short" depends on m && !m When this file is read, each of the two "m" symbols is replaced with "m && MODULES", so the expression reads: (m && MODULES) && !(m && MODULES) then the negation of "!(m && MODULES)" is transformed: (m && MODULES) && (!m || !MODULES) (this corresponds to one of the "De Morgan's laws" in other logical systems) Finally, the bad inference rule applies (although the form of the expression is slightly different from above) and this expression is replaced with: n This is not valid (assigning 2 to MODULES should yield 1 for the whole expression, but "n" evaluates to 0). Does this make things clearer? Regards, Martin Walch -- ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c 2014-09-22 17:13 ` [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c Martin Walch 2014-10-07 8:10 ` Paul Bolle @ 2014-10-11 19:07 ` Paul Bolle 1 sibling, 0 replies; 10+ messages in thread From: Paul Bolle @ 2014-10-11 19:07 UTC (permalink / raw) To: Martin Walch; +Cc: yann.morin.1998, linux-kbuild, linux-kernel On Mon, 2014-09-22 at 19:13 +0200, Martin Walch wrote: > Fix it by removing expr_eliminate_dups2() and the functions that have no use > anywhere else: expr_extract_eq_and(), expr_extract_eq_or(), > and expr_extract_eq() from scripts/kconfig/expr.[ch] > > Currently the bug is not triggered in mainline, so this patch does not modify > the configuration space there. Would I be whining if I'd complained about the lack of documentation for expr_eliminate_dups(), expr_eliminate_dups1(), and expr_eliminate_dups2()? So, I've put in these two calls of fprintf(): @@ -579,6 +606,7 @@ static void expr_eliminate_dups2(enum expr_type type, struct expr **ep1, struct tmp2 = expr_copy(e2); tmp = expr_extract_eq_and(&tmp1, &tmp2); if (expr_is_yes(tmp1)) { + fprintf(stderr, "%s:%d:\n", __func__, __LINE__); expr_free(e1); e1 = expr_alloc_symbol(&symbol_no); trans_count++; @@ -594,6 +622,7 @@ static void expr_eliminate_dups2(enum expr_type type, struct expr **ep1, struct tmp2 = expr_copy(e2); tmp = expr_extract_eq_or(&tmp1, &tmp2); if (expr_is_no(tmp1)) { + fprintf(stderr, "%s:%d:\n", __func__, __LINE__); expr_free(e1); e1 = expr_alloc_symbol(&symbol_yes); trans_count++; None of the 500+ defconfig files in next-20141010 triggered those fprintf's. Does that means it's likely that nothing in next-20141010 actually uses the functionality expr_eliminate_dups2() provides? Paul Bolle ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] kconfig: trivial - adjust comments 2014-09-22 17:13 ` [Resending PATCH v2 0/2] kconfig: fix bad syntactic transformation Martin Walch 2014-09-22 17:13 ` [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c Martin Walch @ 2014-09-22 17:13 ` Martin Walch 2014-10-10 13:01 ` Paul Bolle 1 sibling, 1 reply; 10+ messages in thread From: Martin Walch @ 2014-09-22 17:13 UTC (permalink / raw) To: yann.morin.1998; +Cc: linux-kbuild, linux-kernel, Martin Walch Replace any C99 comments with traditional ones (/*...*/). Also fix the contents of two comments: 1) the second occurrence of (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' is probably copy & paste from above, missing the adjustment corresponding to the code. It should probably read (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' 2) that one is similar: (a!='m') && (a!='n') -> (a='m') it should be (a!='m') && (a!='n') -> (a='y') Signed-off-by: Martin Walch <walch.martin@web.de> --- scripts/kconfig/expr.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 4aa171b..c840e52 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -326,7 +326,7 @@ struct expr *expr_trans_bool(struct expr *e) e->right.expr = expr_trans_bool(e->right.expr); break; case E_UNEQUAL: - // FOO!=n -> FOO + /* FOO!=n -> FOO */ if (e->left.sym->type == S_TRISTATE) { if (e->right.sym == &symbol_no) { e->type = E_SYMBOL; @@ -375,19 +375,19 @@ static struct expr *expr_join_or(struct expr *e1, struct expr *e2) if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_mod) || (e1->right.sym == &symbol_mod && e2->right.sym == &symbol_yes))) { - // (a='y') || (a='m') -> (a!='n') + /* (a='y') || (a='m') -> (a!='n') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_no); } if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_yes))) { - // (a='y') || (a='n') -> (a!='m') + /* (a='y') || (a='n') -> (a!='m') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_mod); } if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_mod && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_mod))) { - // (a='m') || (a='n') -> (a!='y') + /* (a='m') || (a='n') -> (a!='y') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_yes); } } @@ -438,29 +438,29 @@ static struct expr *expr_join_and(struct expr *e1, struct expr *e2) if ((e1->type == E_SYMBOL && e2->type == E_EQUAL && e2->right.sym == &symbol_yes) || (e2->type == E_SYMBOL && e1->type == E_EQUAL && e1->right.sym == &symbol_yes)) - // (a) && (a='y') -> (a='y') + /* (a) && (a='y') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if ((e1->type == E_SYMBOL && e2->type == E_UNEQUAL && e2->right.sym == &symbol_no) || (e2->type == E_SYMBOL && e1->type == E_UNEQUAL && e1->right.sym == &symbol_no)) - // (a) && (a!='n') -> (a) + /* (a) && (a!='n') -> (a) */ return expr_alloc_symbol(sym1); if ((e1->type == E_SYMBOL && e2->type == E_UNEQUAL && e2->right.sym == &symbol_mod) || (e2->type == E_SYMBOL && e1->type == E_UNEQUAL && e1->right.sym == &symbol_mod)) - // (a) && (a!='m') -> (a='y') + /* (a) && (a!='m') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if (sym1->type == S_TRISTATE) { if (e1->type == E_EQUAL && e2->type == E_UNEQUAL) { - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' + /* (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' */ sym2 = e1->right.sym; if ((e2->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) return sym2 != e2->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) : expr_alloc_symbol(&symbol_no); } if (e1->type == E_UNEQUAL && e2->type == E_EQUAL) { - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' + /* (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' */ sym2 = e2->right.sym; if ((e1->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) return sym2 != e1->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) @@ -469,19 +469,19 @@ static struct expr *expr_join_and(struct expr *e1, struct expr *e2) if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_yes))) - // (a!='y') && (a!='n') -> (a='m') + /* (a!='y') && (a!='n') -> (a='m') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_mod); if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_mod) || (e1->right.sym == &symbol_mod && e2->right.sym == &symbol_yes))) - // (a!='y') && (a!='m') -> (a='n') + /* (a!='y') && (a!='m') -> (a='n') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_no); if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_mod && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_mod))) - // (a!='m') && (a!='n') -> (a='m') + /* (a!='m') && (a!='n') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if ((e1->type == E_SYMBOL && e2->type == E_EQUAL && e2->right.sym == &symbol_mod) || @@ -641,7 +641,7 @@ struct expr *expr_transform(struct expr *e) case E_NOT: switch (e->left.expr->type) { case E_NOT: - // !!a -> a + /* !!a -> a */ tmp = e->left.expr->left.expr; free(e->left.expr); free(e); @@ -650,14 +650,14 @@ struct expr *expr_transform(struct expr *e) break; case E_EQUAL: case E_UNEQUAL: - // !a='x' -> a!='x' + /* !a='x' -> a!='x' */ tmp = e->left.expr; free(e); e = tmp; e->type = e->type == E_EQUAL ? E_UNEQUAL : E_EQUAL; break; case E_OR: - // !(a || b) -> !a && !b + /* !(a || b) -> !a && !b */ tmp = e->left.expr; e->type = E_AND; e->right.expr = expr_alloc_one(E_NOT, tmp->right.expr); @@ -666,7 +666,7 @@ struct expr *expr_transform(struct expr *e) e = expr_transform(e); break; case E_AND: - // !(a && b) -> !a || !b + /* !(a && b) -> !a || !b */ tmp = e->left.expr; e->type = E_OR; e->right.expr = expr_alloc_one(E_NOT, tmp->right.expr); @@ -676,7 +676,7 @@ struct expr *expr_transform(struct expr *e) break; case E_SYMBOL: if (e->left.expr->left.sym == &symbol_yes) { - // !'y' -> 'n' + /* !'y' -> 'n' */ tmp = e->left.expr; free(e); e = tmp; @@ -685,7 +685,7 @@ struct expr *expr_transform(struct expr *e) break; } if (e->left.expr->left.sym == &symbol_mod) { - // !'m' -> 'm' + /* !'m' -> 'm' */ tmp = e->left.expr; free(e); e = tmp; @@ -694,7 +694,7 @@ struct expr *expr_transform(struct expr *e) break; } if (e->left.expr->left.sym == &symbol_no) { - // !'n' -> 'y' + /* !'n' -> 'y' */ tmp = e->left.expr; free(e); e = tmp; -- 1.8.5.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] kconfig: trivial - adjust comments 2014-09-22 17:13 ` [PATCH v2 2/2] kconfig: trivial - adjust comments Martin Walch @ 2014-10-10 13:01 ` Paul Bolle 0 siblings, 0 replies; 10+ messages in thread From: Paul Bolle @ 2014-10-10 13:01 UTC (permalink / raw) To: Martin Walch; +Cc: yann.morin.1998, linux-kbuild, linux-kernel Hi Martin, I haven't yet dared to review 1/2, which is the interesting change. So I cheated and skipped to 2/2. On Mon, 2014-09-22 at 19:13 +0200, Martin Walch wrote: > Replace any C99 comments with traditional ones (/*...*/). Perhaps you could split those changes off in a separate, trivial patch. > Also fix the contents of two comments: > > 1) the second occurrence of > (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' > is probably copy & paste from above, missing the adjustment corresponding > to the code. > It should probably read > (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' > > 2) that one is similar: > (a!='m') && (a!='n') -> (a='m') > it should be > (a!='m') && (a!='n') -> (a='y') And perhaps even send these two changes as two separate patches. Not sure, as I don't review much, but it should make it easier to review. > Signed-off-by: Martin Walch <walch.martin@web.de> > --- > scripts/kconfig/expr.c | 38 +++++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index 4aa171b..c840e52 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > [...] > if (sym1->type == S_TRISTATE) { > if (e1->type == E_EQUAL && e2->type == E_UNEQUAL) { > - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' > + /* (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' */ Could you make this match the code? Ie, /* (a='b') && (a!='c') -> 'b'!='c' ? a='b' : 'n' */ if I'm reading the code right. > sym2 = e1->right.sym; > if ((e2->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) > return sym2 != e2->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) > : expr_alloc_symbol(&symbol_no); > } > if (e1->type == E_UNEQUAL && e2->type == E_EQUAL) { > - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' > + /* (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' */ Likewise: /* (a!='b') && (a='c') -> 'b'!='c' ? a='c' : 'n' */ > sym2 = e2->right.sym; > if ((e1->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) > return sym2 != e1->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) (You could also make the code match the comments. But that would make the patch less trivial.) Paul Bolle ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] kconfig: trivial - adjust comments @ 2014-04-18 21:34 Martin Walch 0 siblings, 0 replies; 10+ messages in thread From: Martin Walch @ 2014-04-18 21:34 UTC (permalink / raw) To: linux-kbuild; +Cc: linux-kernel, Yann E. MORIN From: Martin Walch <walch.martin@web.de> Date: Fri, 18 Apr 2014 22:15:37 +0200 Subject: [PATCH v2 2/2] kconfig: trivial - adjust comments Replace any C99 comments with traditional ones (/*...*/). Also fix the contents of two comments: 1) the second occurrence of (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' is probably copy & paste from above, missing the adjustment corresponding to the code. It should probably read (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' 2) that one is similar: (a!='m') && (a!='n') -> (a='m') it should be (a!='m') && (a!='n') -> (a='y') Signed-off-by: Martin Walch <walch.martin@web.de> --- scripts/kconfig/expr.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 4aa171b..c840e52 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -326,7 +326,7 @@ struct expr *expr_trans_bool(struct expr *e) e->right.expr = expr_trans_bool(e->right.expr); break; case E_UNEQUAL: - // FOO!=n -> FOO + /* FOO!=n -> FOO */ if (e->left.sym->type == S_TRISTATE) { if (e->right.sym == &symbol_no) { e->type = E_SYMBOL; @@ -375,19 +375,19 @@ static struct expr *expr_join_or(struct expr *e1, struct expr *e2) if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_mod) || (e1->right.sym == &symbol_mod && e2->right.sym == &symbol_yes))) { - // (a='y') || (a='m') -> (a!='n') + /* (a='y') || (a='m') -> (a!='n') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_no); } if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_yes))) { - // (a='y') || (a='n') -> (a!='m') + /* (a='y') || (a='n') -> (a!='m') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_mod); } if (e1->type == E_EQUAL && e2->type == E_EQUAL && ((e1->right.sym == &symbol_mod && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_mod))) { - // (a='m') || (a='n') -> (a!='y') + /* (a='m') || (a='n') -> (a!='y') */ return expr_alloc_comp(E_UNEQUAL, sym1, &symbol_yes); } } @@ -438,29 +438,29 @@ static struct expr *expr_join_and(struct expr *e1, struct expr *e2) if ((e1->type == E_SYMBOL && e2->type == E_EQUAL && e2->right.sym == &symbol_yes) || (e2->type == E_SYMBOL && e1->type == E_EQUAL && e1->right.sym == &symbol_yes)) - // (a) && (a='y') -> (a='y') + /* (a) && (a='y') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if ((e1->type == E_SYMBOL && e2->type == E_UNEQUAL && e2->right.sym == &symbol_no) || (e2->type == E_SYMBOL && e1->type == E_UNEQUAL && e1->right.sym == &symbol_no)) - // (a) && (a!='n') -> (a) + /* (a) && (a!='n') -> (a) */ return expr_alloc_symbol(sym1); if ((e1->type == E_SYMBOL && e2->type == E_UNEQUAL && e2->right.sym == &symbol_mod) || (e2->type == E_SYMBOL && e1->type == E_UNEQUAL && e1->right.sym == &symbol_mod)) - // (a) && (a!='m') -> (a='y') + /* (a) && (a!='m') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if (sym1->type == S_TRISTATE) { if (e1->type == E_EQUAL && e2->type == E_UNEQUAL) { - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' + /* (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' */ sym2 = e1->right.sym; if ((e2->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) return sym2 != e2->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) : expr_alloc_symbol(&symbol_no); } if (e1->type == E_UNEQUAL && e2->type == E_EQUAL) { - // (a='b') && (a!='c') -> 'b'='c' ? 'n' : a='b' + /* (a!='b') && (a='c') -> 'b'='c' ? 'n' : a='c' */ sym2 = e2->right.sym; if ((e1->right.sym->flags & SYMBOL_CONST) && (sym2->flags & SYMBOL_CONST)) return sym2 != e1->right.sym ? expr_alloc_comp(E_EQUAL, sym1, sym2) @@ -469,19 +469,19 @@ static struct expr *expr_join_and(struct expr *e1, struct expr *e2) if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_yes))) - // (a!='y') && (a!='n') -> (a='m') + /* (a!='y') && (a!='n') -> (a='m') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_mod); if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_yes && e2->right.sym == &symbol_mod) || (e1->right.sym == &symbol_mod && e2->right.sym == &symbol_yes))) - // (a!='y') && (a!='m') -> (a='n') + /* (a!='y') && (a!='m') -> (a='n') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_no); if (e1->type == E_UNEQUAL && e2->type == E_UNEQUAL && ((e1->right.sym == &symbol_mod && e2->right.sym == &symbol_no) || (e1->right.sym == &symbol_no && e2->right.sym == &symbol_mod))) - // (a!='m') && (a!='n') -> (a='m') + /* (a!='m') && (a!='n') -> (a='y') */ return expr_alloc_comp(E_EQUAL, sym1, &symbol_yes); if ((e1->type == E_SYMBOL && e2->type == E_EQUAL && e2->right.sym == &symbol_mod) || @@ -641,7 +641,7 @@ struct expr *expr_transform(struct expr *e) case E_NOT: switch (e->left.expr->type) { case E_NOT: - // !!a -> a + /* !!a -> a */ tmp = e->left.expr->left.expr; free(e->left.expr); free(e); @@ -650,14 +650,14 @@ struct expr *expr_transform(struct expr *e) break; case E_EQUAL: case E_UNEQUAL: - // !a='x' -> a!='x' + /* !a='x' -> a!='x' */ tmp = e->left.expr; free(e); e = tmp; e->type = e->type == E_EQUAL ? E_UNEQUAL : E_EQUAL; break; case E_OR: - // !(a || b) -> !a && !b + /* !(a || b) -> !a && !b */ tmp = e->left.expr; e->type = E_AND; e->right.expr = expr_alloc_one(E_NOT, tmp->right.expr); @@ -666,7 +666,7 @@ struct expr *expr_transform(struct expr *e) e = expr_transform(e); break; case E_AND: - // !(a && b) -> !a || !b + /* !(a && b) -> !a || !b */ tmp = e->left.expr; e->type = E_OR; e->right.expr = expr_alloc_one(E_NOT, tmp->right.expr); @@ -676,7 +676,7 @@ struct expr *expr_transform(struct expr *e) break; case E_SYMBOL: if (e->left.expr->left.sym == &symbol_yes) { - // !'y' -> 'n' + /* !'y' -> 'n' */ tmp = e->left.expr; free(e); e = tmp; @@ -685,7 +685,7 @@ struct expr *expr_transform(struct expr *e) break; } if (e->left.expr->left.sym == &symbol_mod) { - // !'m' -> 'm' + /* !'m' -> 'm' */ tmp = e->left.expr; free(e); e = tmp; @@ -694,7 +694,7 @@ struct expr *expr_transform(struct expr *e) break; } if (e->left.expr->left.sym == &symbol_no) { - // !'n' -> 'y' + /* !'n' -> 'y' */ tmp = e->left.expr; free(e); e = tmp; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-10-11 19:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-10 14:31 [RESEND PATCH v2 0/2] kconfig: Remove bad inference rules expr_eliminate_dups2() Martin Walch 2014-09-22 17:13 ` [Resending PATCH v2 0/2] kconfig: fix bad syntactic transformation Martin Walch 2014-09-22 17:13 ` [PATCH v2 1/2] kconfig: fix bad syntactic transformation in expr.c Martin Walch 2014-10-07 8:10 ` Paul Bolle 2014-10-07 9:21 ` Dirk Gouders 2014-10-11 0:20 ` Martin Walch 2014-10-11 19:07 ` Paul Bolle 2014-09-22 17:13 ` [PATCH v2 2/2] kconfig: trivial - adjust comments Martin Walch 2014-10-10 13:01 ` Paul Bolle -- strict thread matches above, loose matches on Subject: below -- 2014-04-18 21:34 Martin Walch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).