kconfig: print more info when we see a recursive dependency

Consider following kconfig file: config TEST1 bool "test 1" depends on TEST2 config TEST2 bool "test 2" depends on TEST1 Previously kconfig would report: foo:6:error: found recursive dependency: TEST2 -> TEST1 -> TEST2 With the following patch kconfig reports: foo:5:error: recursive dependency detected! foo:5: symbol TEST2 depends on TEST1 foo:1: symbol TEST1 depends on TEST2 Note that we now report where the offending symbols are defined. This can be a great help for complex situations involving several files. Patch is originally from Roman Zippel with a few adjustments by Sam. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Roman Zippel <zippel@linux-m68k.org> Signed-off-by: Michal Marek <mmarek@suse.cz>

kconfig: print more info when we see a recursive dependency
Consider following kconfig file: config TEST1 bool "test 1" depends on TEST2 config TEST2 bool "test 2" depends on TEST1 Previously kconfig would report: foo:6:error: found recursive dependency: TEST2 -> TEST1 -> TEST2 With the following patch kconfig reports: foo:5:error: recursive dependency detected! foo:5: symbol TEST2 depends on TEST1 foo:1: symbol TEST1 depends on TEST2 Note that we now report where the offending symbols are defined. This can be a great help for complex situations involving several files. Patch is originally from Roman Zippel with a few adjustments by Sam. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Roman Zippel <zippel@linux-m68k.org> Signed-off-by: Michal Marek <mmarek@suse.cz>
Roman Zippel · Michal Marek
1 parent 59e89e3ddf
Showing 1 changed file with 127 additions and 15 deletions Side-by-side Diff
scripts/kconfig/symbol.c
@@ -783,7 +783,111 @@
 	return sym_arr;
 }
  
+/*
+ * When we check for recursive dependencies we use a stack to save
+ * current state so we can print out relevant info to user.
+ * The entries are located on the call stack so no need to free memory.
+ * Note inser() remove() must always match to properly clear the stack.
+ */
+static struct dep_stack {
+	struct dep_stack *prev, *next;
+	struct symbol *sym;
+	struct property *prop;
+	struct expr *expr;
+} *check_top;
  
