kconfig: fix relational operators for bool and tristate symbols

Message ID nycvar.YSQ.7.76.1711161956590.20817@knanqh.ubzr
State Accepted
Commit 9059a3493efea6492451430c7e2fa0af799a2abb
Headers show
Series
  • kconfig: fix relational operators for bool and tristate symbols
Related show

Commit Message

Nicolas Pitre Nov. 17, 2017, 1:06 a.m.
Since commit 31847b67bec0 ("kconfig: allow use of relations other than
(in)equality") it is possible to use relational operators in Kconfig
statements. However, those operators give unexpected results when
applied to bool/tristate values:

	(n < y) = y (correct)
	(m < y) = y (correct)
	(n < m) = n (wrong)

This happens because relational operators process bool and tristate
symbols as strings and m sorts before n. It makes little sense to do a
lexicographical compare on bool and tristate values though.

Documentation/kbuild/kconfig-language.txt states that expression can have
a value of 'n', 'm' or 'y' (or 0, 1, 2 respectively for calculations).
Let's make it so for relational comparisons with bool/tristate
expressions as well and document them. If at least one symbol is an
actual string then the lexicographical compare works just as before.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

Comments

Nicolas Pitre Dec. 6, 2017, 3:40 p.m. | #1
Ping.

On Thu, 16 Nov 2017, Nicolas Pitre wrote:

> Since commit 31847b67bec0 ("kconfig: allow use of relations other than

> (in)equality") it is possible to use relational operators in Kconfig

> statements. However, those operators give unexpected results when

> applied to bool/tristate values:

> 

> 	(n < y) = y (correct)

> 	(m < y) = y (correct)

> 	(n < m) = n (wrong)

> 

> This happens because relational operators process bool and tristate

> symbols as strings and m sorts before n. It makes little sense to do a

> lexicographical compare on bool and tristate values though.

> 

> Documentation/kbuild/kconfig-language.txt states that expression can have

> a value of 'n', 'm' or 'y' (or 0, 1, 2 respectively for calculations).

> Let's make it so for relational comparisons with bool/tristate

> expressions as well and document them. If at least one symbol is an

> actual string then the lexicographical compare works just as before.

> 

> Signed-off-by: Nicolas Pitre <nico@linaro.org>

> 

> diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt

> index 262722d886..c4a293a03c 100644

> --- a/Documentation/kbuild/kconfig-language.txt

> +++ b/Documentation/kbuild/kconfig-language.txt

> @@ -200,10 +200,14 @@ module state. Dependency expressions have the following syntax:

>  <expr> ::= <symbol>                             (1)

>             <symbol> '=' <symbol>                (2)

>             <symbol> '!=' <symbol>               (3)

> -           '(' <expr> ')'                       (4)

> -           '!' <expr>                           (5)

> -           <expr> '&&' <expr>                   (6)

> -           <expr> '||' <expr>                   (7)

> +           <symbol1> '<' <symbol2>              (4)

> +           <symbol1> '>' <symbol2>              (4)

> +           <symbol1> '<=' <symbol2>             (4)

> +           <symbol1> '>=' <symbol2>             (4)

> +           '(' <expr> ')'                       (5)

> +           '!' <expr>                           (6)

> +           <expr> '&&' <expr>                   (7)

> +           <expr> '||' <expr>                   (8)

>  

>  Expressions are listed in decreasing order of precedence. 

>  

> @@ -214,10 +218,13 @@ Expressions are listed in decreasing order of precedence.

>      otherwise 'n'.

>  (3) If the values of both symbols are equal, it returns 'n',

>      otherwise 'y'.

> -(4) Returns the value of the expression. Used to override precedence.

> -(5) Returns the result of (2-/expr/).

> -(6) Returns the result of min(/expr/, /expr/).

> -(7) Returns the result of max(/expr/, /expr/).

> +(4) If value of <symbol1> is respectively lower, greater, lower-or-equal,

> +    or greater-or-equal than value of <symbol2>, it returns 'y',

> +    otherwise 'n'.

> +(5) Returns the value of the expression. Used to override precedence.

> +(6) Returns the result of (2-/expr/).

> +(7) Returns the result of min(/expr/, /expr/).

> +(8) Returns the result of max(/expr/, /expr/).

>  

>  An expression can have a value of 'n', 'm' or 'y' (or 0, 1, 2

>  respectively for calculations). A menu entry becomes visible when its

> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c

> index cbf4996dd9..8cee597d33 100644

> --- a/scripts/kconfig/expr.c

> +++ b/scripts/kconfig/expr.c

> @@ -893,7 +893,10 @@ static enum string_value_kind expr_parse_string(const char *str,

>  	switch (type) {

>  	case S_BOOLEAN:

>  	case S_TRISTATE:

> -		return k_string;

> +		val->s = !strcmp(str, "n") ? 0 :

> +			 !strcmp(str, "m") ? 1 :

> +			 !strcmp(str, "y") ? 2 : -1;

> +		return k_signed;

>  	case S_INT:

>  		val->s = strtoll(str, &tail, 10);

>  		kind = k_signed;

>
Randy Dunlap Dec. 6, 2017, 8:24 p.m. | #2
On 12/06/2017 07:40 AM, Nicolas Pitre wrote:
> 

> Ping.

> 

> On Thu, 16 Nov 2017, Nicolas Pitre wrote:

> 

>> Since commit 31847b67bec0 ("kconfig: allow use of relations other than

>> (in)equality") it is possible to use relational operators in Kconfig

>> statements. However, those operators give unexpected results when

>> applied to bool/tristate values:

>>

>> 	(n < y) = y (correct)

>> 	(m < y) = y (correct)

>> 	(n < m) = n (wrong)

>>

>> This happens because relational operators process bool and tristate

>> symbols as strings and m sorts before n. It makes little sense to do a

>> lexicographical compare on bool and tristate values though.

>>

>> Documentation/kbuild/kconfig-language.txt states that expression can have

>> a value of 'n', 'm' or 'y' (or 0, 1, 2 respectively for calculations).

>> Let's make it so for relational comparisons with bool/tristate

>> expressions as well and document them. If at least one symbol is an

>> actual string then the lexicographical compare works just as before.

>>

>> Signed-off-by: Nicolas Pitre <nico@linaro.org>


Acked-by: Randy Dunlap <rdunlap@infradead.org>

Tested-by: Randy Dunlap <rdunlap@infradead.org>



-- 
~Randy
Masahiro Yamada Dec. 9, 2017, 4:35 p.m. | #3
2017-12-07 0:40 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
>

> Ping.

>



I see several kconfig patches queued up.
Patches from Ulf Magnusson have been waiting for longer,
so I will check them first, and I will come back to this as soon as possible.


-- 
Best Regards
Masahiro Yamada
Masahiro Yamada Jan. 5, 2018, 5:34 p.m. | #4
2017-11-17 10:06 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> Since commit 31847b67bec0 ("kconfig: allow use of relations other than

> (in)equality") it is possible to use relational operators in Kconfig

> statements. However, those operators give unexpected results when

> applied to bool/tristate values:

>

>         (n < y) = y (correct)

>         (m < y) = y (correct)

>         (n < m) = n (wrong)

>

> This happens because relational operators process bool and tristate

> symbols as strings and m sorts before n. It makes little sense to do a

> lexicographical compare on bool and tristate values though.

>

> Documentation/kbuild/kconfig-language.txt states that expression can have

> a value of 'n', 'm' or 'y' (or 0, 1, 2 respectively for calculations).

> Let's make it so for relational comparisons with bool/tristate

> expressions as well and document them. If at least one symbol is an

> actual string then the lexicographical compare works just as before.

>

> Signed-off-by: Nicolas Pitre <nico@linaro.org>

>


Applied to linux-kbuild/fixes.  Thanks!

-- 
Best Regards
Masahiro Yamada

Patch

diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
index 262722d886..c4a293a03c 100644
--- a/Documentation/kbuild/kconfig-language.txt
+++ b/Documentation/kbuild/kconfig-language.txt
@@ -200,10 +200,14 @@  module state. Dependency expressions have the following syntax:
 <expr> ::= <symbol>                             (1)
            <symbol> '=' <symbol>                (2)
            <symbol> '!=' <symbol>               (3)
-           '(' <expr> ')'                       (4)
-           '!' <expr>                           (5)
-           <expr> '&&' <expr>                   (6)
-           <expr> '||' <expr>                   (7)
+           <symbol1> '<' <symbol2>              (4)
+           <symbol1> '>' <symbol2>              (4)
+           <symbol1> '<=' <symbol2>             (4)
+           <symbol1> '>=' <symbol2>             (4)
+           '(' <expr> ')'                       (5)
+           '!' <expr>                           (6)
+           <expr> '&&' <expr>                   (7)
+           <expr> '||' <expr>                   (8)
 
 Expressions are listed in decreasing order of precedence. 
 
@@ -214,10 +218,13 @@  Expressions are listed in decreasing order of precedence.
     otherwise 'n'.
 (3) If the values of both symbols are equal, it returns 'n',
     otherwise 'y'.
-(4) Returns the value of the expression. Used to override precedence.
-(5) Returns the result of (2-/expr/).
-(6) Returns the result of min(/expr/, /expr/).
-(7) Returns the result of max(/expr/, /expr/).
+(4) If value of <symbol1> is respectively lower, greater, lower-or-equal,
+    or greater-or-equal than value of <symbol2>, it returns 'y',
+    otherwise 'n'.
+(5) Returns the value of the expression. Used to override precedence.
+(6) Returns the result of (2-/expr/).
+(7) Returns the result of min(/expr/, /expr/).
+(8) Returns the result of max(/expr/, /expr/).
 
 An expression can have a value of 'n', 'm' or 'y' (or 0, 1, 2
 respectively for calculations). A menu entry becomes visible when its
diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index cbf4996dd9..8cee597d33 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -893,7 +893,10 @@  static enum string_value_kind expr_parse_string(const char *str,
 	switch (type) {
 	case S_BOOLEAN:
 	case S_TRISTATE:
-		return k_string;
+		val->s = !strcmp(str, "n") ? 0 :
+			 !strcmp(str, "m") ? 1 :
+			 !strcmp(str, "y") ? 2 : -1;
+		return k_signed;
 	case S_INT:
 		val->s = strtoll(str, &tail, 10);
 		kind = k_signed;