| Message ID | 20221109033728.5623-4-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | eficonfig: add UEFI Secure Boot key maintenance interface | expand |
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>
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;
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(-)