+static void dep_stack_insert(struct dep_stack *stack, struct symbol *sym)
+{
+	memset(stack, 0, sizeof(*stack));
+	if (check_top)
+		check_top->next = stack;
+	stack->prev = check_top;
+	stack->sym = sym;
+	check_top = stack;
+}
+
+static void dep_stack_remove(void)
+{
+	check_top = check_top->prev;
+	if (check_top)
+		check_top->next = NULL;
+}
+
+/*
+ * Called when we have detected a recursive dependency.
+ * check_top point to the top of the stact so we use
+ * the ->prev pointer to locate the bottom of the stack.
+ */
+static void sym_check_print_recursive(struct symbol *last_sym)
+{
+	struct dep_stack *stack;
+	struct symbol *sym, *next_sym;
+	struct menu *menu = NULL;
+	struct property *prop;
+	struct dep_stack cv_stack;
+
+	if (sym_is_choice_value(last_sym)) {
+		dep_stack_insert(&cv_stack, last_sym);
+		last_sym = prop_get_symbol(sym_get_choice_prop(last_sym));
+	}
+
+	for (stack = check_top; stack != NULL; stack = stack->prev)
+		if (stack->sym == last_sym)
+			break;
+	if (!stack) {
+		fprintf(stderr, "unexpected recursive dependency error\n");
+		return;
+	}
+
+	for (; stack; stack = stack->next) {
+		sym = stack->sym;
+		next_sym = stack->next ? stack->next->sym : last_sym;
+		prop = stack->prop;
+
+		/* for choice values find the menu entry (used below) */
+		if (sym_is_choice(sym) || sym_is_choice_value(sym)) {
+			for (prop = sym->prop; prop; prop = prop->next) {
+				menu = prop->menu;
+				if (prop->menu)
+					break;
+			}
+		}
+		if (stack->sym == last_sym)
+			fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
+				prop->file->name, prop->lineno);
+		if (stack->expr) {
+			fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
+				prop->file->name, prop->lineno,
+				sym->name ? sym->name : "<choice>",
+				prop_get_type_name(prop->type),
+				next_sym->name ? next_sym->name : "<choice>");
+		} else if (stack->prop) {
+			fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
+				prop->file->name, prop->lineno,
+				sym->name ? sym->name : "<choice>",
+				next_sym->name ? next_sym->name : "<choice>");
+		} else if (sym_is_choice(sym)) {
+			fprintf(stderr, "%s:%d:\tchoice %s contains symbol %s\n",
+				menu->file->name, menu->lineno,
+				sym->name ? sym->name : "<choice>",
+				next_sym->name ? next_sym->name : "<choice>");
+		} else if (sym_is_choice_value(sym)) {
+			fprintf(stderr, "%s:%d:\tsymbol %s is part of choice %s\n",
+				menu->file->name, menu->lineno,
+				sym->name ? sym->name : "<choice>",
+				next_sym->name ? next_sym->name : "<choice>");
+		} else {
+			fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
+				prop->file->name, prop->lineno,
+				sym->name ? sym->name : "<choice>",
+				next_sym->name ? next_sym->name : "<choice>");
+		}
+	}
+
+	if (check_top == &cv_stack)
+		dep_stack_remove();
+}
+
 static struct symbol *sym_check_expr_deps(struct expr *e)
 {
 	struct symbol *sym;
  
  
  
  
  
  
@@ -819,24 +923,33 @@
 {
 	struct symbol *sym2;
 	struct property *prop;
+	struct dep_stack stack;
  
+	dep_stack_insert(&stack, sym);
+
 	sym2 = sym_check_expr_deps(sym->rev_dep.expr);
 	if (sym2)
-		return sym2;
+		goto out;
  
 	for (prop = sym->prop; prop; prop = prop->next) {
 		if (prop->type == P_CHOICE || prop->type == P_SELECT)
 			continue;
+		stack.prop = prop;
 		sym2 = sym_check_expr_deps(prop->visible.expr);
 		if (sym2)
 			break;
 		if (prop->type != P_DEFAULT || sym_is_choice(sym))
 			continue;
+		stack.expr = prop->expr;
 		sym2 = sym_check_expr_deps(prop->expr);
 		if (sym2)
 			break;
+		stack.expr = NULL;
 	}
  
+out:
+	dep_stack_remove();
+
 	return sym2;
 }
  
  
@@ -845,7 +958,10 @@
 	struct symbol *sym, *sym2;
 	struct property *prop;
 	struct expr *e;
+	struct dep_stack stack;
  
+	dep_stack_insert(&stack, choice);
+
 	prop = sym_get_choice_prop(choice);
 	expr_list_for_each_sym(prop->expr, e, sym)
 		sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
  
@@ -858,10 +974,8 @@
  
 	expr_list_for_each_sym(prop->expr, e, sym) {
 		sym2 = sym_check_sym_deps(sym);
-		if (sym2) {
-			fprintf(stderr, " -> %s", sym->name);
+		if (sym2)
 			break;
-		}
 	}
 out:
 	expr_list_for_each_sym(prop->expr, e, sym)
@@ -871,6 +985,8 @@
 	    prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
 		sym2 = choice;
  
+	dep_stack_remove();
+
 	return sym2;
 }
  
  
  
  
@@ -880,18 +996,20 @@
 	struct property *prop;
  
 	if (sym->flags & SYMBOL_CHECK) {
-		fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
-		        sym->prop->file->name, sym->prop->lineno,
-			sym->name ? sym->name : "<choice>");
+		sym_check_print_recursive(sym);
 		return sym;
 	}
 	if (sym->flags & SYMBOL_CHECKED)
 		return NULL;
  
 	if (sym_is_choice_value(sym)) {
+		struct dep_stack stack;
+
 		/* for choice groups start the check with main choice symbol */
+		dep_stack_insert(&stack, sym);
 		prop = sym_get_choice_prop(sym);
 		sym2 = sym_check_deps(prop_get_symbol(prop));
+		dep_stack_remove();
 	} else if (sym_is_choice(sym)) {
 		sym2 = sym_check_choice_deps(sym);
 	} else {
@@ -900,14 +1018,8 @@
 		sym->flags &= ~SYMBOL_CHECK;
 	}
  
-	if (sym2) {
-		fprintf(stderr, " -> %s", sym->name ? sym->name : "<choice>");
-		if (sym2 == sym) {
-			fprintf(stderr, "\n");
-			zconfnerrs++;
-			sym2 = NULL;
-		}
-	}
+	if (sym2 && sym2 == sym)
+		sym2 = NULL;
  
 	return sym2;
 }
