Hi Kojima-san On Wed, Oct 26, 2022 at 07:43:43PM +0900, Masahisa Kojima wrote: > This commit refactors change boot order implementation > to use 'eficonfig_entry' structure.
Please add an explanation on why we are doing this, instead of what the patch is doing. I am assuming it cleans up some code and allows us to reuse eficonfig_entry since ->data is now pointing to an eficonfig_boot_order_data struct? > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > --- > No update since v5 > > Changes in v5: > - remove direct access mode > > newly created in v4 > > cmd/eficonfig.c | 129 +++++++++++++++++++++++++----------------------- > 1 file changed, 67 insertions(+), 62 deletions(-) > > list_del(&tmp->list); [...] > @@ -1891,11 +1884,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); > @@ -1921,9 +1914,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 ? > false : true; data->active = !!data->active seems a bit better here imho > return EFI_NOT_READY; > } > } > @@ -1949,12 +1944,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); > @@ -1962,31 +1958,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(struct eficonfig_boot_order_data)); sizeof(*data) > + if (!data) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > } > > - entry = calloc(1, sizeof(struct eficonfig_boot_order)); sizeof(*entry) > - 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); Thanks /Ilias