[PATCH 1/6] kconfig: do not call check_conf() for olddefconfig

[PATCH 1/6] kconfig: do not call check_conf() for olddefconfig

[Masahiro Yamada]

check_conf() traverses the menu tree, but it is completely no-op for
olddefconfig because the following if-else block does nothing.

    if (input_mode == listnewconfig) {
            ...
    } else if (input_mode != olddefconfig) {
            ...
    }

As the help message says, olddefconfig automatically sets new symbols
to their default value.  There is no room for manual intervention.
So, calling check_conf() for olddefconfig is odd in the first place.
Rather, olddefconfig belongs to the group of alldefconfig and defconfig.

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

 scripts/kconfig/conf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 866369f..52cbe5d 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -434,7 +434,7 @@ static void check_conf(struct menu *menu)
 				if (sym->name && !sym_is_choice_value(sym)) {
 					printf("%s%s\n", CONFIG_, sym->name);
 				}
-			} else if (input_mode != olddefconfig) {
+			} else {
 				if (!conf_cnt++)
 					printf(_("*\n* Restart config...\n*\n"));
 				rootEntry = menu_get_parent_menu(menu);
@@ -674,15 +674,15 @@ int main(int ac, char **av)
 		/* fall through */
 	case oldconfig:
 	case listnewconfig:
-	case olddefconfig:
 	case silentoldconfig:
 		/* Update until a loop caused no more changes */
 		do {
 			conf_cnt = 0;
 			check_conf(&rootmenu);
-		} while (conf_cnt &&
-			 (input_mode != listnewconfig &&
-			  input_mode != olddefconfig));
+		} while (conf_cnt && input_mode != listnewconfig);
+		break;
+	case olddefconfig:
+	default:
 		break;
 	}
 
-- 
2.7.4

[PATCH 2/6] kconfig: call sym_calc_value() for all symbols before writing to .config

[Masahiro Yamada]

conf_write() skips sym_calc_value() for "choice", but we do not need
to do so.

conf_set_all_new_symbols() may have already called sym_calc_value()
for "choice", but set_all_choice_value() has cleared SYMBOL_VALID away.
So, conf_write() re-calculates "choice" here when calculating the
visibility of its children (choice value).

We can pass NULL to sym_calc_value() since it is no-op.

This should give no impact on behavior, but make the logic more easier
to understand.

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

 scripts/kconfig/confdata.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 027f5b4..bc83965 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -790,6 +790,8 @@ int conf_write(const char *name)
 	menu = rootmenu.list;
 	while (menu) {
 		sym = menu->sym;
+		sym_calc_value(sym);
+
 		if (!sym) {
 			if (!menu_is_visible(menu))
 				goto next;
@@ -799,7 +801,6 @@ int conf_write(const char *name)
 				     "# %s\n"
 				     "#\n", str);
 		} else if (!(sym->flags & SYMBOL_CHOICE)) {
-			sym_calc_value(sym);
 			if (!(sym->flags & SYMBOL_WRITE))
 				goto next;
 			sym->flags &= ~SYMBOL_WRITE;
-- 
2.7.4

[PATCH 3/6] kconfig: do not call conf_set_all_new_symbols() for (all)defconfig

[Masahiro Yamada]

(all)defconfig does not need to call conf_set_all_new_symbols() because
conf_write() calculates all symbols based on their default unless they
have been user-defined.

conf_set_all_new_symbols(def_default) is no-op except for "choice".
It calls sym_cal_value() for "choice", but the SYMBOL_VALID is cleared
by set_all_choice_values() later on.  So, conf_write() will re-calculate
it, which is just a wasteful computation.  The only difference I see
is SYMBOL_DEF_USER flag for "choice", but it gives no difference to the
resulted .config file.

I confirmed this change still produced the same .config for all ARCH.

I also moved savedefconfig to collect the no-op cases at the end of
the switch statement.

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

 scripts/kconfig/conf.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 52cbe5d..8364811 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -655,18 +655,10 @@ int main(int ac, char **av)
 	case allmodconfig:
 		conf_set_all_new_symbols(def_mod);
 		break;
-	case alldefconfig:
-		conf_set_all_new_symbols(def_default);
-		break;
 	case randconfig:
 		/* Really nothing to do in this loop */
 		while (conf_set_all_new_symbols(def_random)) ;
 		break;
-	case defconfig:
-		conf_set_all_new_symbols(def_default);
-		break;
-	case savedefconfig:
-		break;
 	case oldaskconfig:
 		rootEntry = &rootmenu;
 		conf(&rootmenu);
@@ -681,7 +673,10 @@ int main(int ac, char **av)
 			check_conf(&rootmenu);
 		} while (conf_cnt && input_mode != listnewconfig);
 		break;