...	...	@@ -783,7 +783,111 @@
783	783	return sym_arr;
784	784	}
785	785
	786	+/*
	787	+ * When we check for recursive dependencies we use a stack to save
	788	+ * current state so we can print out relevant info to user.
	789	+ * The entries are located on the call stack so no need to free memory.
	790	+ * Note inser() remove() must always match to properly clear the stack.
	791	+ */
	792	+static struct dep_stack {
	793	+ struct dep_stack prev, next;
	794	+ struct symbol *sym;
	795	+ struct property *prop;
	796	+ struct expr *expr;
	797	+} *check_top;
786	798
	799	+static void dep_stack_insert(struct dep_stack stack, struct symbol sym)
	800	+{
	801	+ memset(stack, 0, sizeof(*stack));
	802	+ if (check_top)
	803	+ check_top->next = stack;
	804	+ stack->prev = check_top;
	805	+ stack->sym = sym;
	806	+ check_top = stack;
	807	+}
	808	+
	809	+static void dep_stack_remove(void)
	810	+{
	811	+ check_top = check_top->prev;
	812	+ if (check_top)
	813	+ check_top->next = NULL;
	814	+}
	815	+
	816	+/*
	817	+ * Called when we have detected a recursive dependency.
	818	+ * check_top point to the top of the stact so we use
	819	+ * the ->prev pointer to locate the bottom of the stack.
	820	+ */
	821	+static void sym_check_print_recursive(struct symbol *last_sym)
	822	+{
	823	+ struct dep_stack *stack;
	824	+ struct symbol sym, next_sym;
	825	+ struct menu *menu = NULL;
	826	+ struct property *prop;
	827	+ struct dep_stack cv_stack;
	828	+
	829	+ if (sym_is_choice_value(last_sym)) {
	830	+ dep_stack_insert(&cv_stack, last_sym);
	831	+ last_sym = prop_get_symbol(sym_get_choice_prop(last_sym));
	832	+ }
	833	+
	834	+ for (stack = check_top; stack != NULL; stack = stack->prev)
	835	+ if (stack->sym == last_sym)
	836	+ break;
	837	+ if (!stack) {
	838	+ fprintf(stderr, "unexpected recursive dependency error\n");
	839	+ return;
	840	+ }
	841	+
	842	+ for (; stack; stack = stack->next) {
	843	+ sym = stack->sym;
	844	+ next_sym = stack->next ? stack->next->sym : last_sym;
	845	+ prop = stack->prop;
	846	+
	847	+ /* for choice values find the menu entry (used below) */
	848	+ if (sym_is_choice(sym) \|\| sym_is_choice_value(sym)) {
	849	+ for (prop = sym->prop; prop; prop = prop->next) {
	850	+ menu = prop->menu;
	851	+ if (prop->menu)
	852	+ break;
	853	+ }
	854	+ }
	855	+ if (stack->sym == last_sym)
	856	+ fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
	857	+ prop->file->name, prop->lineno);
	858	+ if (stack->expr) {
	859	+ fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
	860	+ prop->file->name, prop->lineno,
	861	+ sym->name ? sym->name : "<choice>",
	862	+ prop_get_type_name(prop->type),
	863	+ next_sym->name ? next_sym->name : "<choice>");
	864	+ } else if (stack->prop) {
	865	+ fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
	866	+ prop->file->name, prop->lineno,
	867	+ sym->name ? sym->name : "<choice>",
	868	+ next_sym->name ? next_sym->name : "<choice>");
	869	+ } else if (sym_is_choice(sym)) {
	870	+ fprintf(stderr, "%s:%d:\tchoice %s contains symbol %s\n",
	871	+ menu->file->name, menu->lineno,
	872	+ sym->name ? sym->name : "<choice>",
	873	+ next_sym->name ? next_sym->name : "<choice>");
	874	+ } else if (sym_is_choice_value(sym)) {
	875	+ fprintf(stderr, "%s:%d:\tsymbol %s is part of choice %s\n",
	876	+ menu->file->name, menu->lineno,
	877	+ sym->name ? sym->name : "<choice>",
	878	+ next_sym->name ? next_sym->name : "<choice>");
	879	+ } else {
	880	+ fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
	881	+ prop->file->name, prop->lineno,
	882	+ sym->name ? sym->name : "<choice>",
	883	+ next_sym->name ? next_sym->name : "<choice>");
	884	+ }
	885	+ }
	886	+
	887	+ if (check_top == &cv_stack)
	888	+ dep_stack_remove();
	889	+}
	890	+
