[PATCH v3 2/3] eficonfig: refactor change boot order implementation

Masahisa Kojima masahisa.kojima at linaro.org
Thu Jan 5 03:58:53 CET 2023


This commit removes the change boot order specific
menu implementation. The change boot order implementation
calls eficonfig_process_common() same as other menus.

The change boot order menu requires own item_data_print
and item_choice implementation, but display_statusline
function can be a same function as other menus.

Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
---
Changes in v3:
- modify "reverse" local variable type to bool

Changes in v2:
- add comment when the user key press is not valid
- add const qualifier to eficonfig_change_boot_order_desc

 cmd/eficonfig.c | 245 +++++++++++++++++++++++++++++-------------------
 1 file changed, 150 insertions(+), 95 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 274526fe20..6d5ebc4055 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -24,6 +24,11 @@ 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";
 
+static const char *eficonfig_change_boot_order_desc =
+	"  Press UP/DOWN to move, +/- to change orde\n"
+	"  Press SPACE to activate or deactivate the entry\n"
+	"  Select [Save] to complete, ESC/CTRL+C to quit";
+
 #define EFICONFIG_DESCRIPTION_MAX 32
 #define EFICONFIG_OPTIONAL_DATA_MAX 64
 
@@ -105,6 +110,17 @@ struct eficonfig_boot_order_data {
 	bool active;
 };
 
+/**
+ * struct eficonfig_save_boot_order_data - structure to be used to change boot order
+ *
+ * @efi_menu:		pointer to efimenu structure
+ * @selected:		flag to indicate user selects "Save" entry
+ */
+struct eficonfig_save_boot_order_data {
+	struct efimenu *efi_menu;
+	bool selected;
+};
+
 /**
  * eficonfig_print_msg() - print message
  *
@@ -173,10 +189,9 @@ void eficonfig_display_statusline(struct menu *m)
 	      "\n%s\n"
 	       ANSI_CURSOR_POSITION ANSI_CLEAR_LINE ANSI_CURSOR_POSITION
 	       "%s"
-	       ANSI_CLEAR_LINE_TO_END ANSI_CURSOR_POSITION ANSI_CLEAR_LINE,
+	       ANSI_CLEAR_LINE_TO_END,
 	       1, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1,
-	       entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc,
-	       entry->efi_menu->count + 7, 1);
+	       entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc);
 }
 
 /**
@@ -1841,63 +1856,44 @@ out:
 }
 
 /**
- * eficonfig_display_change_boot_order() - display the BootOrder list
+ * eficonfig_print_change_boot_order_entry() - print the boot option entry
  *
- * @efi_menu:	pointer to the efimenu structure
- * Return:	status code
+ * @data:	pointer to the data associated with each menu entry
  */
-static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
+static void eficonfig_print_change_boot_order_entry(void *data)
 {
-	bool reverse;
-	struct list_head *pos, *n;
-	struct eficonfig_entry *entry;
-
-	printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION
-	       "\n  ** Change Boot Order **\n"
-	       ANSI_CURSOR_POSITION
-	       "  Press UP/DOWN to move, +/- to change order"
-	       ANSI_CURSOR_POSITION
-	       "  Press SPACE to activate or deactivate the entry"
-	       ANSI_CURSOR_POSITION
-	       "  Select [Save] to complete, ESC/CTRL+C to quit"
-	       ANSI_CURSOR_POSITION ANSI_CLEAR_LINE,
-	       1, 1, efi_menu->count + 5, 1, efi_menu->count + 6, 1,
-	       efi_menu->count + 7, 1,  efi_menu->count + 8, 1);
-
-	/* draw boot option list */
-	list_for_each_safe(pos, n, &efi_menu->list) {
-		entry = list_entry(pos, struct eficonfig_entry, list);
-		reverse = (entry->num == efi_menu->active);
+	struct eficonfig_entry *entry = data;
+	bool reverse = (entry->efi_menu->active == entry->num);
 
-		printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
+	printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
 
-		if (reverse)
-			puts(ANSI_COLOR_REVERSE);
+	if (reverse)
+		puts(ANSI_COLOR_REVERSE);
 
-		if (entry->num < efi_menu->count - 2) {
-			if (((struct eficonfig_boot_order_data *)entry->data)->active)
-				printf("[*]  ");
-			else
-				printf("[ ]  ");
-		}
+	if (entry->num < entry->efi_menu->count - 2) {
+		if (((struct eficonfig_boot_order_data *)entry->data)->active)
+			printf("[*]  ");
+		else
+			printf("[ ]  ");
+	}
 
-		printf("%s", entry->title);
+	printf("%s", entry->title);
 
-		if (reverse)
-			puts(ANSI_COLOR_RESET);
-	}
+	if (reverse)
+		puts(ANSI_COLOR_RESET);
 }
 
 /**
- * eficonfig_choice_change_boot_order() - handle the BootOrder update
+ * eficonfig_choice_change_boot_order() - user key input handler
  *
- * @efi_menu:	pointer to the efimenu structure
- * Return:	status code
+ * @data:	pointer to the menu entry
+ * Return:	key string to identify the selected entry
  */
-static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
+char *eficonfig_choice_change_boot_order(void *data)
 {
 	int esc = 0;
 	struct list_head *pos, *n;
+	struct efimenu *efi_menu = data;
 	enum bootmenu_key key = KEY_NONE;
 	struct eficonfig_entry *entry, *tmp;
 
@@ -1922,7 +1918,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 		case KEY_UP:
 			if (efi_menu->active > 0)
 				--efi_menu->active;
-			return EFI_NOT_READY;
+			return NULL;
 		case KEY_MINUS:
 			if (efi_menu->active < efi_menu->count - 3) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
@@ -1938,20 +1934,29 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 
 				++efi_menu->active;
 			}
-			return EFI_NOT_READY;
+			return NULL;
 		case KEY_DOWN:
 			if (efi_menu->active < efi_menu->count - 1)
 				++efi_menu->active;
-			return EFI_NOT_READY;
+			return NULL;
 		case KEY_SELECT:
 			/* "Save" */
-			if (efi_menu->active == efi_menu->count - 2)
-				return EFI_SUCCESS;
-
+			if (efi_menu->active == efi_menu->count - 2) {
+				list_for_each_prev_safe(pos, n, &efi_menu->list) {
+					entry = list_entry(pos, struct eficonfig_entry, list);
+					if (entry->num == efi_menu->active)
+						break;
+				}
+				return entry->key;
+			}
 			/* "Quit" */
-			if (efi_menu->active == efi_menu->count - 1)
-				return EFI_ABORTED;
-
+			if (efi_menu->active == efi_menu->count - 1) {
+				entry = list_last_entry(&efi_menu->list,
+							struct eficonfig_entry,
+							list);
+				return entry->key;
+			}
+			/* Pressed key is not valid, wait next key press */
 			break;
 		case KEY_SPACE:
 			if (efi_menu->active < efi_menu->count - 2) {
@@ -1961,15 +1966,18 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 						struct eficonfig_boot_order_data *data = entry->data;
 
 						data->active = !data->active;
-						return EFI_NOT_READY;
+						return NULL;
 					}
 				}
 			}
+			/* Pressed key is not valid, wait next key press */
 			break;
 		case KEY_QUIT:
-			return EFI_ABORTED;
+			entry = list_last_entry(&efi_menu->list,
+						struct eficonfig_entry, list);
+			return entry->key;
 		default:
-			/* Pressed key is not valid, no need to regenerate the menu */
+			/* Pressed key is not valid, wait next key press */
 			break;
 		}
 	}
@@ -2034,6 +2042,66 @@ out:
 	return ret;
 }
 
+/**
+ * eficonfig_process_save_boot_order() - callback function for "Save" entry
+ *
+ * @data:	pointer to the data
+ * Return:	status code
+ */
+static efi_status_t eficonfig_process_save_boot_order(void *data)
+{
+	u32 count = 0;
+	efi_status_t ret;
+	efi_uintn_t size;
+	struct list_head *pos, *n;
+	u16 *new_bootorder;
+	struct efimenu *efi_menu;
+	struct eficonfig_entry *entry;
+	struct eficonfig_save_boot_order_data *save_data = data;
+
+	efi_menu = save_data->efi_menu;
+
+	/*
+	 * The change boot order menu always has "Save" and "Quit" entries.
+	 * !(efi_menu->count - 2) means there is no user defined boot option.
+	 */
+	if (!(efi_menu->count - 2))
+		return EFI_SUCCESS;
+
+	new_bootorder = calloc(1, (efi_menu->count - 2) * sizeof(u16));
+	if (!new_bootorder) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+
+	/* create new BootOrder */
+	count = 0;
+	list_for_each_safe(pos, n, &efi_menu->list) {
+		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);
+	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
+				   EFI_VARIABLE_NON_VOLATILE |
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS,
+				   size, new_bootorder, false);
+
+	save_data->selected = true;
+out:
+	free(new_bootorder);
+
+	return ret;
+}
 /**
  * eficonfig_create_change_boot_order_entry() - create boot order entry
  *
@@ -2050,6 +2118,7 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
 	efi_status_t ret;
 	u16 *var_name16 = NULL;
 	efi_uintn_t size, buf_size;
+	struct eficonfig_save_boot_order_data *save_data;
 
 	/* list the load option in the order of BootOrder variable */
 	for (i = 0; i < num; i++) {
@@ -2100,7 +2169,17 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
 		goto out;
 	}
 
-	ret = eficonfig_append_menu_entry(efi_menu, title, NULL, NULL);
+	save_data = malloc(sizeof(struct eficonfig_save_boot_order_data));
+	if (!save_data) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+	save_data->efi_menu = efi_menu;
+	save_data->selected = false;
+
+	ret = eficonfig_append_menu_entry(efi_menu, title,
+					  eficonfig_process_save_boot_order,
+					  save_data);
 	if (ret != EFI_SUCCESS)
 		goto out;
 
@@ -2123,7 +2202,6 @@ out:
  */
 static efi_status_t eficonfig_process_change_boot_order(void *data)
 {
-	u32 count;
 	u16 *bootorder;
 	efi_status_t ret;
 	efi_uintn_t num, size;
@@ -2144,47 +2222,24 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
 		goto out;
 
 	while (1) {
-		eficonfig_display_change_boot_order(efi_menu);
-
-		ret = eficonfig_choice_change_boot_order(efi_menu);
-		if (ret == EFI_SUCCESS) {
-			u16 *new_bootorder;
-
-			new_bootorder = calloc(1, (efi_menu->count - 2) * sizeof(u16));
-			if (!new_bootorder) {
-				ret = EFI_OUT_OF_RESOURCES;
-				goto out;
-			}
-
-			/* create new BootOrder  */
-			count = 0;
-			list_for_each_safe(pos, n, &efi_menu->list) {
-				struct eficonfig_boot_order_data *data;
-
+		ret = eficonfig_process_common(efi_menu,
+					       "  ** Change Boot Order **",
+					       eficonfig_change_boot_order_desc,
+					       eficonfig_display_statusline,
+					       eficonfig_print_change_boot_order_entry,
+					       eficonfig_choice_change_boot_order);
+		/* exit from the menu if user selects the "Save" entry. */
+		if (ret == EFI_SUCCESS && efi_menu->active == (efi_menu->count - 2)) {
+			list_for_each_prev_safe(pos, n, &efi_menu->list) {
 				entry = list_entry(pos, struct eficonfig_entry, list);
-				/* exit the loop when iteration reaches "Save" */
-				if (!strncmp(entry->title, "Save", strlen("Save")))
+				if (entry->num == efi_menu->active)
 					break;
-
-				data = entry->data;
-				if (data->active)
-					new_bootorder[count++] = data->boot_index;
 			}
-
-			size = count * sizeof(u16);
-			ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
-						   EFI_VARIABLE_NON_VOLATILE |
-						   EFI_VARIABLE_BOOTSERVICE_ACCESS |
-						   EFI_VARIABLE_RUNTIME_ACCESS,
-						   size, new_bootorder, false);
-
-			free(new_bootorder);
-			goto out;
-		} else if (ret == EFI_NOT_READY) {
-			continue;
-		} else {
-			goto out;
+			if (((struct eficonfig_save_boot_order_data *)entry->data)->selected)
+				break;
 		}
+		if (ret != EFI_SUCCESS)
+			break;
 	}
 out:
 	free(bootorder);
-- 
2.17.1



More information about the U-Boot mailing list