+	case alldefconfig:
+	case defconfig:
 	case olddefconfig:
+	case savedefconfig:
 	default:
 		break;
 	}
-- 
2.7.4

[PATCH 4/6] kconfig: remove unneeded input_mode test in conf()

[Masahiro Yamada]

conf() is never called for listnewconfig / olddefconfig.

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

 scripts/kconfig/conf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 8364811..693cd5f 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -368,9 +368,7 @@ static void conf(struct menu *menu)
 
 		switch (prop->type) {
 		case P_MENU:
-			if ((input_mode == silentoldconfig ||
-			     input_mode == listnewconfig ||
-			     input_mode == olddefconfig) &&
+			if (input_mode == silentoldconfig &&
 			    rootEntry != menu) {
 				check_conf(menu);
 				return;
-- 
2.7.4

[PATCH 5/6] kconfig: remove redundant input_mode test for check_conf() loop

[Masahiro Yamada]

check_conf() never increments conf_cnt for listnewconfig, so conf_cnt
is always zero.

In other words, conf_cnt is not zero, "input_mode != listnewconfig"
is met.

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

 scripts/kconfig/conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 693cd5f..1d2ed3e 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -669,7 +669,7 @@ int main(int ac, char **av)
 		do {
 			conf_cnt = 0;
 			check_conf(&rootmenu);
-		} while (conf_cnt && input_mode != listnewconfig);
+		} while (conf_cnt);
 		break;
 	case alldefconfig:
 	case defconfig:
-- 
2.7.4

[PATCH 6/6] kconfig: hide irrelevant menu for oldconfig

[Masahiro Yamada]

Historically, "make oldconfig" has changed its behavior several times,
quieter or louder.  (I attached the history below.)  Currently, it is
not as quiet as it should be.  This commit addresses it.

  Test Case
  ---------

---------------------------(Kconfig)----------------------------
menu "menu"

config FOO
        bool "foo"

menu "sub menu"

config BAR
        bool "bar"

endmenu

endmenu

menu "sibling menu"

config BAZ
        bool "baz"

endmenu
----------------------------------------------------------------

---------------------------(.config)----------------------------
CONFIG_BAR=y
CONFIG_BAZ=y
----------------------------------------------------------------

With the Kconfig and .config above, "make silentoldconfig" and
"make oldconfig" work differently as follows:

  $ make silentoldconfig
  scripts/kconfig/conf  --silentoldconfig Kconfig
  *
  * Restart config...
  *
  *
  * menu
  *
  foo (FOO) [N/y] (NEW) y
  #
  # configuration written to .config
  #

  $ make oldconfig
  scripts/kconfig/conf  --oldconfig Kconfig
  *
  * Restart config...
  *
  *
  * menu
  *
  foo (FOO) [N/y] (NEW) y
  *
  * sub menu
  *
  bar (BAR) [Y/n] y
  #
  # configuration written to .config
  #

Both hide "sibling node" since it is irrelevant.  The difference is
that silentoldconfig hides "sub menu" whereas oldconfig does not.
The behavior of silentoldconfig is preferred since the "sub menu"
does not contain any new symbol.

The root cause is in conf().  There are three input modes that can
call conf(); oldaskconfig, oldconfig, and silentoldconfig.

Everytime conf() encounters a menu entry, it calls check_conf() to
check if it contains new symbols.  If no new symbol is found, the
menu is just skipped.

Currently, this happens only when input_mode == silentoldconfig.
The oldaskconfig enters into the check_conf() loop as silentoldconfig,
so oldaskconfig works likewise for the second loop or later, but it
never happens for oldconfig.  So, irrelevant sub-menus are shown for
oldconfig.

Change the test condition to "input_mode != oldaskconfig".  This is
false only for the first loop of oldaskconfig; it must ask the user
all symbols, so no need to call check_conf().

  History of oldconfig
  --------------------

[0] Originally, "make oldconfig" was as loud as "make config"  (It
    showed the entire .config file)

[1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty")
    made oldconfig quieter, but it was still less quieter than
    silentoldconfig.  (oldconfig did not hide sub-menus)

[2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed
    the input_mode of oldconfig to "ask_silent" from "ask_new".
    So, oldconfig really became as quiet as silentoldconfig.
    (oldconfig hided irrelevant sub-menus)

[3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made
    oldconfig as loud as [0] due to misconversion.

[4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed
    the misconversion of [3], but it made oldconfig quieter only to
    the same level as [1], not [2].

This commit is restoring the behavior of [2].

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

 scripts/kconfig/conf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1d2ed3e..8b9cdf4 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -368,8 +368,7 @@ static void conf(struct menu *menu)
 
 		switch (prop->type) {
 		case P_MENU:
-			if (input_mode == silentoldconfig &&
-			    rootEntry != menu) {
+			if (input_mode != oldaskconfig && rootEntry != menu) {
 				check_conf(menu);
 				return;
 			}
@@ -660,7 +659,7 @@ int main(int ac, char **av)
 	case oldaskconfig:
 		rootEntry = &rootmenu;
 		conf(&rootmenu);
-		input_mode = silentoldconfig;
+		input_mode = oldconfig;
 		/* fall through */
 	case oldconfig:
 	case listnewconfig:
-- 
2.7.4

Re: [PATCH 1/6] kconfig: do not call check_conf() for olddefconfig

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> check_conf() traverses the menu tree, but it is completely no-op for
> olddefconfig because the following if-else block does nothing.
>
>     if (input_mode == listnewconfig) {
>             ...
>     } else if (input_mode != olddefconfig) {
>             ...
>     }
>
> As the help message says, olddefconfig automatically sets new symbols
> to their default value.  There is no room for manual intervention.
> So, calling check_conf() for olddefconfig is odd in the first place.
> Rather, olddefconfig belongs to the group of alldefconfig and defconfig.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 866369f..52cbe5d 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -434,7 +434,7 @@ static void check_conf(struct menu *menu)
>                                 if (sym->name && !sym_is_choice_value(sym)) {
>                                         printf("%s%s\n", CONFIG_, sym->name);
>                                 }
> -                       } else if (input_mode != olddefconfig) {
> +                       } else {
>                                 if (!conf_cnt++)
>                                         printf(_("*\n* Restart config...\n*\n"));
>                                 rootEntry = menu_get_parent_menu(menu);
> @@ -674,15 +674,15 @@ int main(int ac, char **av)
>                 /* fall through */
>         case oldconfig:
>         case listnewconfig:
> -       case olddefconfig:
>         case silentoldconfig:
>                 /* Update until a loop caused no more changes */
>                 do {
>                         conf_cnt = 0;
>                         check_conf(&rootmenu);
> -               } while (conf_cnt &&
> -                        (input_mode != listnewconfig &&
> -                         input_mode != olddefconfig));
> +               } while (conf_cnt && input_mode != listnewconfig);
> +               break;
> +       case olddefconfig:
> +       default:
>                 break;
>         }
>
> --
> 2.7.4
>

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

Re: [PATCH 2/6] kconfig: call sym_calc_value() for all symbols before writing to .config

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> conf_write() skips sym_calc_value() for "choice", but we do not need
> to do so.
>
> conf_set_all_new_symbols() may have already called sym_calc_value()
> for "choice", but set_all_choice_value() has cleared SYMBOL_VALID away.
> So, conf_write() re-calculates "choice" here when calculating the
> visibility of its children (choice value).
>
> We can pass NULL to sym_calc_value() since it is no-op.
>
> This should give no impact on behavior, but make the logic more easier
> to understand.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/confdata.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 027f5b4..bc83965 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -790,6 +790,8 @@ int conf_write(const char *name)
>         menu = rootmenu.list;
>         while (menu) {
>                 sym = menu->sym;
> +               sym_calc_value(sym);
> +
>                 if (!sym) {
>                         if (!menu_is_visible(menu))
>                                 goto next;
> @@ -799,7 +801,6 @@ int conf_write(const char *name)
>                                      "# %s\n"
>                                      "#\n", str);
>                 } else if (!(sym->flags & SYMBOL_CHOICE)) {

Unrelated, but this could use !sym_is_choice(sym) as well.

> -                       sym_calc_value(sym);
>                         if (!(sym->flags & SYMBOL_WRITE))
>                                 goto next;
>                         sym->flags &= ~SYMBOL_WRITE;
> --
> 2.7.4
>

Not tested, but looks fine.

As a note if anyone else takes a look, choices get propagated to the
property conditions of the contained symbols, which indirectly causes
the choice to be calculated when the visibility of a contained symbol
is calculated.

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

Re: [PATCH 3/6] kconfig: do not call conf_set_all_new_symbols() for (all)defconfig

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> (all)defconfig does not need to call conf_set_all_new_symbols() because
> conf_write() calculates all symbols based on their default unless they
> have been user-defined.
>
> conf_set_all_new_symbols(def_default) is no-op except for "choice".
> It calls sym_cal_value() for "choice", but the SYMBOL_VALID is cleared
> by set_all_choice_values() later on.  So, conf_write() will re-calculate
> it, which is just a wasteful computation.  The only difference I see
> is SYMBOL_DEF_USER flag for "choice", but it gives no difference to the
> resulted .config file.
>
> I confirmed this change still produced the same .config for all ARCH.
>
> I also moved savedefconfig to collect the no-op cases at the end of
> the switch statement.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 52cbe5d..8364811 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -655,18 +655,10 @@ int main(int ac, char **av)
>         case allmodconfig:
>                 conf_set_all_new_symbols(def_mod);
>                 break;
> -       case alldefconfig:
> -               conf_set_all_new_symbols(def_default);
> -               break;
>         case randconfig:
>                 /* Really nothing to do in this loop */
>                 while (conf_set_all_new_symbols(def_random)) ;
>                 break;
> -       case defconfig:
> -               conf_set_all_new_symbols(def_default);
> -               break;
> -       case savedefconfig:
> -               break;
>         case oldaskconfig:
>                 rootEntry = &rootmenu;
>                 conf(&rootmenu);
> @@ -681,7 +673,10 @@ int main(int ac, char **av)
>                         check_conf(&rootmenu);
>                 } while (conf_cnt && input_mode != listnewconfig);
>                 break;
> +       case alldefconfig:
> +       case defconfig:
>         case olddefconfig:
> +       case savedefconfig:
>         default:
>                 break;
>         }
> --
> 2.7.4
>

Another side effect of conf_set_all_new_symbols(def_default) is that
SYMBOL_NEED_SET_CHOICE_VALUES is set on all choice values. Might want
to check fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
selects options that another choice menu depends on") to make sure
nothing breaks related to that.

Cheers,
Ulf

Re: [PATCH 4/6] kconfig: remove unneeded input_mode test in conf()

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> conf() is never called for listnewconfig / olddefconfig.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 8364811..693cd5f 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -368,9 +368,7 @@ static void conf(struct menu *menu)
>
>                 switch (prop->type) {
>                 case P_MENU:
> -                       if ((input_mode == silentoldconfig ||
> -                            input_mode == listnewconfig ||
> -                            input_mode == olddefconfig) &&
> +                       if (input_mode == silentoldconfig &&
>                             rootEntry != menu) {
>                                 check_conf(menu);
>                                 return;
> --
> 2.7.4
>

LGTM

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

Re: [PATCH 5/6] kconfig: remove redundant input_mode test for check_conf() loop

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> check_conf() never increments conf_cnt for listnewconfig, so conf_cnt
> is always zero.
>
> In other words, conf_cnt is not zero, "input_mode != listnewconfig"
> is met.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 693cd5f..1d2ed3e 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -669,7 +669,7 @@ int main(int ac, char **av)
>                 do {
>                         conf_cnt = 0;
>                         check_conf(&rootmenu);
> -               } while (conf_cnt && input_mode != listnewconfig);
> +               } while (conf_cnt);
>                 break;
>         case alldefconfig:
>         case defconfig:
> --
> 2.7.4
>

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

Re: [PATCH 6/6] kconfig: hide irrelevant menu for oldconfig

[Ulf Magnusson]

On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Historically, "make oldconfig" has changed its behavior several times,
> quieter or louder.  (I attached the history below.)  Currently, it is
> not as quiet as it should be.  This commit addresses it.
>
>   Test Case
>   ---------
>
> ---------------------------(Kconfig)----------------------------
> menu "menu"
>
> config FOO
>         bool "foo"
>
> menu "sub menu"
>
> config BAR
>         bool "bar"
>
> endmenu
>
> endmenu
>
> menu "sibling menu"
>
> config BAZ
>         bool "baz"
>
> endmenu
> ----------------------------------------------------------------
>
> ---------------------------(.config)----------------------------
> CONFIG_BAR=y
> CONFIG_BAZ=y
> ----------------------------------------------------------------
>
> With the Kconfig and .config above, "make silentoldconfig" and
> "make oldconfig" work differently as follows:
>
>   $ make silentoldconfig
>   scripts/kconfig/conf  --silentoldconfig Kconfig
>   *
>   * Restart config...
>   *
>   *
>   * menu
>   *
>   foo (FOO) [N/y] (NEW) y
>   #
>   # configuration written to .config
>   #
>
>   $ make oldconfig
>   scripts/kconfig/conf  --oldconfig Kconfig
>   *
>   * Restart config...
>   *
>   *
>   * menu
>   *
>   foo (FOO) [N/y] (NEW) y
>   *
>   * sub menu
>   *
>   bar (BAR) [Y/n] y
>   #
>   # configuration written to .config
>   #
>
> Both hide "sibling node" since it is irrelevant.  The difference is
> that silentoldconfig hides "sub menu" whereas oldconfig does not.
> The behavior of silentoldconfig is preferred since the "sub menu"
> does not contain any new symbol.
>
> The root cause is in conf().  There are three input modes that can
> call conf(); oldaskconfig, oldconfig, and silentoldconfig.
>
> Everytime conf() encounters a menu entry, it calls check_conf() to
> check if it contains new symbols.  If no new symbol is found, the
> menu is just skipped.
>
> Currently, this happens only when input_mode == silentoldconfig.
> The oldaskconfig enters into the check_conf() loop as silentoldconfig,
> so oldaskconfig works likewise for the second loop or later, but it
> never happens for oldconfig.  So, irrelevant sub-menus are shown for
> oldconfig.
>
> Change the test condition to "input_mode != oldaskconfig".  This is
> false only for the first loop of oldaskconfig; it must ask the user
> all symbols, so no need to call check_conf().
>
>   History of oldconfig
>   --------------------
>
> [0] Originally, "make oldconfig" was as loud as "make config"  (It
>     showed the entire .config file)
>
> [1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty")
>     made oldconfig quieter, but it was still less quieter than
>     silentoldconfig.  (oldconfig did not hide sub-menus)
>
> [2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed
>     the input_mode of oldconfig to "ask_silent" from "ask_new".
>     So, oldconfig really became as quiet as silentoldconfig.
>     (oldconfig hided irrelevant sub-menus)
>
> [3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made
>     oldconfig as loud as [0] due to misconversion.
>
> [4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed
>     the misconversion of [3], but it made oldconfig quieter only to
>     the same level as [1], not [2].
>
> This commit is restoring the behavior of [2].
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 1d2ed3e..8b9cdf4 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -368,8 +368,7 @@ static void conf(struct menu *menu)
>
>                 switch (prop->type) {
>                 case P_MENU:
> -                       if (input_mode == silentoldconfig &&
> -                           rootEntry != menu) {
> +                       if (input_mode != oldaskconfig && rootEntry != menu) {
>                                 check_conf(menu);
>                                 return;
>                         }
> @@ -660,7 +659,7 @@ int main(int ac, char **av)
>         case oldaskconfig:
>                 rootEntry = &rootmenu;
>                 conf(&rootmenu);
> -               input_mode = silentoldconfig;
> +               input_mode = oldconfig;
>                 /* fall through */
>         case oldconfig:
>         case listnewconfig:
> --
> 2.7.4
>

Looks good to me.

Maybe a comment along the following lines would be nice for
check_conf() as well:

/*
 * Recursively process modifiable symbols (and choices) in 'menu' that don't
 * have a user value
 */

check_conf() could be renamed to something like
process_new_modifiable_syms() as well.

The check_conf() call in conf() could have this above it as well:

/*
 * Except in oldaskconfig mode, we're only interested in new modifiable symbols
 */

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

Re: [PATCH 1/6] kconfig: do not call check_conf() for olddefconfig

[Ulf Magnusson]

On Fri, Jan 12, 2018 at 1:26 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> check_conf() traverses the menu tree, but it is completely no-op for
>> olddefconfig because the following if-else block does nothing.
>>
>>     if (input_mode == listnewconfig) {
>>             ...
>>     } else if (input_mode != olddefconfig) {
>>             ...
>>     }
>>
>> As the help message says, olddefconfig automatically sets new symbols
>> to their default value.  There is no room for manual intervention.
>> So, calling check_conf() for olddefconfig is odd in the first place.
>> Rather, olddefconfig belongs to the group of alldefconfig and defconfig.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 866369f..52cbe5d 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -434,7 +434,7 @@ static void check_conf(struct menu *menu)
>>                                 if (sym->name && !sym_is_choice_value(sym)) {
>>                                         printf("%s%s\n", CONFIG_, sym->name);
>>                                 }
>> -                       } else if (input_mode != olddefconfig) {
>> +                       } else {
>>                                 if (!conf_cnt++)
>>                                         printf(_("*\n* Restart config...\n*\n"));
>>                                 rootEntry = menu_get_parent_menu(menu);
>> @@ -674,15 +674,15 @@ int main(int ac, char **av)
>>                 /* fall through */
>>         case oldconfig:
>>         case listnewconfig:
>> -       case olddefconfig:
>>         case silentoldconfig:
>>                 /* Update until a loop caused no more changes */
>>                 do {
>>                         conf_cnt = 0;
>>                         check_conf(&rootmenu);
>> -               } while (conf_cnt &&
>> -                        (input_mode != listnewconfig &&
>> -                         input_mode != olddefconfig));
>> +               } while (conf_cnt && input_mode != listnewconfig);
>> +               break;
>> +       case olddefconfig:
>> +       default:
>>                 break;
>>         }
>>
>> --
>> 2.7.4
>>
>
> Acked-by: Ulf Magnusson <ulfalizer@gmail.com>
>
> Cheers,
> Ulf

Should have been Reviewed-by, obviously. Sorry about that. :)

Cheers,
Ulf

Re: [PATCH 3/6] kconfig: do not call conf_set_all_new_symbols() for (all)defconfig

[Masahiro Yamada]

Hi Ulf,

2018-01-12 23:28 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> (all)defconfig does not need to call conf_set_all_new_symbols() because
>> conf_write() calculates all symbols based on their default unless they
>> have been user-defined.
>>
>> conf_set_all_new_symbols(def_default) is no-op except for "choice".
>> It calls sym_cal_value() for "choice", but the SYMBOL_VALID is cleared
>> by set_all_choice_values() later on.  So, conf_write() will re-calculate
>> it, which is just a wasteful computation.  The only difference I see
>> is SYMBOL_DEF_USER flag for "choice", but it gives no difference to the
>> resulted .config file.
>>
>> I confirmed this change still produced the same .config for all ARCH.
>>
>> I also moved savedefconfig to collect the no-op cases at the end of
>> the switch statement.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 52cbe5d..8364811 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -655,18 +655,10 @@ int main(int ac, char **av)
>>         case allmodconfig:
>>                 conf_set_all_new_symbols(def_mod);
>>                 break;
>> -       case alldefconfig:
>> -               conf_set_all_new_symbols(def_default);
>> -               break;
>>         case randconfig:
>>                 /* Really nothing to do in this loop */
>>                 while (conf_set_all_new_symbols(def_random)) ;
>>                 break;
>> -       case defconfig:
>> -               conf_set_all_new_symbols(def_default);
>> -               break;
>> -       case savedefconfig:
>> -               break;
>>         case oldaskconfig:
>>                 rootEntry = &rootmenu;
>>                 conf(&rootmenu);
>> @@ -681,7 +673,10 @@ int main(int ac, char **av)
>>                         check_conf(&rootmenu);
>>                 } while (conf_cnt && input_mode != listnewconfig);
>>                 break;
>> +       case alldefconfig:
>> +       case defconfig:
>>         case olddefconfig:
>> +       case savedefconfig:
>>         default:
>>                 break;
>>         }
>> --
>> 2.7.4
>>
>
> Another side effect of conf_set_all_new_symbols(def_default) is that
> SYMBOL_NEED_SET_CHOICE_VALUES is set on all choice values. Might want
> to check fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
> selects options that another choice menu depends on") to make sure
> nothing breaks related to that.
>
> Cheers,
> Ulf
> --

Thanks for your comments!

>From a quick test, this patch seems OK,
but I need to take a closer look at this.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/6] kconfig: do not call conf_set_all_new_symbols() for (all)defconfig

[Masahiro Yamada]

Hi Ulf,

2018-01-12 23:28 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> (all)defconfig does not need to call conf_set_all_new_symbols() because
>> conf_write() calculates all symbols based on their default unless they
>> have been user-defined.
>>
>> conf_set_all_new_symbols(def_default) is no-op except for "choice".
>> It calls sym_cal_value() for "choice", but the SYMBOL_VALID is cleared
>> by set_all_choice_values() later on.  So, conf_write() will re-calculate
>> it, which is just a wasteful computation.  The only difference I see
>> is SYMBOL_DEF_USER flag for "choice", but it gives no difference to the
>> resulted .config file.
>>
>> I confirmed this change still produced the same .config for all ARCH.
>>
>> I also moved savedefconfig to collect the no-op cases at the end of
>> the switch statement.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 52cbe5d..8364811 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -655,18 +655,10 @@ int main(int ac, char **av)
>>         case allmodconfig:
>>                 conf_set_all_new_symbols(def_mod);
>>                 break;
>> -       case alldefconfig:
>> -               conf_set_all_new_symbols(def_default);
>> -               break;
>>         case randconfig:
>>                 /* Really nothing to do in this loop */
>>                 while (conf_set_all_new_symbols(def_random)) ;
>>                 break;
>> -       case defconfig:
>> -               conf_set_all_new_symbols(def_default);
>> -               break;
>> -       case savedefconfig:
>> -               break;
>>         case oldaskconfig:
>>                 rootEntry = &rootmenu;
>>                 conf(&rootmenu);
>> @@ -681,7 +673,10 @@ int main(int ac, char **av)
>>                         check_conf(&rootmenu);
>>                 } while (conf_cnt && input_mode != listnewconfig);
>>                 break;
>> +       case alldefconfig:
>> +       case defconfig:
>>         case olddefconfig:
>> +       case savedefconfig:
>>         default:
>>                 break;
>>         }
>> --
>> 2.7.4
>>
>
> Another side effect of conf_set_all_new_symbols(def_default) is that
> SYMBOL_NEED_SET_CHOICE_VALUES is set on all choice values. Might want
> to check fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
> selects options that another choice menu depends on") to make sure
> nothing breaks related to that.
>
> Cheers,
> Ulf
> --

Thanks for your comments!

From a quick test, this patch seems OK,
but I need to take a closer look at this.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/6] kconfig: do not call check_conf() for olddefconfig

[Masahiro Yamada]

Hi Ulf,

2018-01-13 1:49 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Fri, Jan 12, 2018 at 1:26 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>> On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> check_conf() traverses the menu tree, but it is completely no-op for
>>> olddefconfig because the following if-else block does nothing.
>>>
>>>     if (input_mode == listnewconfig) {
>>>             ...
>>>     } else if (input_mode != olddefconfig) {
>>>             ...
>>>     }
>>>
>>> As the help message says, olddefconfig automatically sets new symbols
>>> to their default value.  There is no room for manual intervention.
>>> So, calling check_conf() for olddefconfig is odd in the first place.
>>> Rather, olddefconfig belongs to the group of alldefconfig and defconfig.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 866369f..52cbe5d 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -434,7 +434,7 @@ static void check_conf(struct menu *menu)
>>>                                 if (sym->name && !sym_is_choice_value(sym)) {
>>>                                         printf("%s%s\n", CONFIG_, sym->name);
>>>                                 }
>>> -                       } else if (input_mode != olddefconfig) {
>>> +                       } else {
>>>                                 if (!conf_cnt++)
>>>                                         printf(_("*\n* Restart config...\n*\n"));
>>>                                 rootEntry = menu_get_parent_menu(menu);
>>> @@ -674,15 +674,15 @@ int main(int ac, char **av)
>>>                 /* fall through */
>>>         case oldconfig:
>>>         case listnewconfig:
>>> -       case olddefconfig:
>>>         case silentoldconfig:
>>>                 /* Update until a loop caused no more changes */
>>>                 do {
>>>                         conf_cnt = 0;
>>>                         check_conf(&rootmenu);
>>> -               } while (conf_cnt &&
>>> -                        (input_mode != listnewconfig &&
>>> -                         input_mode != olddefconfig));
>>> +               } while (conf_cnt && input_mode != listnewconfig);
>>> +               break;
>>> +       case olddefconfig:
>>> +       default:
>>>                 break;
>>>         }
>>>
>>> --
>>> 2.7.4
>>>
>>
>> Acked-by: Ulf Magnusson <ulfalizer@gmail.com>
>>
>> Cheers,
>> Ulf
>
> Should have been Reviewed-by, obviously. Sorry about that. :)
>


I will turn it into Reviewed-by.

Anyway, thanks a lot for checking these patches!

-- 
Best Regards
Masahiro Yamada