From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: u-boot@lists.denx.de, Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: Re: [PATCH v4 1/4] eficonfig: refactor eficonfig_process_common function
Date: Mon, 23 Jan 2023 15:04:21 +0200 [thread overview]
Message-ID: <Y86F1dHIZXg06sOy@hera> (raw)
In-Reply-To: <20230120084358.5919-2-masahisa.kojima@linaro.org>
On Fri, Jan 20, 2023 at 05:43:55PM +0900, Masahisa Kojima wrote:
> Current change boot order implementation does not call
> eficonfig_process_common() and call own menu functions
> for display_statusline, item_data_print and item_choice.
> Change boot order functionality should call
> eficonfig_process_common() to improve maintenanceability.
>
> This commit is a preparation to remove the change boot
> order specific implementation. The menu functions
> (display_statusline, item_data_print and item_choice) are
> added as argument of eficonfig_process_common().
> The menu description string displayed at the bottom of
> the menu is also added as argument.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> No update since v3
>
> Changes in v3:
> - modify "reverse" local variable type to bool
>
> Changes in v2:
> - add const qualifier to eficonfig_menu_desc, change it to pointer
>
> cmd/eficonfig.c | 71 +++++++++++++++++++++++++++++++++----------
> cmd/eficonfig_sbkey.c | 18 +++++++++--
> include/efi_config.h | 13 +++++++-
> 3 files changed, 82 insertions(+), 20 deletions(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index d830e4af53..24d6bdb6bf 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -22,6 +22,8 @@
> #include <linux/delay.h>
>
> static struct efi_simple_text_input_protocol *cin;
> +const char *eficonfig_menu_desc =
> + " Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit";
>
> #define EFICONFIG_DESCRIPTION_MAX 32
> #define EFICONFIG_OPTIONAL_DATA_MAX 64
> @@ -134,10 +136,10 @@ void eficonfig_print_msg(char *msg)
> *
> * @data: pointer to the data associated with each menu entry
> */
> -static void eficonfig_print_entry(void *data)
> +void eficonfig_print_entry(void *data)
> {
> struct eficonfig_entry *entry = data;
> - int reverse = (entry->efi_menu->active == entry->num);
> + bool reverse = (entry->efi_menu->active == entry->num);
>
> /* TODO: support scroll or page for many entries */
>
> @@ -161,7 +163,7 @@ static void eficonfig_print_entry(void *data)
> *
> * @m: pointer to the menu structure
> */
> -static void eficonfig_display_statusline(struct menu *m)
> +void eficonfig_display_statusline(struct menu *m)
> {
> struct eficonfig_entry *entry;
>
> @@ -171,10 +173,11 @@ static void eficonfig_display_statusline(struct menu *m)
> printf(ANSI_CURSOR_POSITION
> "\n%s\n"
> ANSI_CURSOR_POSITION ANSI_CLEAR_LINE ANSI_CURSOR_POSITION
> - " Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit"
> + "%s"
> ANSI_CLEAR_LINE_TO_END ANSI_CURSOR_POSITION ANSI_CLEAR_LINE,
> 1, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1,
> - entry->efi_menu->count + 6, 1, entry->efi_menu->count + 7, 1);
> + entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc,
> + entry->efi_menu->count + 7, 1);
> }
>
> /**
> @@ -183,7 +186,7 @@ static void eficonfig_display_statusline(struct menu *m)
> * @data: pointer to the efimenu structure
> * Return: key string to identify the selected entry
> */
> -static char *eficonfig_choice_entry(void *data)
> +char *eficonfig_choice_entry(void *data)
> {
> struct cli_ch_state s_cch, *cch = &s_cch;
> struct list_head *pos, *n;
> @@ -361,9 +364,17 @@ out:
> *
> * @efi_menu: pointer to the efimenu structure
> * @menu_header: pointer to the menu header string
> + * @menu_desc: pointer to the menu description
> + * @display_statusline: function pointer to draw statusline
> + * @item_data_print: function pointer to draw the menu item
> + * @item_choice: function pointer to handle the key press
> * Return: status code
> */
> -efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_header)
> +efi_status_t eficonfig_process_common(struct efimenu *efi_menu,
> + char *menu_header, const char *menu_desc,
> + void (*display_statusline)(struct menu *),
> + void (*item_data_print)(void *),
> + char *(*item_choice)(void *))
> {
> struct menu *menu;
> void *choice = NULL;
> @@ -382,10 +393,11 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_heade
> if (!efi_menu->menu_header)
> return EFI_OUT_OF_RESOURCES;
> }
> + if (menu_desc)
> + efi_menu->menu_desc = menu_desc;
>
> - menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,
> - eficonfig_print_entry, eficonfig_choice_entry,
> - efi_menu);
> + menu = menu_create(NULL, 0, 1, display_statusline, item_data_print,
> + item_choice, efi_menu);
> if (!menu)
> return EFI_INVALID_PARAMETER;
>
> @@ -644,7 +656,12 @@ static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *f
> if (ret != EFI_SUCCESS)
> goto out;
>
> - ret = eficonfig_process_common(efi_menu, " ** Select Volume **");
> + ret = eficonfig_process_common(efi_menu, " ** Select Volume **",
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> +
> out:
> efi_free_pool(volume_handles);
> list_for_each_safe(pos, n, &efi_menu->list) {
> @@ -819,7 +836,11 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
> if (ret != EFI_SUCCESS)
> goto err;
>
> - ret = eficonfig_process_common(efi_menu, " ** Select File **");
> + ret = eficonfig_process_common(efi_menu, " ** Select File **",
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> err:
> EFI_CALL(f->close(f));
> eficonfig_destroy(efi_menu);
> @@ -980,7 +1001,11 @@ efi_status_t eficonfig_process_show_file_option(void *data)
> if (!efi_menu)
> return EFI_OUT_OF_RESOURCES;
>
> - ret = eficonfig_process_common(efi_menu, " ** Update File **");
> + ret = eficonfig_process_common(efi_menu, " ** Update File **",
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> if (ret != EFI_SUCCESS) /* User selects "Clear" or "Quit" */
> ret = EFI_NOT_READY;
>
> @@ -1326,7 +1351,12 @@ static efi_status_t eficonfig_show_boot_option(struct eficonfig_boot_option *bo,
> if (ret != EFI_SUCCESS)
> goto out;
>
> - ret = eficonfig_process_common(efi_menu, header_str);
> + ret = eficonfig_process_common(efi_menu, header_str,
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> +
> out:
> eficonfig_destroy(efi_menu);
>
> @@ -1745,7 +1775,11 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected)
> if (ret != EFI_SUCCESS)
> goto out;
>
> - ret = eficonfig_process_common(efi_menu, " ** Select Boot Option **");
> + ret = eficonfig_process_common(efi_menu, " ** Select Boot Option **",
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> out:
> list_for_each_safe(pos, n, &efi_menu->list) {
> entry = list_entry(pos, struct eficonfig_entry, list);
> @@ -2567,7 +2601,12 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
> if (!efi_menu)
> return CMD_RET_FAILURE;
>
> - ret = eficonfig_process_common(efi_menu, " ** UEFI Maintenance Menu **");
> + ret = eficonfig_process_common(efi_menu,
> + " ** UEFI Maintenance Menu **",
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> eficonfig_destroy(efi_menu);
>
> if (ret == EFI_ABORTED)
> diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
> index ed39aab081..caca27495e 100644
> --- a/cmd/eficonfig_sbkey.c
> +++ b/cmd/eficonfig_sbkey.c
> @@ -410,7 +410,10 @@ static efi_status_t enumerate_and_show_signature_database(void *varname)
> goto out;
>
> snprintf(buf, sizeof(buf), " ** Show Signature Database (%ls) **", (u16 *)varname);
> - ret = eficonfig_process_common(efi_menu, buf);
> + ret = eficonfig_process_common(efi_menu, buf, eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> out:
> list_for_each_safe(pos, n, &efi_menu->list) {
> entry = list_entry(pos, struct eficonfig_entry, list);
> @@ -472,7 +475,11 @@ static efi_status_t eficonfig_process_set_secure_boot_key(void *data)
> efi_menu = eficonfig_create_fixed_menu(key_config_menu_items,
> ARRAY_SIZE(key_config_menu_items));
>
> - ret = eficonfig_process_common(efi_menu, header_str);
> + ret = eficonfig_process_common(efi_menu, header_str,
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> eficonfig_destroy(efi_menu);
>
> if (ret == EFI_ABORTED)
> @@ -518,7 +525,12 @@ efi_status_t eficonfig_process_secure_boot_config(void *data)
> break;
> }
>
> - ret = eficonfig_process_common(efi_menu, header_str);
> + ret = eficonfig_process_common(efi_menu, header_str,
> + eficonfig_menu_desc,
> + eficonfig_display_statusline,
> + eficonfig_print_entry,
> + eficonfig_choice_entry);
> +
> eficonfig_destroy(efi_menu);
>
> if (ret == EFI_ABORTED)
> diff --git a/include/efi_config.h b/include/efi_config.h
> index fd69926343..cec5715f84 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -9,12 +9,14 @@
> #define _EFI_CONFIG_H
>
> #include <efi_loader.h>
> +#include <menu.h>
>
> #define EFICONFIG_ENTRY_NUM_MAX 99
> #define EFICONFIG_VOLUME_PATH_MAX 512
> #define EFICONFIG_FILE_PATH_MAX 512
> #define EFICONFIG_FILE_PATH_BUF_SIZE (EFICONFIG_FILE_PATH_MAX * sizeof(u16))
>
> +extern const char *eficonfig_menu_desc;
> typedef efi_status_t (*eficonfig_entry_func)(void *data);
>
> /**
> @@ -45,6 +47,7 @@ struct eficonfig_entry {
> * @active: active menu entry index
> * @count: total count of menu entry
> * @menu_header: menu header string
> + * @menu_desc: menu description string
> * @list: menu entry list structure
> */
> struct efimenu {
> @@ -52,6 +55,7 @@ struct efimenu {
> int active;
> int count;
> char *menu_header;
> + const char *menu_desc;
> struct list_head list;
> };
>
> @@ -86,9 +90,16 @@ struct eficonfig_select_file_info {
> };
>
> void eficonfig_print_msg(char *msg);
> +void eficonfig_print_entry(void *data);
> +void eficonfig_display_statusline(struct menu *m);
> +char *eficonfig_choice_entry(void *data);
> void eficonfig_destroy(struct efimenu *efi_menu);
> efi_status_t eficonfig_process_quit(void *data);
> -efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_header);
> +efi_status_t eficonfig_process_common(struct efimenu *efi_menu,
> + char *menu_header, const char *menu_desc,
> + void (*display_statusline)(struct menu *),
> + void (*item_data_print)(void *),
> + char *(*item_choice)(void *));
> efi_status_t eficonfig_process_select_file(void *data);
> efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
> efi_uintn_t buf_size, u32 *index);
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
next prev parent reply other threads:[~2023-01-23 13:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 8:43 [PATCH v4 0/4] eficonfig: add vertical scroll support and refactoring Masahisa Kojima
2023-01-20 8:43 ` [PATCH v4 1/4] eficonfig: refactor eficonfig_process_common function Masahisa Kojima
2023-01-23 13:04 ` Ilias Apalodimas [this message]
2023-01-20 8:43 ` [PATCH v4 2/4] eficonfig: refactor change boot order implementation Masahisa Kojima
2023-01-23 14:57 ` Ilias Apalodimas
2023-01-20 8:43 ` [PATCH v4 3/4] eficonfig: add vertical scroll support Masahisa Kojima
2023-01-23 15:07 ` Ilias Apalodimas
2023-01-24 4:43 ` Masahisa Kojima
2023-01-20 8:43 ` [PATCH v4 4/4] eficonfig: increase the number of menu entries Masahisa Kojima
2023-01-23 15:08 ` Ilias Apalodimas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y86F1dHIZXg06sOy@hera \
--to=ilias.apalodimas@linaro.org \
--cc=masahisa.kojima@linaro.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.