diff mbox series

[v4,1/2] kconfig: warn unmet direct dependency of tristate symbols selected by y

Message ID 1520934968-30406-1-git-send-email-yamada.masahiro@socionext.com
State Accepted
Commit f622f8279581626170154d77d67e67bcb227ca2f
Headers show
Series [v4,1/2] kconfig: warn unmet direct dependency of tristate symbols selected by y | expand

Commit Message

Masahiro Yamada March 13, 2018, 9:56 a.m. UTC 
Commit 246cf9c26bf1 ("kbuild: Warn on selecting symbols with unmet
direct dependencies") forcibly promoted ->dir_dep.tri to yes from mod.
So, the unmet direct dependencies of tristate symbols are not reported.

[Test Case]

  config MODULES
          def_bool y
          option modules

  config A
          def_bool y
          select B

  config B
          tristate "B"
          depends on m

This causes unmet dependency because 'B' is forced 'y' ignoring
'depends on m'.  This should be warned.

On the other hand, the following case ('B' is bool) should not be
warned, so 'depends on m' for bool symbols should be naturally treated
as 'depends on y'.

[Test Case2 (not unmet dependency)]

  config MODULES
          def_bool y
          option modules

  config A
          def_bool y
          select B

  config B
          bool "B"
          depends on m

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

Changes in v4:
  - newly added


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

-- 
2.7.4

Comments

Eugeniu Rosca March 18, 2018, 10:17 p.m. UTC | #1 
Hi Masahiro,

On Tue, Mar 13, 2018 at 06:56:07PM +0900, Masahiro Yamada wrote:
> Commit 246cf9c26bf1 ("kbuild: Warn on selecting symbols with unmet

> direct dependencies") forcibly promoted ->dir_dep.tri to yes from mod.

> So, the unmet direct dependencies of tristate symbols are not reported.

> 

> [Test Case]

> 

>   config MODULES

>           def_bool y

>           option modules

> 

>   config A

>           def_bool y

>           select B

> 

>   config B

>           tristate "B"

>           depends on m

> 

> This causes unmet dependency because 'B' is forced 'y' ignoring

> 'depends on m'.  This should be warned.

> 

> On the other hand, the following case ('B' is bool) should not be

> warned, so 'depends on m' for bool symbols should be naturally treated

> as 'depends on y'.

> 

> [Test Case2 (not unmet dependency)]

> 

>   config MODULES

>           def_bool y

>           option modules

> 

>   config A

>           def_bool y

>           select B

> 

>   config B

>           bool "B"

>           depends on m

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

> 

> Changes in v4:

>   - newly added

> 

> 

>  scripts/kconfig/symbol.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c

> index 0f7eba7..def860b 100644

> --- a/scripts/kconfig/symbol.c

> +++ b/scripts/kconfig/symbol.c

> @@ -243,7 +243,7 @@ static void sym_calc_visibility(struct symbol *sym)

>  	tri = yes;

>  	if (sym->dir_dep.expr)

>  		tri = expr_calc_value(sym->dir_dep.expr);

> -	if (tri == mod)

> +	if (sym->type == S_BOOLEAN && tri == mod)


This seems to be very close to one of the issues fixed in the old
v2.6.5 commit 5a87d187fce1 ("[PATCH] config: choice fix") accessible at
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?h=5a87d187fce1

IMHO you could probably use (not tested!):
+	if (tri == mod && sym_get_type(sym) == S_BOOLEAN)

instead of
+	if (sym->type == S_BOOLEAN && tri == mod)

to be consistent with the rest of sym_calc_visibility().

Also, since after this commit there will be a great amount of symmetry
between how dirdep/revdep/weak-revdep are calculated inside
sym_calc_visibility(), somebody could probably unify the implementations
in future (not subject of this patch).

>  		tri = yes;

>  	if (sym->dir_dep.tri != tri) {

>  		sym->dir_dep.tri = tri;

> @@ -414,7 +414,7 @@ void sym_calc_value(struct symbol *sym)

>  				}

>  			}

>  		calc_newval:

> -			if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) {

> +			if (sym->dir_dep.tri < sym->rev_dep.tri) {


Looks good.

>  				struct expr *e;

>  				e = expr_simplify_unmet_dep(sym->rev_dep.expr,

>  				    sym->dir_dep.expr);

> -- 

> 2.7.4

> 


Best regards,
Eugeniu.
Masahiro Yamada March 20, 2018, 3:57 a.m. UTC | #2 
2018-03-19 7:17 GMT+09:00 Eugeniu Rosca <roscaeugeniu@gmail.com>:
> Hi Masahiro,

>

> On Tue, Mar 13, 2018 at 06:56:07PM +0900, Masahiro Yamada wrote:

>> Commit 246cf9c26bf1 ("kbuild: Warn on selecting symbols with unmet

>> direct dependencies") forcibly promoted ->dir_dep.tri to yes from mod.

>> So, the unmet direct dependencies of tristate symbols are not reported.

>>

>> [Test Case]

>>

>>   config MODULES

>>           def_bool y

>>           option modules

>>

>>   config A

>>           def_bool y

>>           select B

>>

>>   config B

>>           tristate "B"

>>           depends on m

>>

>> This causes unmet dependency because 'B' is forced 'y' ignoring

>> 'depends on m'.  This should be warned.

>>

>> On the other hand, the following case ('B' is bool) should not be

>> warned, so 'depends on m' for bool symbols should be naturally treated

>> as 'depends on y'.

>>

>> [Test Case2 (not unmet dependency)]

>>

>>   config MODULES

>>           def_bool y

>>           option modules

>>

>>   config A

>>           def_bool y

>>           select B

>>

>>   config B

>>           bool "B"

>>           depends on m

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> ---

>>

>> Changes in v4:

>>   - newly added

>>

>>

>>  scripts/kconfig/symbol.c | 4 ++--

>>  1 file changed, 2 insertions(+), 2 deletions(-)

>>

>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c

>> index 0f7eba7..def860b 100644

>> --- a/scripts/kconfig/symbol.c

>> +++ b/scripts/kconfig/symbol.c

>> @@ -243,7 +243,7 @@ static void sym_calc_visibility(struct symbol *sym)

>>       tri = yes;

>>       if (sym->dir_dep.expr)

>>               tri = expr_calc_value(sym->dir_dep.expr);

>> -     if (tri == mod)

>> +     if (sym->type == S_BOOLEAN && tri == mod)

>

> This seems to be very close to one of the issues fixed in the old

> v2.6.5 commit 5a87d187fce1 ("[PATCH] config: choice fix") accessible at

> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?h=5a87d187fce1

>

> IMHO you could probably use (not tested!):

> +       if (tri == mod && sym_get_type(sym) == S_BOOLEAN)

>

> instead of

> +       if (sym->type == S_BOOLEAN && tri == mod)

>

> to be consistent with the rest of sym_calc_visibility().



I tested it, and probably OK.

I will squash the diff.

Thanks.




> Also, since after this commit there will be a great amount of symmetry

> between how dirdep/revdep/weak-revdep are calculated inside

> sym_calc_visibility(), somebody could probably unify the implementations

> in future (not subject of this patch).

>

>>               tri = yes;

>>       if (sym->dir_dep.tri != tri) {

>>               sym->dir_dep.tri = tri;

>> @@ -414,7 +414,7 @@ void sym_calc_value(struct symbol *sym)

>>                               }

>>                       }

>>               calc_newval:

>> -                     if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) {

>> +                     if (sym->dir_dep.tri < sym->rev_dep.tri) {

>

> Looks good.

>

>>                               struct expr *e;

>>                               e = expr_simplify_unmet_dep(sym->rev_dep.expr,

>>                                   sym->dir_dep.expr);

>> --

>> 2.7.4

>>

>

> Best regards,

> Eugeniu.

> --

> 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




-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 0f7eba7..def860b 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -243,7 +243,7 @@  static void sym_calc_visibility(struct symbol *sym)
 	tri = yes;
 	if (sym->dir_dep.expr)
 		tri = expr_calc_value(sym->dir_dep.expr);
-	if (tri == mod)
+	if (sym->type == S_BOOLEAN && tri == mod)
 		tri = yes;
 	if (sym->dir_dep.tri != tri) {
 		sym->dir_dep.tri = tri;
@@ -414,7 +414,7 @@  void sym_calc_value(struct symbol *sym)
 				}
 			}
 		calc_newval:
-			if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) {
+			if (sym->dir_dep.tri < sym->rev_dep.tri) {
 				struct expr *e;
 				e = expr_simplify_unmet_dep(sym->rev_dep.expr,
 				    sym->dir_dep.expr);