Re: [PATCH v2] kconfig: Add transitional symbol attribute for migration support

[Vegard Nossum <vegard.nossum@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]


On 04/09/2025 04:51, Kees Cook wrote:
> On Mon, Sep 01, 2025 at 08:44:56PM +0200, Vegard Nossum wrote:
>> If you change sym_calc_visibility() to always return 'yes' for
>> transitional values then I don't think you need to touch
>> sym_calc_value() at all.
> 
> Hm, it looks like sym_calc_visibility() doesn't strictly just look at
> visibility. And visibility seems to "last"? And I think the "tri" still
> can't just be "yes", don't we need the other stuff handled?
> 
> Do you see a way to do it how you're suggesting? And now I wrote the
> regression tests so we can test any alternatives! ;)

Here's what I had in mind (on top of your kcfi patchset), see the
attachment.

It basically undoes all your additions to sym_calc_value() in favour of
two straightforward additions:

@@ -214,6 +214,11 @@ static void sym_calc_visibility(struct symbol *sym)
         struct property *prop;
         tristate tri;

+       if (sym->flags & SYMBOL_HIDDEN) {
+               sym->visible = yes;
+               return;
+       }
+

and

@@ -536,7 +531,7 @@ void sym_calc_value(struct symbol *sym)
                 }
         }

-       if (sym_is_choice(sym))
+       if (sym_is_choice(sym) || sym->flags & SYMBOL_HIDDEN)
                 sym->flags &= ~SYMBOL_WRITE;
  }

Let me know how that works for you (the new test passes here).


Vegard

[-- Attachment #2: hidden.patch --]
[-- Type: text/x-patch, Size: 2101 bytes --]

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b2686dba05ece..37958ff57f707 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -214,6 +214,11 @@ static void sym_calc_visibility(struct symbol *sym)
 	struct property *prop;
 	tristate tri;
 
+	if (sym->flags & SYMBOL_HIDDEN) {
+		sym->visible = yes;
+		return;
+	}
+
 	/* any prompt visible? */
 	tri = no;
 	for_all_prompts(sym, prop) {
@@ -408,7 +413,6 @@ void sym_calc_value(struct symbol *sym)
 	struct symbol_value newval, oldval;
 	struct property *prop;
 	struct menu *choice_menu;
-	bool usable;
 
 	if (!sym)
 		return;
@@ -448,13 +452,6 @@ void sym_calc_value(struct symbol *sym)
 	if (sym->visible != no)
 		sym->flags |= SYMBOL_WRITE;
 
-	/*
-	 * For a symbol to be processed during configuration it needs to
-	 * be either visible or a transitional symbol that is hidden from
-	 * menus and omitted from newly written .config files.
-	 */
-	usable = (sym->visible != no || (sym->flags & SYMBOL_HIDDEN));
-
 	/* set default if recursively called */
 	sym->curr = newval;
 
@@ -467,15 +464,13 @@ void sym_calc_value(struct symbol *sym)
 			sym_calc_choice(choice_menu);
 			newval.tri = sym->curr.tri;
 		} else {
-			if (usable) {
+			if (sym->visible != no) {
 				/* if the symbol is visible use the user value
 				 * if available, otherwise try the default value
 				 */
 				if (sym_has_value(sym)) {
-					tristate value = (sym->flags & SYMBOL_HIDDEN) ?
-						sym->def[S_DEF_USER].tri : sym->visible;
 					newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri,
-							      value);
+							      sym->visible);
 					goto calc_newval;
 				}
 			}
@@ -507,7 +502,7 @@ void sym_calc_value(struct symbol *sym)
 	case S_STRING:
 	case S_HEX:
 	case S_INT:
-		if (usable && sym_has_value(sym)) {
+		if (sym->visible != no && sym_has_value(sym)) {
 			newval.val = sym->def[S_DEF_USER].val;
 			break;
 		}
@@ -536,7 +531,7 @@ void sym_calc_value(struct symbol *sym)
 		}
 	}
 
-	if (sym_is_choice(sym))
+	if (sym_is_choice(sym) || sym->flags & SYMBOL_HIDDEN)
 		sym->flags &= ~SYMBOL_WRITE;
 }