Skip to contentSkip to conversation

kconfig: allow use of relations other than (in)equality

PATCH In review Jan Beulich 2 participants 5 messages Nov 4, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/2 kconfig: allow use of relations other than (in)equality

Over the years I found it desirable to be able to use all sorts of relations, not just (in)equality. And apparently I'm not the only one, as there's at least one example in the tree where the programmer assumed this would work (see DEBUG_UART_8250_WORD in arch/arm/Kconfig.debug). Another possibly use would e.g. be to fold the two SMP/NR_CPUS prompt into one: SMP could be promptless, simply depending on NR_CPUS > 1.

A (desirable) side effect of this change - resulting from numeric values now necessarily being compared as numbers rather than as strings - is that comparing hex values now works as expected: Other than int ones (which aren't allowed to have leading zeroes), zeroes following the 0x prefix made them compare unequal even if their values were equal.

Question: Should "<>" and/or "==" then perhaps also be permitted?

---
 scripts/kconfig/expr.c              |  170 ++++++++++-
 scripts/kconfig/expr.h              |    4 
 scripts/kconfig/symbol.c            |    4 
 scripts/kconfig/zconf.l             |    4 
 scripts/kconfig/zconf.lex.c_shipped |  291 +++++++++++--------
 scripts/kconfig/zconf.tab.c_shipped |  524 +++++++++++++++++++-----------------
 scripts/kconfig/zconf.y             |    9 
 7 files changed, 627 insertions(+), 379 deletions(-)
scripts/kconfig/expr.c +160 -10
@@ -76,6 +76,10 @@ struct expr *expr_copy(const struct expr
 		e->left.expr = expr_copy(org->left.expr);
 		break;
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 		e->left.sym = org->left.sym;
 		e->right.sym = org->right.sym;
@@ -108,6 +112,10 @@ void expr_free(struct expr *e)
 		expr_free(e->left.expr);
 		return;
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 		break;
 	case E_OR:
@@ -194,6 +202,10 @@ int expr_eq(struct expr *e1, struct expr
 		return 0;
 	switch (e1->type) {
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 		return e1->left.sym == e2->left.sym && e1->right.sym == e2->right.sym;
 	case E_SYMBOL:
@@ -641,6 +653,10 @@ struct expr *expr_transform(struct expr 
 		return NULL;
 	switch (e->type) {
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 	case E_SYMBOL:
 	case E_LIST:
@@ -713,6 +729,22 @@ struct expr *expr_transform(struct expr 
 			e = tmp;
 			e->type = e->type == E_EQUAL ? E_UNEQUAL : E_EQUAL;
 			break;
+		case E_LEQ:
+		case E_GEQ:
+			// !a<='x' -> a>'x'
+			tmp = e->left.expr;
+			free(e);
+			e = tmp;
+			e->type = e->type == E_LEQ ? E_GTH : E_LTH;
+			break;
+		case E_LTH:
+		case E_GTH:
+			// !a<'x' -> a>='x'
+			tmp = e->left.expr;
+			free(e);
+			e = tmp;
+			e->type = e->type == E_LTH ? E_GEQ : E_LEQ;
+			break;
 		case E_OR:
 			// !(a || b) -> !a && !b
 			tmp = e->left.expr;
@@ -783,6 +815,10 @@ int expr_contains_symbol(struct expr *de
 	case E_SYMBOL:
 		return dep->left.sym == sym;
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 		return dep->left.sym == sym ||
 		       dep->right.sym == sym;
@@ -908,6 +944,10 @@ struct expr *expr_trans_compare(struct e
 	case E_NOT:
 		return expr_trans_compare(e->left.expr, type == E_EQUAL ? E_UNEQUAL : E_EQUAL, sym);
 	case E_UNEQUAL:
+	case E_LTH:
+	case E_LEQ:
+	case E_GTH:
+	case E_GEQ:
 	case E_EQUAL:
 		if (type == E_EQUAL) {
 			if (sym == &symbol_yes)
@@ -935,10 +975,57 @@ struct expr *expr_trans_compare(struct e
 	return NULL;
 }
 
+enum string_value_kind {
+	k_string,
+	k_signed,
+	k_unsigned,
+	k_invalid
+};
+
+union string_value {
+	unsigned long long u;
+	signed long long s;
+};
+
+static enum string_value_kind expr_parse_string(const char *str,
+						enum symbol_type type,
+						union string_value *val)
+{
+	char *tail;
+	enum string_value_kind kind;
+
+	errno = 0;
+	switch (type) {
+	case S_BOOLEAN:
+	case S_TRISTATE:
+		return k_string;
+	case S_INT:
+		val->s = strtoll(str, &tail, 10);
+		kind = k_signed;
+		break;
+	case S_HEX:
+		val->u = strtoull(str, &tail, 16);
+		kind = k_unsigned;
+		break;
+	case S_STRING:
+	case S_UNKNOWN:
+		val->s = strtoll(str, &tail, 0);
+		kind = k_signed;
+		break;
+	default:
+		return k_invalid;
+	}
+	return !errno && !*tail && tail > str && isxdigit(tail[-1])
+	       ? kind : k_string;
+}
+
 tristate expr_calc_value(struct expr *e)
 {
 	tristate val1, val2;
 	const char *str1, *str2;
+	enum string_value_kind k1 = k_string, k2 = k_string;
+	union string_value lval = {}, rval = {};
+	int res;
 
 	if (!e)
 		return yes;
@@ -959,21 +1046,60 @@ tristate expr_calc_value(struct expr *e)
 		val1 = expr_calc_value(e->left.expr);
 		return EXPR_NOT(val1);
 	case E_EQUAL:
-		sym_calc_value(e->left.sym);
-		sym_calc_value(e->right.sym);
-		str1 = sym_get_string_value(e->left.sym);
-		str2 = sym_get_string_value(e->right.sym);
-		return !strcmp(str1, str2) ? yes : no;
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
-		sym_calc_value(e->left.sym);
-		sym_calc_value(e->right.sym);
-		str1 = sym_get_string_value(e->left.sym);
-		str2 = sym_get_string_value(e->right.sym);
-		return !strcmp(str1, str2) ? no : yes;
+		break;
 	default:
 		printf("expr_calc_value: %d?\n", e->type);
 		return no;
 	}
+
+	sym_calc_value(e->left.sym);
+	sym_calc_value(e->right.sym);
+	str1 = sym_get_string_value(e->left.sym);
+	str2 = sym_get_string_value(e->right.sym);
+
+	if (e->left.sym->type != S_STRING || e->right.sym->type != S_STRING) {
+		k1 = expr_parse_string(str1, e->left.sym->type, &lval);
+		k2 = expr_parse_string(str2, e->right.sym->type, &rval);
+	}
+
+	if (k1 == k_string || k2 == k_string)
+		res = strcmp(str1, str2);
+	else if (k1 == k_invalid || k2 == k_invalid) {
+		if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
+			printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
+printf("(%s -> %d, %s -> %d)\n", sym_type_name(e->left.sym->type), k1, sym_type_name(e->right.sym->type), k2);//temp
+			return no;
+		}
+		res = strcmp(str1, str2);
+	} else if (k1 == k_unsigned || k2 == k_unsigned)
+		res = (lval.u > rval.u) - (lval.u < rval.u);
+	else /* if (k1 == k_signed && k2 == k_signed) */
+		res = (lval.s > rval.s) - (lval.s < rval.s);
+
+	switch(e->type) {
+	case E_EQUAL:
+if(!strcmp(str1, str2) != !res) printf("EQU(\"%s\",\"%s\") -> %d/%d\n", str1, str2, strcmp(str1, str2), res);//temp
+		return res ? no : yes;
+	case E_GEQ:
+		return res >= 0 ? yes : no;
+	case E_GTH:
+		return res > 0 ? yes : no;
+	case E_LEQ:
+		return res <= 0 ? yes : no;
+	case E_LTH:
+		return res < 0 ? yes : no;
+	case E_UNEQUAL:
+if(!strcmp(str1, str2) != !res) printf("UEQ(\"%s\",\"%s\") -> %d/%d\n", str1, str2, strcmp(str1, str2), res);//temp
+		return res ? yes : no;
+	default:
+		printf("expr_calc_value: relation %d?\n", e->type);
+		return no;
+	}
 }
 
 int expr_compare_type(enum expr_type t1, enum expr_type t2)
@@ -984,6 +1110,12 @@ int expr_compare_type(enum expr_type t1,
 	if (t1 == t2)
 		return 0;
 	switch (t1) {
+	case E_LEQ:
+	case E_LTH:
+	case E_GEQ:
+	case E_GTH:
+		if (t2 == E_EQUAL || t2 == E_UNEQUAL)
+			return 1;
 	case E_EQUAL:
 	case E_UNEQUAL:
 		if (t2 == E_NOT)
@@ -1078,6 +1210,24 @@ void expr_print(struct expr *e, void (*f
 		fn(data, NULL, "=");
 		fn(data, e->right.sym, e->right.sym->name);
 		break;
+	case E_LEQ:
+	case E_LTH:
+		if (e->left.sym->name)
+			fn(data, e->left.sym, e->left.sym->name);
+		else
+			fn(data, NULL, "<choice>");
+		fn(data, NULL, e->type == E_LEQ ? "<=" : "<");
+		fn(data, e->right.sym, e->right.sym->name);
+		break;
+	case E_GEQ:
+	case E_GTH:
+		if (e->left.sym->name)
+			fn(data, e->left.sym, e->left.sym->name);
+		else
+			fn(data, NULL, "<choice>");
+		fn(data, NULL, e->type == E_LEQ ? ">=" : ">");
+		fn(data, e->right.sym, e->right.sym->name);
+		break;
 	case E_UNEQUAL:
 		if (e->left.sym->name)
 			fn(data, e->left.sym, e->left.sym->name);
scripts/kconfig/expr.h +3 -1
@@ -29,7 +29,9 @@ typedef enum tristate {
 } tristate;
 
 enum expr_type {
-	E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_LIST, E_SYMBOL, E_RANGE
+	E_NONE, E_OR, E_AND, E_NOT,
+	E_EQUAL, E_UNEQUAL, E_LTH, E_LEQ, E_GTH, E_GEQ,
+	E_LIST, E_SYMBOL, E_RANGE
 };
 
 union expr_data {
scripts/kconfig/symbol.c +4 -0
@@ -1166,6 +1166,10 @@ static struct symbol *sym_check_expr_dep
 	case E_NOT:
 		return sym_check_expr_deps(e->left.expr);
 	case E_EQUAL:
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
 		sym = sym_check_deps(e->left.sym);
 		if (sym)
scripts/kconfig/zconf.l +4 -0
@@ -122,6 +122,10 @@ n	[A-Za-z0-9_]
 	"!"	return T_NOT;
 	"="	return T_EQUAL;
 	"!="	return T_UNEQUAL;
+	"<="	return T_LESS_EQUAL;
+	">="	return T_GREATER_EQUAL;
+	"<"	return T_LESS;
+	">"	return T_GREATER;
 	\"|\'	{
 		str = yytext[0];
 		new_string();
scripts/kconfig/zconf.lex.c_shipped +164 -127

657 lines not on this page · raw message

scripts/kconfig/zconf.tab.c_shipped +283 -241

776 lines not on this page · raw message

scripts/kconfig/zconf.y +9 -0
@@ -69,6 +69,10 @@ static struct menu *current_menu, *curre
 %token <string> T_WORD
 %token <string> T_WORD_QUOTE
 %token T_UNEQUAL
+%token T_LESS
+%token T_LESS_EQUAL
+%token T_GREATER
+%token T_GREATER_EQUAL
 %token T_CLOSE_PAREN
 %token T_OPEN_PAREN
 %token T_EOL
@@ -76,6 +80,7 @@ static struct menu *current_menu, *curre
 %left T_OR
 %left T_AND
 %left T_EQUAL T_UNEQUAL
+%left T_LESS T_LESS_EQUAL T_GREATER T_GREATER_EQUAL
 %nonassoc T_NOT
 
 %type <string> prompt
@@ -467,6 +472,10 @@ if_expr:  /* empty */			{ $$ = NULL; }
 ;
 
 expr:	  symbol				{ $$ = expr_alloc_symbol($1); }
+	| symbol T_LESS symbol			{ $$ = expr_alloc_comp(E_LTH, $1, $3); }
+	| symbol T_LESS_EQUAL symbol		{ $$ = expr_alloc_comp(E_LEQ, $1, $3); }
+	| symbol T_GREATER symbol		{ $$ = expr_alloc_comp(E_GTH, $1, $3); }
+	| symbol T_GREATER_EQUAL symbol		{ $$ = expr_alloc_comp(E_GEQ, $1, $3); }
 	| symbol T_EQUAL symbol			{ $$ = expr_alloc_comp(E_EQUAL, $1, $3); }
 	| symbol T_UNEQUAL symbol		{ $$ = expr_alloc_comp(E_UNEQUAL, $1, $3); }
 	| T_OPEN_PAREN expr T_CLOSE_PAREN	{ $$ = $2; }
#1 lore raw

Just a quick reply (three odd lines I noted while unsuccessfully trying to apply this on top of my local changes).

Quoted 8 lines · Jan Beulich

Over the years I found it desirable to be able to use all sorts of relations, not just (in)equality. And apparently I'm not the only one, as there's at least one example in the tree where the programmer assumed this would work (see DEBUG_UART_8250_WORD in arch/arm/Kconfig.debug). Another possibly use would e.g. be to fold the two SMP/NR_CPUS prompt into one: SMP could be promptless, simply depending on NR_CPUS > 1.

Quoted 8 lines

A (desirable) side effect of this change - resulting from numeric values now necessarily being compared as numbers rather than as strings - is that comparing hex values now works as expected: Other than int ones (which aren't allowed to have leading zeroes), zeroes following the 0x prefix made them compare unequal even if their values were equal.

Question: Should "<>" and/or "==" then perhaps also be permitted?

You mean as aliases for "!=" and "="?

Quoted 45 lines
---
 scripts/kconfig/expr.c              |  170 ++++++++++-
 scripts/kconfig/expr.h              |    4 
 scripts/kconfig/symbol.c            |    4 
 scripts/kconfig/zconf.l             |    4 
 scripts/kconfig/zconf.lex.c_shipped |  291 +++++++++++--------
 scripts/kconfig/zconf.tab.c_shipped |  524 +++++++++++++++++++-----------------
 scripts/kconfig/zconf.y             |    9 
 7 files changed, 627 insertions(+), 379 deletions(-)
scripts/kconfig/expr.c +12 -10
[...]
@@ -959,21 +1046,60 @@ tristate expr_calc_value(struct expr *e)
 		val1 = expr_calc_value(e->left.expr);
 		return EXPR_NOT(val1);
 	case E_EQUAL:
-		sym_calc_value(e->left.sym);
-		sym_calc_value(e->right.sym);
-		str1 = sym_get_string_value(e->left.sym);
-		str2 = sym_get_string_value(e->right.sym);
-		return !strcmp(str1, str2) ? yes : no;
+	case E_GEQ:
+	case E_GTH:
+	case E_LEQ:
+	case E_LTH:
 	case E_UNEQUAL:
-		sym_calc_value(e->left.sym);
-		sym_calc_value(e->right.sym);
-		str1 = sym_get_string_value(e->left.sym);
-		str2 = sym_get_string_value(e->right.sym);
-		return !strcmp(str1, str2) ? no : yes;
+		break;
 	default:
 		printf("expr_calc_value: %d?\n", e->type);
 		return no;
 	}
+
+	sym_calc_value(e->left.sym);
+	sym_calc_value(e->right.sym);
+	str1 = sym_get_string_value(e->left.sym);
+	str2 = sym_get_string_value(e->right.sym);
+
+	if (e->left.sym->type != S_STRING || e->right.sym->type != S_STRING) {

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e) · line 1075

Quoted 10 lines
scripts/kconfig/expr.c +10 -0
+		k1 = expr_parse_string(str1, e->left.sym->type, &lval);
+		k2 = expr_parse_string(str2, e->right.sym->type, &rval);
+	}
+
+	if (k1 == k_string || k2 == k_string)
+		res = strcmp(str1, str2);
+	else if (k1 == k_invalid || k2 == k_invalid) {
+		if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
+			printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
+printf("(%s -> %d, %s -> %d)\n", sym_type_name(e->left.sym->type), k1, sym_type_name(e->right.sym->type), k2);//temp

Leftover from development?

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e) · line 1086

Quoted 11 lines
scripts/kconfig/expr.c +11 -0
+			return no;
+		}
+		res = strcmp(str1, str2);
+	} else if (k1 == k_unsigned || k2 == k_unsigned)
+		res = (lval.u > rval.u) - (lval.u < rval.u);
+	else /* if (k1 == k_signed && k2 == k_signed) */
+		res = (lval.s > rval.s) - (lval.s < rval.s);
+
+	switch(e->type) {
+	case E_EQUAL:
+if(!strcmp(str1, str2) != !res) printf("EQU(\"%s\",\"%s\") -> %d/%d\n", str1, str2, strcmp(str1, str2), res);//temp

Ditto.

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e) · line 1097

Quoted 11 lines
scripts/kconfig/expr.c +11 -0
+		return res ? no : yes;
+	case E_GEQ:
+		return res >= 0 ? yes : no;
+	case E_GTH:
+		return res > 0 ? yes : no;
+	case E_LEQ:
+		return res <= 0 ? yes : no;
+	case E_LTH:
+		return res < 0 ? yes : no;
+	case E_UNEQUAL:
+if(!strcmp(str1, str2) != !res) printf("UEQ(\"%s\",\"%s\") -> %d/%d\n", str1, str2, strcmp(str1, str2), res);//temp

Ditto.

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e) · line 1105

Quoted 8 lines
scripts/kconfig/expr.c +5 -0
+		return res ? yes : no;
+	default:
+		printf("expr_calc_value: relation %d?\n", e->type);
+		return no;
+	}
 }
 
 int expr_compare_type(enum expr_type t1, enum expr_type t2)

Paul Bolle

#2 lore raw
Quoted 5 lines
Quoted 1 line
Quoted 1 line · On 04.11.14 at 14:14, <[email protected]> wrote:
Quoted 1 line · Jan Beulich

Question: Should "<>" and/or "==" then perhaps also be permitted?

You mean as aliases for "!=" and "="?

Yes.

Quoted 9 lines

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e)

Quoted 6 lines
scripts/kconfig/expr.c +6 -0
+	if (k1 == k_string || k2 == k_string)
+		res = strcmp(str1, str2);
+	else if (k1 == k_invalid || k2 == k_invalid) {
+		if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
+			printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
+printf("(%s -> %d, %s -> %d)\n", sym_type_name(e->left.sym->type), k1, 

sym_type_name(e->right.sym->type), k2);//temp

Leftover from development?

Argh - yes, definitely; I even have a note in the patch that these should get dropped, but I managed to ignore that note.

Jan

#3 lore raw
Quoted 8 lines · Jan Beulich
Quoted 8 lines

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e)

Quoted 6 lines
scripts/kconfig/expr.c +6 -0
+	if (k1 == k_string || k2 == k_string)
+		res = strcmp(str1, str2);
+	else if (k1 == k_invalid || k2 == k_invalid) {
+		if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
+			printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
+printf("(%s -> %d, %s -> %d)\n", sym_type_name(e->left.sym->type), k1, 

sym_type_name(e->right.sym->type), k2);//temp

Quoted 4 lines
Quoted 1 line

Leftover from development?

Argh - yes, definitely; I even have a note in the patch that these should get dropped, but I managed to ignore that note.

If all you have to change is drop those three lines, I'll play with this patch anyway, as I finally maltreated it enough to apply on top of my local changes.

Paul Bolle

#4 lore raw
Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 04.11.14 at 14:28, <[email protected]> wrote:
Quoted 8 lines · Jan Beulich
Quoted 8 lines

scripts/kconfig/expr.c tristate expr_calc_value(struct expr *e)

Quoted 6 lines
scripts/kconfig/expr.c +6 -0
+	if (k1 == k_string || k2 == k_string)
+		res = strcmp(str1, str2);
+	else if (k1 == k_invalid || k2 == k_invalid) {
+		if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
+			printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
+printf("(%s -> %d, %s -> %d)\n", sym_type_name(e->left.sym->type), k1, 

sym_type_name(e->right.sym->type), k2);//temp

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Leftover from development?

Argh - yes, definitely; I even have a note in the patch that these should get dropped, but I managed to ignore that note.

If all you have to change is drop those three lines, I'll play with this patch anyway, as I finally maltreated it enough to apply on top of my local changes.

Yes, unless other comments require me to make further changes, this is all I currently see needing adjustment and re-sending.

Jan