menu: fix timeout duration

Message ID 1527136146-12665-1-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series
  • menu: fix timeout duration
Related show

Commit Message

Masahiro Yamada May 24, 2018, 4:29 a.m.
menu_interactive_choice() divides the timeout value by 10 before
passing it to cli_readline_into_buffer().

For distro-boot, the "timeout" variable in the boot script should
specify the time in _seconds_ to wait for keyboard input before
booting the default menu entry.

Due to the division, "timeout 50" actually wait for only 5 seconds
instead of 50.  What is worse, "timeout 5" never breaks because
"m->timeout / 10" is zero, which means no timeout.

For CONFIG_MENU_SHOW case, menu_show() should also take the timeout
value in seconds because its default comes from CONFIG_BOOTDELAY.

The "division by 10" was introduced by commit 8594753ba0a7 ("menu:
only timeout when menu is displayed").  Its log claimed "fixed",
but to me, it rather looks the root cause of the problem.

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

Rob,

I know commit 8594753ba0a7 is already 6 years ago.
If you remember something about "/ 10", please comment.


 common/menu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Masahiro Yamada May 24, 2018, 5:38 a.m. | #1
2018-05-24 13:29 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> menu_interactive_choice() divides the timeout value by 10 before
> passing it to cli_readline_into_buffer().
>
> For distro-boot, the "timeout" variable in the boot script should
> specify the time in _seconds_ to wait for keyboard input before
> booting the default menu entry.
>
> Due to the division, "timeout 50" actually wait for only 5 seconds
> instead of 50.  What is worse, "timeout 5" never breaks because
> "m->timeout / 10" is zero, which means no timeout.
>
> For CONFIG_MENU_SHOW case, menu_show() should also take the timeout
> value in seconds because its default comes from CONFIG_BOOTDELAY.
>
> The "division by 10" was introduced by commit 8594753ba0a7 ("menu:
> only timeout when menu is displayed").  Its log claimed "fixed",
> but to me, it rather looks the root cause of the problem.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Rob,
>
> I know commit 8594753ba0a7 is already 6 years ago.
> If you remember something about "/ 10", please comment.
>

I was misunderstanding.

I will do v2.



>  common/menu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/common/menu.c b/common/menu.c
> index bf2b471..bf23194 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -194,8 +194,7 @@ static inline int menu_interactive_choice(struct menu *m, void **choice)
>
>                 if (!m->item_choice) {
>                         readret = cli_readline_into_buffer("Enter choice: ",
> -                                                          cbuf,
> -                                                          m->timeout / 10);
> +                                                          cbuf, timeout);
>
>                         if (readret >= 0) {
>                                 choice_item = menu_item_by_key(m, cbuf);
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Patch

diff --git a/common/menu.c b/common/menu.c
index bf2b471..bf23194 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -194,8 +194,7 @@  static inline int menu_interactive_choice(struct menu *m, void **choice)
 
 		if (!m->item_choice) {
 			readret = cli_readline_into_buffer("Enter choice: ",
-							   cbuf,
-							   m->timeout / 10);
+							   cbuf, timeout);
 
 			if (readret >= 0) {
 				choice_item = menu_item_by_key(m, cbuf);