Re: [PATCH 05/24] menu: Update bootmenu_autoboot_loop() to return the code

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 10/17/22 22:29, Simon Glass wrote:

...

Use the return value to save having to pass around a pointer. This also resolves any ambiguity about what *key contains when the function is called.

Signed-off-by: Simon Glass sjg@chromium.org

cmd/bootmenu.c | 2 +- common/menu.c | 16 +++++++++------- include/menu.h | 7 +++---- 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index c80004c54dc..0e22f504fe4 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -93,7 +93,7 @@ static char *bootmenu_choice_entry(void *data)

First of all this function should be moved to common/menu.c.

...

  while (1) {
          if (menu->delay >= 0) {
                  /* Autoboot was not stopped */

•                 bootmenu_autoboot_loop(menu, &key, &esc);
  

•                 key = bootmenu_autoboot_loop(menu, &esc);
          } else {
                  /* Some key was pressed, so autoboot was stopped */
                  bootmenu_loop(menu, &key, &esc);
  

diff --git a/common/menu.c b/common/menu.c index 087e4c246e2..bccc104a39b 100644 --- a/common/menu.c +++ b/common/menu.c @@ -425,9 +425,9 @@ int menu_destroy(struct menu *m) return 1; }

-void bootmenu_autoboot_loop(struct bootmenu_data *menu,

•                     enum bootmenu_key *key, int *esc)
  

+enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc) {

• enum bootmenu_key key = BKEY_NONE;
  int i, c;
  
  while (menu->delay > 0) {
  

@@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu, switch (c) { case '\e': *esc = 1;

•                         *key = BKEY_NONE;
  

•                         key = BKEY_NONE;
                          break;
                  case '\r':
  

•                         *key = BKEY_SELECT;
  

•                         key = BKEY_SELECT;
                          break;
                  case 0x3: /* ^C */
  

•                         *key = BKEY_QUIT;
  

•                         key = BKEY_QUIT;
                          break;
                  default:
  

•                         *key = BKEY_NONE;
  

•                         key = BKEY_NONE;
                          break;
                  }
  
  

@@ -471,7 +471,9 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu, printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);

  if (menu->delay == 0)

•         *key = BKEY_SELECT;
  

•         key = BKEY_SELECT;
  

• return key;
  

  }

  void bootmenu_loop(struct bootmenu_data *menu,

diff --git a/include/menu.h b/include/menu.h index 29b457921e9..9f30a3c1acd 100644 --- a/include/menu.h +++ b/include/menu.h @@ -65,14 +65,13 @@ enum bootmenu_key {

• indicating that the current option should be chosen.
• @menu: Menu being processed

• • @key: Returns the code for the key the user pressed:

• • @esc: Set to 1 if the escape key is pressed, otherwise not updated
• • Returns: code for the key the user pressed:
  • enter: KEY_SELECT
  • Ctrl-C: KEY_QUIT
  • anything else: KEY_NONE

• • @esc: Set to 1 if the escape key is pressed, otherwise not updated
  */

-void bootmenu_autoboot_loop(struct bootmenu_data *menu,

•                     enum bootmenu_key *key, int *esc);
  

+enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);

This function should not be exported.

Out of scope for this patch.

enum bootmenu cannot accomomdate accelerator keys. The return type should be int.

What accelerator keys? I don't see how they work at present.

Regards, Simon