From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: kojima.masahisa@socionext.com, u-boot@lists.denx.de,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Simon Glass <sjg@chromium.org>,
Takahiro Akashi <takahiro.akashi@linaro.org>,
Etienne Carriere <etienne.carriere@linaro.org>
Subject: Re: [PATCH v7 3/5] eficonfig: refactor change boot order implementation
Date: Wed, 9 Nov 2022 16:50:48 +0200 [thread overview]
Message-ID: <Y2u+SAx1Lc0/ZXw/@hera> (raw)
In-Reply-To: <20221109033728.5623-4-masahisa.kojima@linaro.org>
On Wed, Nov 09, 2022 at 12:37:26PM +0900, Masahisa Kojima wrote:
> All the eficonfig menus other than "Change Boot Order"
> use 'eficonfig_entry' structure for each menu entry.
> This commit refactors change boot order implementation
> to use 'eficonfig_entry' structure same as other menus
> to have consistent menu handling.
>
> This commit also simplifies the data->active handling when
> KEY_SPACE is pressed, and sizeof() parameter.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v7:
> - simplify the data->active handling
> - update sizeof() parameter
> - update commit message
>
> Changes in v5:
> - remove direct access mode
>
> newly created in v4
>
> cmd/eficonfig.c | 129 +++++++++++++++++++++++++-----------------------
> 1 file changed, 67 insertions(+), 62 deletions(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index b392de7954..12babb76c2 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -93,20 +93,14 @@ struct eficonfig_boot_selection_data {
> };
>
> /**
> - * struct eficonfig_boot_order - structure to be used to update BootOrder variable
> + * struct eficonfig_boot_order_data - structure to be used to update BootOrder variable
> *
> - * @num: index in the menu entry
> - * @description: pointer to the description string
> * @boot_index: boot option index
> * @active: flag to include the boot option into BootOrder variable
> - * @list: list structure
> */
> -struct eficonfig_boot_order {
> - u32 num;
> - u16 *description;
> +struct eficonfig_boot_order_data {
> u32 boot_index;
> bool active;
> - struct list_head list;
> };
>
> /**
> @@ -1802,7 +1796,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
> {
> bool reverse;
> struct list_head *pos, *n;
> - struct eficonfig_boot_order *entry;
> + struct eficonfig_entry *entry;
>
> printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION
> "\n ** Change Boot Order **\n"
> @@ -1818,7 +1812,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
>
> /* draw boot option list */
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> + entry = list_entry(pos, struct eficonfig_entry, list);
> reverse = (entry->num == efi_menu->active);
>
> printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
> @@ -1827,13 +1821,13 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
> puts(ANSI_COLOR_REVERSE);
>
> if (entry->num < efi_menu->count - 2) {
> - if (entry->active)
> + if (((struct eficonfig_boot_order_data *)entry->data)->active)
> printf("[*] ");
> else
> printf("[ ] ");
> }
>
> - printf("%ls", entry->description);
> + printf("%s", entry->title);
>
> if (reverse)
> puts(ANSI_COLOR_RESET);
> @@ -1850,9 +1844,8 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> {
> int esc = 0;
> struct list_head *pos, *n;
> - struct eficonfig_boot_order *tmp;
> enum bootmenu_key key = KEY_NONE;
> - struct eficonfig_boot_order *entry;
> + struct eficonfig_entry *entry, *tmp;
>
> while (1) {
> bootmenu_loop(NULL, &key, &esc);
> @@ -1861,11 +1854,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> case KEY_PLUS:
> if (efi_menu->active > 0) {
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> + entry = list_entry(pos, struct eficonfig_entry, list);
> if (entry->num == efi_menu->active)
> break;
> }
> - tmp = list_entry(pos->prev, struct eficonfig_boot_order, list);
> + tmp = list_entry(pos->prev, struct eficonfig_entry, list);
> entry->num--;
> tmp->num++;
> list_del(&tmp->list);
> @@ -1879,11 +1872,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> case KEY_MINUS:
> if (efi_menu->active < efi_menu->count - 3) {
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> + entry = list_entry(pos, struct eficonfig_entry, list);
> if (entry->num == efi_menu->active)
> break;
> }
> - tmp = list_entry(pos->next, struct eficonfig_boot_order, list);
> + tmp = list_entry(pos->next, struct eficonfig_entry, list);
> entry->num++;
> tmp->num--;
> list_del(&entry->list);
> @@ -1909,9 +1902,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> case KEY_SPACE:
> if (efi_menu->active < efi_menu->count - 2) {
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> + entry = list_entry(pos, struct eficonfig_entry, list);
> if (entry->num == efi_menu->active) {
> - entry->active = entry->active ? false : true;
> + struct eficonfig_boot_order_data *data = entry->data;
> +
> + data->active = !data->active;
> return EFI_NOT_READY;
> }
> }
> @@ -1937,12 +1932,13 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_menu,
> u32 boot_index, bool active)
> {
> + char *title, *p;
> efi_status_t ret;
> efi_uintn_t size;
> void *load_option;
> struct efi_load_option lo;
> u16 varname[] = u"Boot####";
> - struct eficonfig_boot_order *entry;
> + struct eficonfig_boot_order_data *data;
>
> efi_create_indexed_name(varname, sizeof(varname), "Boot", boot_index);
> load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> @@ -1950,31 +1946,38 @@ static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_me
> return EFI_SUCCESS;
>
> ret = efi_deserialize_load_option(&lo, load_option, &size);
> - if (ret != EFI_SUCCESS) {
> - free(load_option);
> - return ret;
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + data = calloc(1, sizeof(*data));
> + if (!data) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> }
>
> - entry = calloc(1, sizeof(struct eficonfig_boot_order));
> - if (!entry) {
> - free(load_option);
> - return EFI_OUT_OF_RESOURCES;
> + title = calloc(1, utf16_utf8_strlen(lo.label) + 1);
> + if (!title) {
> + free(data);
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> }
> + p = title;
> + utf16_utf8_strcpy(&p, lo.label);
>
> - entry->description = u16_strdup(lo.label);
> - if (!entry->description) {
> - free(load_option);
> - free(entry);
> - return EFI_OUT_OF_RESOURCES;
> + data->boot_index = boot_index;
> + data->active = active;
> +
> + ret = eficonfig_append_menu_entry(efi_menu, title, NULL, data);
> + if (ret != EFI_SUCCESS) {
> + free(data);
> + free(title);
> + goto out;
> }
> - entry->num = efi_menu->count++;
> - entry->boot_index = boot_index;
> - entry->active = active;
> - list_add_tail(&entry->list, &efi_menu->list);
>
> +out:
> free(load_option);
>
> - return EFI_SUCCESS;
> + return ret;
> }
>
> /**
> @@ -1989,8 +1992,8 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
> u16 *bootorder, efi_uintn_t num)
> {
> u32 i;
> + char *title;
> efi_status_t ret;
> - struct eficonfig_boot_order *entry;
>
> /* list the load option in the order of BootOrder variable */
> for (i = 0; i < num; i++) {
> @@ -2017,27 +2020,25 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
> }
>
> /* add "Save" and "Quit" entries */
> - entry = calloc(1, sizeof(struct eficonfig_boot_order));
> - if (!entry)
> + title = strdup("Save");
> + if (!title) {
> + ret = EFI_OUT_OF_RESOURCES;
> goto out;
> + }
>
> - entry->num = efi_menu->count++;
> - entry->description = u16_strdup(u"Save");
> - list_add_tail(&entry->list, &efi_menu->list);
> -
> - entry = calloc(1, sizeof(struct eficonfig_boot_order));
> - if (!entry)
> + ret = eficonfig_append_menu_entry(efi_menu, title, NULL, NULL);
> + if (ret != EFI_SUCCESS)
> goto out;
>
> - entry->num = efi_menu->count++;
> - entry->description = u16_strdup(u"Quit");
> - list_add_tail(&entry->list, &efi_menu->list);
> + ret = eficonfig_append_quit_entry(efi_menu);
> + if (ret != EFI_SUCCESS)
> + goto out;
>
> efi_menu->active = 0;
>
> return EFI_SUCCESS;
> out:
> - return EFI_OUT_OF_RESOURCES;
> + return ret;
> }
>
> /**
> @@ -2053,7 +2054,7 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
> efi_status_t ret;
> efi_uintn_t num, size;
> struct list_head *pos, *n;
> - struct eficonfig_boot_order *entry;
> + struct eficonfig_entry *entry;
> struct efimenu *efi_menu;
>
> efi_menu = calloc(1, sizeof(struct efimenu));
> @@ -2084,9 +2085,16 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
> /* create new BootOrder */
> count = 0;
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> - if (entry->active)
> - new_bootorder[count++] = entry->boot_index;
> + struct eficonfig_boot_order_data *data;
> +
> + entry = list_entry(pos, struct eficonfig_entry, list);
> + /* exit the loop when iteration reaches "Save" */
> + if (!strncmp(entry->title, "Save", strlen("Save")))
> + break;
> +
> + data = entry->data;
> + if (data->active)
> + new_bootorder[count++] = data->boot_index;
> }
>
> size = count * sizeof(u16);
> @@ -2105,15 +2113,12 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
> }
> }
> out:
> + free(bootorder);
> list_for_each_safe(pos, n, &efi_menu->list) {
> - entry = list_entry(pos, struct eficonfig_boot_order, list);
> - list_del(&entry->list);
> - free(entry->description);
> - free(entry);
> + entry = list_entry(pos, struct eficonfig_entry, list);
> + free(entry->data);
> }
> -
> - free(bootorder);
> - free(efi_menu);
> + eficonfig_destroy(efi_menu);
>
> /* to stay the parent menu */
> ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
> --
> 2.17.1
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
next prev parent reply other threads:[~2022-11-09 14:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 3:37 [PATCH v7 0/5] eficonfig: add UEFI Secure Boot key maintenance interface Masahisa Kojima
2022-11-09 3:37 ` [PATCH v7 1/5] eficonfig: refactor file selection handling Masahisa Kojima
2022-11-09 14:50 ` Ilias Apalodimas
2022-11-09 3:37 ` [PATCH v7 2/5] eficonfig: expose append entry function Masahisa Kojima
2022-11-09 3:37 ` [PATCH v7 3/5] eficonfig: refactor change boot order implementation Masahisa Kojima
2022-11-09 14:50 ` Ilias Apalodimas [this message]
2022-11-09 3:37 ` [PATCH v7 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface Masahisa Kojima
2022-11-09 16:04 ` Ilias Apalodimas
2022-11-10 2:42 ` Masahisa Kojima
2022-11-10 6:46 ` Ilias Apalodimas
2022-11-10 7:13 ` Masahisa Kojima
2022-11-09 3:37 ` [PATCH v7 5/5] eficonfig: add "Show Signature Database" menu entry Masahisa Kojima
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=Y2u+SAx1Lc0/ZXw/@hera \
--to=ilias.apalodimas@linaro.org \
--cc=etienne.carriere@linaro.org \
--cc=kojima.masahisa@socionext.com \
--cc=masahisa.kojima@linaro.org \
--cc=sjg@chromium.org \
--cc=takahiro.akashi@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.