diff mbox

[v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

Message ID ghinz04yaz.fsf@lena.gouders.net (mailing list archive)
State New, archived
Headers show

Commit Message

Dirk Gouders April 29, 2016, 8:24 a.m. UTC
If choices consist of choice_values of type tristate that depend on
symbols set to 'm', those choice_values are not set to 'n' if the
choice is changed from 'm' to 'y' (in which case only one active
choice_value is allowed). Those values are also written to the config
file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem;
with the frontend of your choice set "Choice 0" and "Choice 1" to 'm',
then set "Tristate Choice" to 'y' and save the configuration:

config modules
	boolean modules
	default y
	option modules

config dependency
	tristate "Dependency"
	default m

choice
	prompt "Tristate Choice"
	default choice0

config choice0
	tristate "Choice 0"

config choice1
	tristate "Choice 1"
	depends on dependency

endchoice

This patch sets tristate choice_values' visibility that depend on
symbols set to 'm' to 'n' if the corresponding choice is set to 'y'.

This makes them disappear from the choice list and will also cause the
choice_values' value set to 'n' in sym_calc_value() and as a result
they are written as "not set" to the resulting .config file.

Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v5: This patch should handle tristate choice-values, only.

    I assumed that only tristate choice-values can have visibility 'm',
    which was wrong: tristate dependencies can result in 'm'
    visibility.

    So, add an explicit test if a symbol is of type tristate.

    I am a bit unsure how to handle Tested-By credits when patches change
    substantially and left the credits untouched but new test reports
    are welcome.

---
 scripts/kconfig/symbol.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Roger Quadros May 2, 2016, 8:43 a.m. UTC | #1
On 29/04/16 11:24, Dirk Gouders wrote:
> If choices consist of choice_values of type tristate that depend on
> symbols set to 'm', those choice_values are not set to 'n' if the
> choice is changed from 'm' to 'y' (in which case only one active
> choice_value is allowed). Those values are also written to the config
> file causing modules to be built when they should not.
> 
> The following config can be used to reproduce and examine the problem;
> with the frontend of your choice set "Choice 0" and "Choice 1" to 'm',
> then set "Tristate Choice" to 'y' and save the configuration:
> 
> config modules
> 	boolean modules
> 	default y
> 	option modules
> 
> config dependency
> 	tristate "Dependency"
> 	default m
> 
> choice
> 	prompt "Tristate Choice"
> 	default choice0
> 
> config choice0
> 	tristate "Choice 0"
> 
> config choice1
> 	tristate "Choice 1"
> 	depends on dependency
> 
> endchoice
> 
> This patch sets tristate choice_values' visibility that depend on
> symbols set to 'm' to 'n' if the corresponding choice is set to 'y'.
> 
> This makes them disappear from the choice list and will also cause the
> choice_values' value set to 'n' in sym_calc_value() and as a result
> they are written as "not set" to the resulting .config file.
> 
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v5: This patch should handle tristate choice-values, only.
> 
>     I assumed that only tristate choice-values can have visibility 'm',
>     which was wrong: tristate dependencies can result in 'm'
>     visibility.
> 
>     So, add an explicit test if a symbol is of type tristate.
> 
>     I am a bit unsure how to handle Tested-By credits when patches change
>     substantially and left the credits untouched but new test reports
>     are welcome.

If you made a non cosmetic change old Tested-By tags aren't valid.

> 
> ---

The USB gadget case works fine for me. Thanks :)

Tested-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger

>  scripts/kconfig/symbol.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 25cf0c2..2432298 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -209,12 +209,26 @@ static void sym_set_all_changed(void)
>  static void sym_calc_visibility(struct symbol *sym)
>  {
>  	struct property *prop;
> +	struct symbol *choice_sym = NULL;
>  	tristate tri;
>  
>  	/* any prompt visible? */
>  	tri = no;
> +
> +	if (sym_is_choice_value(sym))
> +		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
> +
>  	for_all_prompts(sym, prop) {
>  		prop->visible.tri = expr_calc_value(prop->visible.expr);
> +		/*
> +		 * Tristate choice_values with visibility 'mod' are
> +		 * not visible if the corresponding choice's value is
> +		 * 'yes'.
> +		 */
> +		if (choice_sym && sym->type == S_TRISTATE &&
> +		    prop->visible.tri == mod && choice_sym->curr.tri == yes)
> +			prop->visible.tri = no;
> +
>  		tri = EXPR_OR(tri, prop->visible.tri);
>  	}
>  	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek May 10, 2016, 7:15 p.m. UTC | #2
On Mon, May 02, 2016 at 11:43:30AM +0300, Roger Quadros wrote:
> On 29/04/16 11:24, Dirk Gouders wrote:
> > If choices consist of choice_values of type tristate that depend on
> > symbols set to 'm', those choice_values are not set to 'n' if the
> > choice is changed from 'm' to 'y' (in which case only one active
> > choice_value is allowed). Those values are also written to the config
> > file causing modules to be built when they should not.
> > 
> > The following config can be used to reproduce and examine the problem;
> > with the frontend of your choice set "Choice 0" and "Choice 1" to 'm',
> > then set "Tristate Choice" to 'y' and save the configuration:
> > 
> > config modules
> > 	boolean modules
> > 	default y
> > 	option modules
> > 
> > config dependency
> > 	tristate "Dependency"
> > 	default m
> > 
> > choice
> > 	prompt "Tristate Choice"
> > 	default choice0
> > 
> > config choice0
> > 	tristate "Choice 0"
> > 
> > config choice1
> > 	tristate "Choice 1"
> > 	depends on dependency
> > 
> > endchoice
> > 
> > This patch sets tristate choice_values' visibility that depend on
> > symbols set to 'm' to 'n' if the corresponding choice is set to 'y'.
> > 
> > This makes them disappear from the choice list and will also cause the
> > choice_values' value set to 'n' in sym_calc_value() and as a result
> > they are written as "not set" to the resulting .config file.
> > 
> > Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Signed-off-by: Dirk Gouders <dirk@gouders.net>
> > Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > v5: This patch should handle tristate choice-values, only.
> > 
> >     I assumed that only tristate choice-values can have visibility 'm',
> >     which was wrong: tristate dependencies can result in 'm'
> >     visibility.
> > 
> >     So, add an explicit test if a symbol is of type tristate.
> > 
> >     I am a bit unsure how to handle Tested-By credits when patches change
> >     substantially and left the credits untouched but new test reports
> >     are welcome.
> 
> If you made a non cosmetic change old Tested-By tags aren't valid.
> 
> > 
> > ---
> 
> The USB gadget case works fine for me. Thanks :)
> 
> Tested-by: Roger Quadros <rogerq@ti.com>

Applied to kbuild.git#kconfig.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 25cf0c2..2432298 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -209,12 +209,26 @@  static void sym_set_all_changed(void)
 static void sym_calc_visibility(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *choice_sym = NULL;
 	tristate tri;
 
 	/* any prompt visible? */
 	tri = no;
+
+	if (sym_is_choice_value(sym))
+		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
 	for_all_prompts(sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
+		/*
+		 * Tristate choice_values with visibility 'mod' are
+		 * not visible if the corresponding choice's value is
+		 * 'yes'.
+		 */
+		if (choice_sym && sym->type == S_TRISTATE &&
+		    prop->visible.tri == mod && choice_sym->curr.tri == yes)
+			prop->visible.tri = no;
+
 		tri = EXPR_OR(tri, prop->visible.tri);
 	}
 	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))