787	891	static struct symbol sym_check_expr_deps(struct expr e)
788	892	{
789	893	struct symbol *sym;
790	894
791	895
792	896
793	897
794	898
795	899
...	...	@@ -819,24 +923,33 @@
819	923	{
820	924	struct symbol *sym2;
821	925	struct property *prop;
	926	+ struct dep_stack stack;
822	927
	928	+ dep_stack_insert(&stack, sym);
	929	+
823	930	sym2 = sym_check_expr_deps(sym->rev_dep.expr);
824	931	if (sym2)
825		- return sym2;
	932	+ goto out;
826	933
827	934	for (prop = sym->prop; prop; prop = prop->next) {
828	935	if (prop->type == P_CHOICE \|\| prop->type == P_SELECT)
829	936	continue;
	937	+ stack.prop = prop;
830	938	sym2 = sym_check_expr_deps(prop->visible.expr);
831	939	if (sym2)
832	940	break;
833	941	if (prop->type != P_DEFAULT \|\| sym_is_choice(sym))
834	942	continue;
	943	+ stack.expr = prop->expr;
835	944	sym2 = sym_check_expr_deps(prop->expr);
836	945	if (sym2)
837	946	break;
	947	+ stack.expr = NULL;
838	948	}
839	949
	950	+out:
	951	+ dep_stack_remove();
	952	+
840	953	return sym2;
841	954	}
842	955
843	956
...	...	@@ -845,7 +958,10 @@
845	958	struct symbol sym, sym2;
846	959	struct property *prop;
847	960	struct expr *e;
	961	+ struct dep_stack stack;
848	962
	963	+ dep_stack_insert(&stack, choice);
	964	+
849	965	prop = sym_get_choice_prop(choice);
850	966	expr_list_for_each_sym(prop->expr, e, sym)
851	967	sym->flags \|= (SYMBOL_CHECK \| SYMBOL_CHECKED);
852	968
...	...	@@ -858,10 +974,8 @@
858	974
859	975	expr_list_for_each_sym(prop->expr, e, sym) {
860	976	sym2 = sym_check_sym_deps(sym);
861		- if (sym2) {
862		- fprintf(stderr, " -> %s", sym->name);
	977	+ if (sym2)
863	978	break;
864		- }
865	979	}
866	980	out:
867	981	expr_list_for_each_sym(prop->expr, e, sym)
...	...	@@ -871,6 +985,8 @@
871	985	prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
872	986	sym2 = choice;
873	987
	988	+ dep_stack_remove();
	989	+
874	990	return sym2;
875	991	}
876	992
877	993
878	994
879	995
...	...	@@ -880,18 +996,20 @@
880	996	struct property *prop;
881	997
882	998	if (sym->flags & SYMBOL_CHECK) {
883		- fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
884		- sym->prop->file->name, sym->prop->lineno,
885		- sym->name ? sym->name : "<choice>");
	999	+ sym_check_print_recursive(sym);
886	1000	return sym;
887	1001	}
888	1002	if (sym->flags & SYMBOL_CHECKED)
889	1003	return NULL;
890	1004
891	1005	if (sym_is_choice_value(sym)) {
	1006	+ struct dep_stack stack;
	1007	+
892	1008	/* for choice groups start the check with main choice symbol */
	1009	+ dep_stack_insert(&stack, sym);
893	1010	prop = sym_get_choice_prop(sym);
894	1011	sym2 = sym_check_deps(prop_get_symbol(prop));
	1012	+ dep_stack_remove();
895	1013	} else if (sym_is_choice(sym)) {
896	1014	sym2 = sym_check_choice_deps(sym);
897	1015	} else {
...	...	@@ -900,14 +1018,8 @@
900	1018	sym->flags &= ~SYMBOL_CHECK;
901	1019	}
902	1020
903		- if (sym2) {
904		- fprintf(stderr, " -> %s", sym->name ? sym->name : "<choice>");
905		- if (sym2 == sym) {
906		- fprintf(stderr, "\n");
907		- zconfnerrs++;
908		- sym2 = NULL;
909		- }
910		- }
	1021	+ if (sym2 && sym2 == sym)
	1022	+ sym2 = NULL;
911	1023
912	1024	return sym2;
913	1025	}