[PATCH] kconfig: make 'imply' obey the direct dependency

* [PATCH] kconfig: make 'imply' obey the direct dependency
@ 2020-02-19  7:49 Masahiro Yamada
  2020-02-19  8:42 ` Geert Uytterhoeven
  2020-02-19 16:15 ` Nicolas Pitre
  0 siblings, 2 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-02-19  7:49 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Arnd Bergmann, Nicolas Pitre, Richard Cochran, Thomas Gleixner,
	John Stultz, Josh Triplett, Mark Brown, Ulf Magnusson,
	Geert Uytterhoeven, Masahiro Yamada, Jonathan Corbet,
	Michal Marek, linux-doc, linux-kernel

The 'imply' statement may create unmet direct dependency when the
implied symbol depends on m.

[Test Code]

  config FOO
          tristate "foo"
          imply BAZ

  config BAZ
          tristate "baz"
          depends on BAR

  config BAR
          def_tristate m

  config MODULES
          def_bool y
          option modules

If you set FOO=y, BAZ is also promoted to y, which results in the
following .config file:

  CONFIG_FOO=y
  CONFIG_BAZ=y
  CONFIG_BAR=m
  CONFIG_MODULES=y

This ignores the dependency "BAZ depends on BAR".

Unlike 'select', what is worse, Kconfig never shows the
"WARNING: unmet direct dependencies detected for ..." for this case.

Because 'imply' should be weaker than 'depends on', Kconfig should
take the direct dependency into account.

Describe this case in Documentation/kbuild/kconfig-language.rst for
clarification.

Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that
a symbol implied by y is restricted to y or n, excluding m.

As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded
by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the
possible value is BAZ=n.

Having said that, this case was probably "We don't care" at that time
because Kconfig did not handle 'depends on m' correctly until
commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate
symbols selected by y") fixed it.

Backporting this to 4.19+ will probably be fine. If you care this
problem on 4.14.x, you need to backport f622f8279581 as well.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Documentation/kbuild/kconfig-language.rst |  7 +++++--
 scripts/kconfig/symbol.c                  | 17 +++++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index d0111dd26410..9141f03ff744 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -159,11 +159,11 @@ applicable everywhere (see syntax).
   Given the following example::
 
     config FOO
-	tristate
+	tristate "foo"
 	imply BAZ
 
     config BAZ
-	tristate
+	tristate "baz"
 	depends on BAR
 
   The following values are possible:
@@ -174,6 +174,9 @@ applicable everywhere (see syntax).
 	n		y		n		N/m/y
 	m		y		m		M/y/n
 	y		y		y		Y/n
+	n		m		n		N/m
+	m		m		m		M/n
+	y		m		n		N
 	y		n		*		N
 	===		===		=============	==============
 
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 8d38b700b314..6100e6ead8ad 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -221,7 +221,7 @@ static void sym_calc_visibility(struct symbol *sym)
 		sym_set_changed(sym);
 	}
 	tri = no;
-	if (sym->implied.expr && sym->dir_dep.tri != no)
+	if (sym->implied.expr)
 		tri = expr_calc_value(sym->implied.expr);
 	if (tri == mod && sym_get_type(sym) == S_BOOLEAN)
 		tri = yes;
@@ -393,7 +393,17 @@ void sym_calc_value(struct symbol *sym)
 				}
 				if (sym->implied.tri != no) {
 					sym->flags |= SYMBOL_WRITE;
-					newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
+					newval.tri = EXPR_OR(newval.tri,
+							     sym->implied.tri);
+					newval.tri = EXPR_AND(newval.tri,
+							      sym->dir_dep.tri);
+
+					if (newval.tri == mod && sym->implied.tri == yes) {
+						if (sym->dir_dep.tri == yes)
+							newval.tri = yes;
+						else
+							newval.tri = no;
+					}
 				}
 			}
 		calc_newval:
@@ -401,8 +411,7 @@ void sym_calc_value(struct symbol *sym)
 				sym_warn_unmet_dep(sym);
 			newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
 		}
-		if (newval.tri == mod &&
-		    (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes))
+		if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
 			newval.tri = yes;
 		break;
 	case S_STRING:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread