Re: [PATCH v7 3/5] eficonfig: refactor change boot order implementation

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