diff mbox series

[v4,3/7] eficonfig: add direct menu entry access mode

Message ID 20221024044804.3351-4-masahisa.kojima@linaro.org
State New
Headers show
Series eficonfig: add UEFI Secure Boot key maintenance interface | expand

Commit Message

Masahisa Kojima Oct. 24, 2022, 4:48 a.m. UTC
This commit adds the direct menu entry access mode.
User can select the menu entry by '&' key followed by
the menu title name.

User input is read in UTF-16, then UTF-16 string is converted
into UTF-8 internally because string comparison relies on strncasecmp().
There is no equivalent string comparison function for UTF-16.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly added in v4

 cmd/eficonfig.c      | 120 ++++++++++++++++++++++++++++++++++++++++++-
 common/menu.c        |   3 ++
 include/efi_config.h |   3 ++
 include/menu.h       |   1 +
 4 files changed, 126 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt Oct. 24, 2022, 5:40 a.m. UTC | #1
On 10/24/22 06:48, Masahisa Kojima wrote:
> This commit adds the direct menu entry access mode.
> User can select the menu entry by '&' key followed by
> the menu title name.
>
> User input is read in UTF-16, then UTF-16 string is converted
> into UTF-8 internally because string comparison relies on strncasecmp().
> There is no equivalent string comparison function for UTF-16.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly added in v4
>
>   cmd/eficonfig.c      | 120 ++++++++++++++++++++++++++++++++++++++++++-
>   common/menu.c        |   3 ++
>   include/efi_config.h |   3 ++
>   include/menu.h       |   1 +
>   4 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 0cb0770ac3..56d9268f9f 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -22,6 +22,7 @@
>
>   static struct efi_simple_text_input_protocol *cin;
>
> +#define EFICONFIG_ACCESSOR_STR_MAX 16
>   #define EFICONFIG_DESCRIPTION_MAX 32
>   #define EFICONFIG_OPTIONAL_DATA_MAX 64
>
> @@ -155,7 +156,28 @@ static void eficonfig_print_entry(void *data)
>   	if (reverse)
>   		puts(ANSI_COLOR_REVERSE);
>
> -	printf("%s", entry->title);
> +	if (reverse && entry->efi_menu->direct_access_mode) {
> +		size_t len = u16_strlen(entry->efi_menu->accessor_str);
> +		char *accessor_str, *p;
> +
> +		accessor_str = calloc(1, utf16_utf8_strlen(entry->efi_menu->accessor_str) + 1);
> +		if (!accessor_str) {
> +			printf("%s", entry->title);
> +			return;
> +		}
> +		p = accessor_str;
> +		utf16_utf8_strncpy(&p, entry->efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> +		len = strlen(accessor_str);
> +		if (!strncasecmp(accessor_str, entry->title, len)) {
> +			printf("%.*s" ANSI_COLOR_RESET "%s", (int)len, entry->title,
> +			       &entry->title[len]);
> +		} else {
> +			printf("%s", entry->title);
> +		}
> +		free(accessor_str);
> +	} else {
> +		printf("%s", entry->title);
> +	}
>
>   	if (reverse)
>   		puts(ANSI_COLOR_RESET);
> @@ -182,6 +204,83 @@ static void eficonfig_display_statusline(struct menu *m)
>   	       entry->efi_menu->count + 6, 1, entry->efi_menu->count + 7, 1);
>   }
>
> +/**
> + * eficonfig_handle_direct_accessor() - handle direct access user input
> + *
> + * @efi_menu:	pointer to the efimenu structure
> + * Return:	key string to identify the selected entry
> + */
> +static char *eficonfig_handle_direct_accessor(struct efimenu *efi_menu)
> +{
> +	efi_status_t ret;
> +	char *accessor_str, *p;
> +	struct efi_input_key key;
> +	struct list_head *pos, *n;
> +	struct eficonfig_entry *entry;
> +	static int len;
> +
> +	/* Read user input */
> +	do {
> +		ret = EFI_CALL(cin->read_key_stroke(cin, &key));
> +		mdelay(10);
> +	} while (ret == EFI_NOT_READY);
> +
> +	/* If user presses Ctrl+C or ESC, exit direct access mode */
> +	if (key.unicode_char == 0x3 || key.scan_code == 23)
> +		goto out;
> +
> +	/* If user presses ENTER, exit direct access mode and return the active entry */
> +	if (key.unicode_char == u'\r') {
> +		list_for_each_safe(pos, n, &efi_menu->list) {
> +			entry = list_entry(pos, struct eficonfig_entry, list);
> +			if (entry->num == efi_menu->active) {
> +				efi_menu->direct_access_mode = false;
> +				memset(efi_menu->accessor_str, 0,
> +				       EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> +				return entry->key;
> +			}
> +		}
> +
> +		/* no matching entry */
> +		goto out;
> +	}
> +
> +	/* Ignore other control code and efi scan code */
> +	if (key.unicode_char < 0x20 || key.scan_code != 0)
> +		return NULL;
> +
> +	len = u16_strlen(efi_menu->accessor_str);
> +	if (len < EFICONFIG_ACCESSOR_STR_MAX - 1)
> +		efi_menu->accessor_str[len] = key.unicode_char;
> +
> +	accessor_str = calloc(1, utf16_utf8_strlen(efi_menu->accessor_str) + 1);
> +	if (!accessor_str)
> +		return NULL;
> +
> +	p = accessor_str;
> +	utf16_utf8_strncpy(&p, efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> +
> +	list_for_each_safe(pos, n, &efi_menu->list) {
> +		entry = list_entry(pos, struct eficonfig_entry, list);
> +		if (!strncasecmp(accessor_str, entry->title, strlen(accessor_str))) {
> +			efi_menu->active = entry->num;
> +			free(accessor_str);
> +			return NULL;
> +		}
> +	}
> +
> +	/* does not match any entries */
> +	free(accessor_str);
> +	efi_menu->active = 0;
> +	return NULL;
> +
> +out:
> +	efi_menu->direct_access_mode = false;
> +	memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> +	efi_menu->active = 0;
> +	return NULL;
> +}
> +
>   /**
>    * eficonfig_choice_entry() - user key input handler
>    *
> @@ -196,6 +295,9 @@ static char *eficonfig_choice_entry(void *data)
>   	enum bootmenu_key key = KEY_NONE;
>   	struct efimenu *efi_menu = data;
>
> +	if (efi_menu->direct_access_mode)
> +		return eficonfig_handle_direct_accessor(efi_menu);
> +
>   	while (1) {
>   		bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
>
> @@ -221,6 +323,10 @@ static char *eficonfig_choice_entry(void *data)
>   			/* Quit by choosing the last entry */
>   			entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
>   			return entry->key;
> +		case KEY_AMPERSAND:
> +			memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> +			efi_menu->direct_access_mode = true;
> +			return NULL;
>   		default:
>   			/* Pressed key is not valid, no need to regenerate the menu */
>   			break;
> @@ -248,6 +354,7 @@ void eficonfig_destroy(struct efimenu *efi_menu)
>   		free(entry);
>   	}
>   	free(efi_menu->menu_header);
> +	free(efi_menu->accessor_str);
>   	free(efi_menu);
>   }
>
> @@ -385,6 +492,9 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_heade
>   		if (!efi_menu->menu_header)
>   			return EFI_OUT_OF_RESOURCES;
>   	}
> +	efi_menu->accessor_str = calloc(1, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> +	if (!efi_menu->accessor_str)
> +		return EFI_OUT_OF_RESOURCES;
>
>   	menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,
>   			   eficonfig_print_entry, eficonfig_choice_entry,
> @@ -1866,6 +1976,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   	enum bootmenu_key key = KEY_NONE;
>   	struct eficonfig_boot_order *entry;
>
> +	if (efi_menu->direct_access_mode) {
> +		eficonfig_handle_direct_accessor(efi_menu);
> +		return EFI_NOT_READY;
> +	}
> +
>   	while (1) {
>   		bootmenu_loop(NULL, &key, &esc);
>
> @@ -1931,6 +2046,9 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   			break;
>   		case KEY_QUIT:
>   			return EFI_ABORTED;
> +		case KEY_AMPERSAND:
> +			efi_menu->direct_access_mode = true;
> +			return EFI_NOT_READY;
>   		default:
>   			/* Pressed key is not valid, no need to regenerate the menu */
>   			break;
> diff --git a/common/menu.c b/common/menu.c
> index 8fe00965c0..6ea9f5c9b8 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -557,4 +557,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
>
>   	if (c == ' ')
>   		*key = KEY_SPACE;
> +
> +	if (c == '&')
> +		*key = KEY_AMPERSAND;

I am not really happy with how U-Boot menus work.

I think there should be one function to which you pass the menu entries
and you get back the index of the chosen entry (or some error code if
ESC for pressed).

My idea about "ampersand" was: You pass a list of strings to the menu
function like:

&Open
&Close
E&xit

The displayed menu would highlight the access key in a different color,
e.g. white instead of grey.

*O*pen
*C*lose
E*x*it

The user can navigate with either UP, Down and press Enter then you will
get back the chosen entry. Or the user presses 'o', 'c', or 'x' and you
will get back the index of the respective menu entry.

The user would never use the '&' key.

Best regards

Heinrich

>   }
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 86bc801211..1b84e2d579 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -45,6 +45,7 @@ struct eficonfig_entry {
>    * @active:		active menu entry index
>    * @count:		total count of menu entry
>    * @menu_header:	menu header string
> + * @accessor_str:	pointer to the accessor string for entry shortcut
>    * @list:		menu entry list structure
>    */
>   struct efimenu {
> @@ -52,6 +53,8 @@ struct efimenu {
>   	int active;
>   	int count;
>   	char *menu_header;
> +	bool direct_access_mode;
> +	u16 *accessor_str;
>   	struct list_head list;
>   };
>
> diff --git a/include/menu.h b/include/menu.h
> index 702aacb170..03bf8dc4f5 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -51,6 +51,7 @@ enum bootmenu_key {
>   	KEY_PLUS,
>   	KEY_MINUS,
>   	KEY_SPACE,
> +	KEY_AMPERSAND,
>   };
>
>   void bootmenu_autoboot_loop(struct bootmenu_data *menu,
Masahisa Kojima Oct. 24, 2022, 6:34 a.m. UTC | #2
Hi Heinrich,

On Mon, 24 Oct 2022 at 14:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/24/22 06:48, Masahisa Kojima wrote:
> > This commit adds the direct menu entry access mode.
> > User can select the menu entry by '&' key followed by
> > the menu title name.
> >
> > User input is read in UTF-16, then UTF-16 string is converted
> > into UTF-8 internally because string comparison relies on strncasecmp().
> > There is no equivalent string comparison function for UTF-16.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Newly added in v4
> >
> >   cmd/eficonfig.c      | 120 ++++++++++++++++++++++++++++++++++++++++++-
> >   common/menu.c        |   3 ++
> >   include/efi_config.h |   3 ++
> >   include/menu.h       |   1 +
> >   4 files changed, 126 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > index 0cb0770ac3..56d9268f9f 100644
> > --- a/cmd/eficonfig.c
> > +++ b/cmd/eficonfig.c
> > @@ -22,6 +22,7 @@
> >
> >   static struct efi_simple_text_input_protocol *cin;
> >
> > +#define EFICONFIG_ACCESSOR_STR_MAX 16
> >   #define EFICONFIG_DESCRIPTION_MAX 32
> >   #define EFICONFIG_OPTIONAL_DATA_MAX 64
> >
> > @@ -155,7 +156,28 @@ static void eficonfig_print_entry(void *data)
> >       if (reverse)
> >               puts(ANSI_COLOR_REVERSE);
> >
> > -     printf("%s", entry->title);
> > +     if (reverse && entry->efi_menu->direct_access_mode) {
> > +             size_t len = u16_strlen(entry->efi_menu->accessor_str);
> > +             char *accessor_str, *p;
> > +
> > +             accessor_str = calloc(1, utf16_utf8_strlen(entry->efi_menu->accessor_str) + 1);
> > +             if (!accessor_str) {
> > +                     printf("%s", entry->title);
> > +                     return;
> > +             }
> > +             p = accessor_str;
> > +             utf16_utf8_strncpy(&p, entry->efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> > +             len = strlen(accessor_str);
> > +             if (!strncasecmp(accessor_str, entry->title, len)) {
> > +                     printf("%.*s" ANSI_COLOR_RESET "%s", (int)len, entry->title,
> > +                            &entry->title[len]);
> > +             } else {
> > +                     printf("%s", entry->title);
> > +             }
> > +             free(accessor_str);
> > +     } else {
> > +             printf("%s", entry->title);
> > +     }
> >
> >       if (reverse)
> >               puts(ANSI_COLOR_RESET);
> > @@ -182,6 +204,83 @@ static void eficonfig_display_statusline(struct menu *m)
> >              entry->efi_menu->count + 6, 1, entry->efi_menu->count + 7, 1);
> >   }
> >
> > +/**
> > + * eficonfig_handle_direct_accessor() - handle direct access user input
> > + *
> > + * @efi_menu:        pointer to the efimenu structure
> > + * Return:   key string to identify the selected entry
> > + */
> > +static char *eficonfig_handle_direct_accessor(struct efimenu *efi_menu)
> > +{
> > +     efi_status_t ret;
> > +     char *accessor_str, *p;
> > +     struct efi_input_key key;
> > +     struct list_head *pos, *n;
> > +     struct eficonfig_entry *entry;
> > +     static int len;
> > +
> > +     /* Read user input */
> > +     do {
> > +             ret = EFI_CALL(cin->read_key_stroke(cin, &key));
> > +             mdelay(10);
> > +     } while (ret == EFI_NOT_READY);
> > +
> > +     /* If user presses Ctrl+C or ESC, exit direct access mode */
> > +     if (key.unicode_char == 0x3 || key.scan_code == 23)
> > +             goto out;
> > +
> > +     /* If user presses ENTER, exit direct access mode and return the active entry */
> > +     if (key.unicode_char == u'\r') {
> > +             list_for_each_safe(pos, n, &efi_menu->list) {
> > +                     entry = list_entry(pos, struct eficonfig_entry, list);
> > +                     if (entry->num == efi_menu->active) {
> > +                             efi_menu->direct_access_mode = false;
> > +                             memset(efi_menu->accessor_str, 0,
> > +                                    EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > +                             return entry->key;
> > +                     }
> > +             }
> > +
> > +             /* no matching entry */
> > +             goto out;
> > +     }
> > +
> > +     /* Ignore other control code and efi scan code */
> > +     if (key.unicode_char < 0x20 || key.scan_code != 0)
> > +             return NULL;
> > +
> > +     len = u16_strlen(efi_menu->accessor_str);
> > +     if (len < EFICONFIG_ACCESSOR_STR_MAX - 1)
> > +             efi_menu->accessor_str[len] = key.unicode_char;
> > +
> > +     accessor_str = calloc(1, utf16_utf8_strlen(efi_menu->accessor_str) + 1);
> > +     if (!accessor_str)
> > +             return NULL;
> > +
> > +     p = accessor_str;
> > +     utf16_utf8_strncpy(&p, efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> > +
> > +     list_for_each_safe(pos, n, &efi_menu->list) {
> > +             entry = list_entry(pos, struct eficonfig_entry, list);
> > +             if (!strncasecmp(accessor_str, entry->title, strlen(accessor_str))) {
> > +                     efi_menu->active = entry->num;
> > +                     free(accessor_str);
> > +                     return NULL;
> > +             }
> > +     }
> > +
> > +     /* does not match any entries */
> > +     free(accessor_str);
> > +     efi_menu->active = 0;
> > +     return NULL;
> > +
> > +out:
> > +     efi_menu->direct_access_mode = false;
> > +     memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > +     efi_menu->active = 0;
> > +     return NULL;
> > +}
> > +
> >   /**
> >    * eficonfig_choice_entry() - user key input handler
> >    *
> > @@ -196,6 +295,9 @@ static char *eficonfig_choice_entry(void *data)
> >       enum bootmenu_key key = KEY_NONE;
> >       struct efimenu *efi_menu = data;
> >
> > +     if (efi_menu->direct_access_mode)
> > +             return eficonfig_handle_direct_accessor(efi_menu);
> > +
> >       while (1) {
> >               bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
> >
> > @@ -221,6 +323,10 @@ static char *eficonfig_choice_entry(void *data)
> >                       /* Quit by choosing the last entry */
> >                       entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
> >                       return entry->key;
> > +             case KEY_AMPERSAND:
> > +                     memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > +                     efi_menu->direct_access_mode = true;
> > +                     return NULL;
> >               default:
> >                       /* Pressed key is not valid, no need to regenerate the menu */
> >                       break;
> > @@ -248,6 +354,7 @@ void eficonfig_destroy(struct efimenu *efi_menu)
> >               free(entry);
> >       }
> >       free(efi_menu->menu_header);
> > +     free(efi_menu->accessor_str);
> >       free(efi_menu);
> >   }
> >
> > @@ -385,6 +492,9 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_heade
> >               if (!efi_menu->menu_header)
> >                       return EFI_OUT_OF_RESOURCES;
> >       }
> > +     efi_menu->accessor_str = calloc(1, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > +     if (!efi_menu->accessor_str)
> > +             return EFI_OUT_OF_RESOURCES;
> >
> >       menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,
> >                          eficonfig_print_entry, eficonfig_choice_entry,
> > @@ -1866,6 +1976,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> >       enum bootmenu_key key = KEY_NONE;
> >       struct eficonfig_boot_order *entry;
> >
> > +     if (efi_menu->direct_access_mode) {
> > +             eficonfig_handle_direct_accessor(efi_menu);
> > +             return EFI_NOT_READY;
> > +     }
> > +
> >       while (1) {
> >               bootmenu_loop(NULL, &key, &esc);
> >
> > @@ -1931,6 +2046,9 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> >                       break;
> >               case KEY_QUIT:
> >                       return EFI_ABORTED;
> > +             case KEY_AMPERSAND:
> > +                     efi_menu->direct_access_mode = true;
> > +                     return EFI_NOT_READY;
> >               default:
> >                       /* Pressed key is not valid, no need to regenerate the menu */
> >                       break;
> > diff --git a/common/menu.c b/common/menu.c
> > index 8fe00965c0..6ea9f5c9b8 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -557,4 +557,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
> >
> >       if (c == ' ')
> >               *key = KEY_SPACE;
> > +
> > +     if (c == '&')
> > +             *key = KEY_AMPERSAND;
>
> I am not really happy with how U-Boot menus work.
>
> I think there should be one function to which you pass the menu entries
> and you get back the index of the chosen entry (or some error code if
> ESC for pressed).
>
> My idea about "ampersand" was: You pass a list of strings to the menu
> function like:
>
> &Open
> &Close
> E&xit
>
> The displayed menu would highlight the access key in a different color,
> e.g. white instead of grey.
>
> *O*pen
> *C*lose
> E*x*it
>
> The user can navigate with either UP, Down and press Enter then you will
> get back the chosen entry. Or the user presses 'o', 'c', or 'x' and you
> will get back the index of the respective menu entry.

Thank you for your quick reply.
I think this shortcut key will work for the static(pre-defined) menu.
We also need to deal with the dynamic menu like file selection to select
the secure boot key file, etc.
I can't imagine how this shortcut key works when the following file
name appears in the menu.

  db.auth
  db1.auth
  db2.auth
  dbx.auth
  dbx1.auth
  dbx2.auth

Another idea is that implementing the numeric navigation key like a flip phone.

  0: db.auth
  1: db1.auth
  2: db2.auth
  3: dbx.auth
  4: dbx1.auth
  5: dbx2.auth
  6: Quit

Pressing '2' selects db2.auth, pressing '4' selects dbx1.auth.

Thanks,
Masahisa Kojima

>
> The user would never use the '&' key.
>
> Best regards
>
> Heinrich
>
> >   }
> > diff --git a/include/efi_config.h b/include/efi_config.h
> > index 86bc801211..1b84e2d579 100644
> > --- a/include/efi_config.h
> > +++ b/include/efi_config.h
> > @@ -45,6 +45,7 @@ struct eficonfig_entry {
> >    * @active:         active menu entry index
> >    * @count:          total count of menu entry
> >    * @menu_header:    menu header string
> > + * @accessor_str:    pointer to the accessor string for entry shortcut
> >    * @list:           menu entry list structure
> >    */
> >   struct efimenu {
> > @@ -52,6 +53,8 @@ struct efimenu {
> >       int active;
> >       int count;
> >       char *menu_header;
> > +     bool direct_access_mode;
> > +     u16 *accessor_str;
> >       struct list_head list;
> >   };
> >
> > diff --git a/include/menu.h b/include/menu.h
> > index 702aacb170..03bf8dc4f5 100644
> > --- a/include/menu.h
> > +++ b/include/menu.h
> > @@ -51,6 +51,7 @@ enum bootmenu_key {
> >       KEY_PLUS,
> >       KEY_MINUS,
> >       KEY_SPACE,
> > +     KEY_AMPERSAND,
> >   };
> >
> >   void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>
Masahisa Kojima Oct. 25, 2022, 2:53 a.m. UTC | #3
Hi Heinrich,

On Mon, 24 Oct 2022 at 15:34, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> Hi Heinrich,
>
> On Mon, 24 Oct 2022 at 14:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 10/24/22 06:48, Masahisa Kojima wrote:
> > > This commit adds the direct menu entry access mode.
> > > User can select the menu entry by '&' key followed by
> > > the menu title name.
> > >
> > > User input is read in UTF-16, then UTF-16 string is converted
> > > into UTF-8 internally because string comparison relies on strncasecmp().
> > > There is no equivalent string comparison function for UTF-16.
> > >
> > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > > ---
> > > Newly added in v4
> > >
> > >   cmd/eficonfig.c      | 120 ++++++++++++++++++++++++++++++++++++++++++-
> > >   common/menu.c        |   3 ++
> > >   include/efi_config.h |   3 ++
> > >   include/menu.h       |   1 +
> > >   4 files changed, 126 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > > index 0cb0770ac3..56d9268f9f 100644
> > > --- a/cmd/eficonfig.c
> > > +++ b/cmd/eficonfig.c
> > > @@ -22,6 +22,7 @@
> > >
> > >   static struct efi_simple_text_input_protocol *cin;
> > >
> > > +#define EFICONFIG_ACCESSOR_STR_MAX 16
> > >   #define EFICONFIG_DESCRIPTION_MAX 32
> > >   #define EFICONFIG_OPTIONAL_DATA_MAX 64
> > >
> > > @@ -155,7 +156,28 @@ static void eficonfig_print_entry(void *data)
> > >       if (reverse)
> > >               puts(ANSI_COLOR_REVERSE);
> > >
> > > -     printf("%s", entry->title);
> > > +     if (reverse && entry->efi_menu->direct_access_mode) {
> > > +             size_t len = u16_strlen(entry->efi_menu->accessor_str);
> > > +             char *accessor_str, *p;
> > > +
> > > +             accessor_str = calloc(1, utf16_utf8_strlen(entry->efi_menu->accessor_str) + 1);
> > > +             if (!accessor_str) {
> > > +                     printf("%s", entry->title);
> > > +                     return;
> > > +             }
> > > +             p = accessor_str;
> > > +             utf16_utf8_strncpy(&p, entry->efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> > > +             len = strlen(accessor_str);
> > > +             if (!strncasecmp(accessor_str, entry->title, len)) {
> > > +                     printf("%.*s" ANSI_COLOR_RESET "%s", (int)len, entry->title,
> > > +                            &entry->title[len]);
> > > +             } else {
> > > +                     printf("%s", entry->title);
> > > +             }
> > > +             free(accessor_str);
> > > +     } else {
> > > +             printf("%s", entry->title);
> > > +     }
> > >
> > >       if (reverse)
> > >               puts(ANSI_COLOR_RESET);
> > > @@ -182,6 +204,83 @@ static void eficonfig_display_statusline(struct menu *m)
> > >              entry->efi_menu->count + 6, 1, entry->efi_menu->count + 7, 1);
> > >   }
> > >
> > > +/**
> > > + * eficonfig_handle_direct_accessor() - handle direct access user input
> > > + *
> > > + * @efi_menu:        pointer to the efimenu structure
> > > + * Return:   key string to identify the selected entry
> > > + */
> > > +static char *eficonfig_handle_direct_accessor(struct efimenu *efi_menu)
> > > +{
> > > +     efi_status_t ret;
> > > +     char *accessor_str, *p;
> > > +     struct efi_input_key key;
> > > +     struct list_head *pos, *n;
> > > +     struct eficonfig_entry *entry;
> > > +     static int len;
> > > +
> > > +     /* Read user input */
> > > +     do {
> > > +             ret = EFI_CALL(cin->read_key_stroke(cin, &key));
> > > +             mdelay(10);
> > > +     } while (ret == EFI_NOT_READY);
> > > +
> > > +     /* If user presses Ctrl+C or ESC, exit direct access mode */
> > > +     if (key.unicode_char == 0x3 || key.scan_code == 23)
> > > +             goto out;
> > > +
> > > +     /* If user presses ENTER, exit direct access mode and return the active entry */
> > > +     if (key.unicode_char == u'\r') {
> > > +             list_for_each_safe(pos, n, &efi_menu->list) {
> > > +                     entry = list_entry(pos, struct eficonfig_entry, list);
> > > +                     if (entry->num == efi_menu->active) {
> > > +                             efi_menu->direct_access_mode = false;
> > > +                             memset(efi_menu->accessor_str, 0,
> > > +                                    EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > > +                             return entry->key;
> > > +                     }
> > > +             }
> > > +
> > > +             /* no matching entry */
> > > +             goto out;
> > > +     }
> > > +
> > > +     /* Ignore other control code and efi scan code */
> > > +     if (key.unicode_char < 0x20 || key.scan_code != 0)
> > > +             return NULL;
> > > +
> > > +     len = u16_strlen(efi_menu->accessor_str);
> > > +     if (len < EFICONFIG_ACCESSOR_STR_MAX - 1)
> > > +             efi_menu->accessor_str[len] = key.unicode_char;
> > > +
> > > +     accessor_str = calloc(1, utf16_utf8_strlen(efi_menu->accessor_str) + 1);
> > > +     if (!accessor_str)
> > > +             return NULL;
> > > +
> > > +     p = accessor_str;
> > > +     utf16_utf8_strncpy(&p, efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
> > > +
> > > +     list_for_each_safe(pos, n, &efi_menu->list) {
> > > +             entry = list_entry(pos, struct eficonfig_entry, list);
> > > +             if (!strncasecmp(accessor_str, entry->title, strlen(accessor_str))) {
> > > +                     efi_menu->active = entry->num;
> > > +                     free(accessor_str);
> > > +                     return NULL;
> > > +             }
> > > +     }
> > > +
> > > +     /* does not match any entries */
> > > +     free(accessor_str);
> > > +     efi_menu->active = 0;
> > > +     return NULL;
> > > +
> > > +out:
> > > +     efi_menu->direct_access_mode = false;
> > > +     memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > > +     efi_menu->active = 0;
> > > +     return NULL;
> > > +}
> > > +
> > >   /**
> > >    * eficonfig_choice_entry() - user key input handler
> > >    *
> > > @@ -196,6 +295,9 @@ static char *eficonfig_choice_entry(void *data)
> > >       enum bootmenu_key key = KEY_NONE;
> > >       struct efimenu *efi_menu = data;
> > >
> > > +     if (efi_menu->direct_access_mode)
> > > +             return eficonfig_handle_direct_accessor(efi_menu);
> > > +
> > >       while (1) {
> > >               bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
> > >
> > > @@ -221,6 +323,10 @@ static char *eficonfig_choice_entry(void *data)
> > >                       /* Quit by choosing the last entry */
> > >                       entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
> > >                       return entry->key;
> > > +             case KEY_AMPERSAND:
> > > +                     memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > > +                     efi_menu->direct_access_mode = true;
> > > +                     return NULL;
> > >               default:
> > >                       /* Pressed key is not valid, no need to regenerate the menu */
> > >                       break;
> > > @@ -248,6 +354,7 @@ void eficonfig_destroy(struct efimenu *efi_menu)
> > >               free(entry);
> > >       }
> > >       free(efi_menu->menu_header);
> > > +     free(efi_menu->accessor_str);
> > >       free(efi_menu);
> > >   }
> > >
> > > @@ -385,6 +492,9 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_heade
> > >               if (!efi_menu->menu_header)
> > >                       return EFI_OUT_OF_RESOURCES;
> > >       }
> > > +     efi_menu->accessor_str = calloc(1, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
> > > +     if (!efi_menu->accessor_str)
> > > +             return EFI_OUT_OF_RESOURCES;
> > >
> > >       menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,
> > >                          eficonfig_print_entry, eficonfig_choice_entry,
> > > @@ -1866,6 +1976,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> > >       enum bootmenu_key key = KEY_NONE;
> > >       struct eficonfig_boot_order *entry;
> > >
> > > +     if (efi_menu->direct_access_mode) {
> > > +             eficonfig_handle_direct_accessor(efi_menu);
> > > +             return EFI_NOT_READY;
> > > +     }
> > > +
> > >       while (1) {
> > >               bootmenu_loop(NULL, &key, &esc);
> > >
> > > @@ -1931,6 +2046,9 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> > >                       break;
> > >               case KEY_QUIT:
> > >                       return EFI_ABORTED;
> > > +             case KEY_AMPERSAND:
> > > +                     efi_menu->direct_access_mode = true;
> > > +                     return EFI_NOT_READY;
> > >               default:
> > >                       /* Pressed key is not valid, no need to regenerate the menu */
> > >                       break;
> > > diff --git a/common/menu.c b/common/menu.c
> > > index 8fe00965c0..6ea9f5c9b8 100644
> > > --- a/common/menu.c
> > > +++ b/common/menu.c
> > > @@ -557,4 +557,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
> > >
> > >       if (c == ' ')
> > >               *key = KEY_SPACE;
> > > +
> > > +     if (c == '&')
> > > +             *key = KEY_AMPERSAND;
> >
> > I am not really happy with how U-Boot menus work.
> >
> > I think there should be one function to which you pass the menu entries
> > and you get back the index of the chosen entry (or some error code if
> > ESC for pressed).
> >
> > My idea about "ampersand" was: You pass a list of strings to the menu
> > function like:
> >
> > &Open
> > &Close
> > E&xit
> >
> > The displayed menu would highlight the access key in a different color,
> > e.g. white instead of grey.
> >
> > *O*pen
> > *C*lose
> > E*x*it
> >
> > The user can navigate with either UP, Down and press Enter then you will
> > get back the chosen entry. Or the user presses 'o', 'c', or 'x' and you
> > will get back the index of the respective menu entry.
>
> Thank you for your quick reply.
> I think this shortcut key will work for the static(pre-defined) menu.
> We also need to deal with the dynamic menu like file selection to select
> the secure boot key file, etc.
> I can't imagine how this shortcut key works when the following file
> name appears in the menu.
>
>   db.auth
>   db1.auth
>   db2.auth
>   dbx.auth
>   dbx1.auth
>   dbx2.auth
>
> Another idea is that implementing the numeric navigation key like a flip phone.
>
>   0: db.auth
>   1: db1.auth
>   2: db2.auth
>   3: dbx.auth
>   4: dbx1.auth
>   5: dbx2.auth
>   6: Quit
>
> Pressing '2' selects db2.auth, pressing '4' selects dbx1.auth.

This shortcut key implementation is feature enhancement and
not directly related to the UEFI Secure Boot key menu feature.
Let me make this shortcut implementation as a separate series.

Thanks,
Masahisa Kojima

>
> Thanks,
> Masahisa Kojima
>
> >
> > The user would never use the '&' key.
> >
> > Best regards
> >
> > Heinrich
> >
> > >   }
> > > diff --git a/include/efi_config.h b/include/efi_config.h
> > > index 86bc801211..1b84e2d579 100644
> > > --- a/include/efi_config.h
> > > +++ b/include/efi_config.h
> > > @@ -45,6 +45,7 @@ struct eficonfig_entry {
> > >    * @active:         active menu entry index
> > >    * @count:          total count of menu entry
> > >    * @menu_header:    menu header string
> > > + * @accessor_str:    pointer to the accessor string for entry shortcut
> > >    * @list:           menu entry list structure
> > >    */
> > >   struct efimenu {
> > > @@ -52,6 +53,8 @@ struct efimenu {
> > >       int active;
> > >       int count;
> > >       char *menu_header;
> > > +     bool direct_access_mode;
> > > +     u16 *accessor_str;
> > >       struct list_head list;
> > >   };
> > >
> > > diff --git a/include/menu.h b/include/menu.h
> > > index 702aacb170..03bf8dc4f5 100644
> > > --- a/include/menu.h
> > > +++ b/include/menu.h
> > > @@ -51,6 +51,7 @@ enum bootmenu_key {
> > >       KEY_PLUS,
> > >       KEY_MINUS,
> > >       KEY_SPACE,
> > > +     KEY_AMPERSAND,
> > >   };
> > >
> > >   void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >
diff mbox series

Patch

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 0cb0770ac3..56d9268f9f 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -22,6 +22,7 @@ 
 
 static struct efi_simple_text_input_protocol *cin;
 
+#define EFICONFIG_ACCESSOR_STR_MAX 16
 #define EFICONFIG_DESCRIPTION_MAX 32
 #define EFICONFIG_OPTIONAL_DATA_MAX 64
 
@@ -155,7 +156,28 @@  static void eficonfig_print_entry(void *data)
 	if (reverse)
 		puts(ANSI_COLOR_REVERSE);
 
-	printf("%s", entry->title);
+	if (reverse && entry->efi_menu->direct_access_mode) {
+		size_t len = u16_strlen(entry->efi_menu->accessor_str);
+		char *accessor_str, *p;
+
+		accessor_str = calloc(1, utf16_utf8_strlen(entry->efi_menu->accessor_str) + 1);
+		if (!accessor_str) {
+			printf("%s", entry->title);
+			return;
+		}
+		p = accessor_str;
+		utf16_utf8_strncpy(&p, entry->efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
+		len = strlen(accessor_str);
+		if (!strncasecmp(accessor_str, entry->title, len)) {
+			printf("%.*s" ANSI_COLOR_RESET "%s", (int)len, entry->title,
+			       &entry->title[len]);
+		} else {
+			printf("%s", entry->title);
+		}
+		free(accessor_str);
+	} else {
+		printf("%s", entry->title);
+	}
 
 	if (reverse)
 		puts(ANSI_COLOR_RESET);
@@ -182,6 +204,83 @@  static void eficonfig_display_statusline(struct menu *m)
 	       entry->efi_menu->count + 6, 1, entry->efi_menu->count + 7, 1);
 }
 
+/**
+ * eficonfig_handle_direct_accessor() - handle direct access user input
+ *
+ * @efi_menu:	pointer to the efimenu structure
+ * Return:	key string to identify the selected entry
+ */
+static char *eficonfig_handle_direct_accessor(struct efimenu *efi_menu)
+{
+	efi_status_t ret;
+	char *accessor_str, *p;
+	struct efi_input_key key;
+	struct list_head *pos, *n;
+	struct eficonfig_entry *entry;
+	static int len;
+
+	/* Read user input */
+	do {
+		ret = EFI_CALL(cin->read_key_stroke(cin, &key));
+		mdelay(10);
+	} while (ret == EFI_NOT_READY);
+
+	/* If user presses Ctrl+C or ESC, exit direct access mode */
+	if (key.unicode_char == 0x3 || key.scan_code == 23)
+		goto out;
+
+	/* If user presses ENTER, exit direct access mode and return the active entry */
+	if (key.unicode_char == u'\r') {
+		list_for_each_safe(pos, n, &efi_menu->list) {
+			entry = list_entry(pos, struct eficonfig_entry, list);
+			if (entry->num == efi_menu->active) {
+				efi_menu->direct_access_mode = false;
+				memset(efi_menu->accessor_str, 0,
+				       EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
+				return entry->key;
+			}
+		}
+
+		/* no matching entry */
+		goto out;
+	}
+
+	/* Ignore other control code and efi scan code */
+	if (key.unicode_char < 0x20 || key.scan_code != 0)
+		return NULL;
+
+	len = u16_strlen(efi_menu->accessor_str);
+	if (len < EFICONFIG_ACCESSOR_STR_MAX - 1)
+		efi_menu->accessor_str[len] = key.unicode_char;
+
+	accessor_str = calloc(1, utf16_utf8_strlen(efi_menu->accessor_str) + 1);
+	if (!accessor_str)
+		return NULL;
+
+	p = accessor_str;
+	utf16_utf8_strncpy(&p, efi_menu->accessor_str, EFICONFIG_ACCESSOR_STR_MAX);
+
+	list_for_each_safe(pos, n, &efi_menu->list) {
+		entry = list_entry(pos, struct eficonfig_entry, list);
+		if (!strncasecmp(accessor_str, entry->title, strlen(accessor_str))) {
+			efi_menu->active = entry->num;
+			free(accessor_str);
+			return NULL;
+		}
+	}
+
+	/* does not match any entries */
+	free(accessor_str);
+	efi_menu->active = 0;
+	return NULL;
+
+out:
+	efi_menu->direct_access_mode = false;
+	memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
+	efi_menu->active = 0;
+	return NULL;
+}
+
 /**
  * eficonfig_choice_entry() - user key input handler
  *
@@ -196,6 +295,9 @@  static char *eficonfig_choice_entry(void *data)
 	enum bootmenu_key key = KEY_NONE;
 	struct efimenu *efi_menu = data;
 
+	if (efi_menu->direct_access_mode)
+		return eficonfig_handle_direct_accessor(efi_menu);
+
 	while (1) {
 		bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
 
@@ -221,6 +323,10 @@  static char *eficonfig_choice_entry(void *data)
 			/* Quit by choosing the last entry */
 			entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
 			return entry->key;
+		case KEY_AMPERSAND:
+			memset(efi_menu->accessor_str, 0, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
+			efi_menu->direct_access_mode = true;
+			return NULL;
 		default:
 			/* Pressed key is not valid, no need to regenerate the menu */
 			break;
@@ -248,6 +354,7 @@  void eficonfig_destroy(struct efimenu *efi_menu)
 		free(entry);
 	}
 	free(efi_menu->menu_header);
+	free(efi_menu->accessor_str);
 	free(efi_menu);
 }
 
@@ -385,6 +492,9 @@  efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_heade
 		if (!efi_menu->menu_header)
 			return EFI_OUT_OF_RESOURCES;
 	}
+	efi_menu->accessor_str = calloc(1, EFICONFIG_ACCESSOR_STR_MAX * sizeof(u16));
+	if (!efi_menu->accessor_str)
+		return EFI_OUT_OF_RESOURCES;
 
 	menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,
 			   eficonfig_print_entry, eficonfig_choice_entry,
@@ -1866,6 +1976,11 @@  static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 	enum bootmenu_key key = KEY_NONE;
 	struct eficonfig_boot_order *entry;
 
+	if (efi_menu->direct_access_mode) {
+		eficonfig_handle_direct_accessor(efi_menu);
+		return EFI_NOT_READY;
+	}
+
 	while (1) {
 		bootmenu_loop(NULL, &key, &esc);
 
@@ -1931,6 +2046,9 @@  static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 			break;
 		case KEY_QUIT:
 			return EFI_ABORTED;
+		case KEY_AMPERSAND:
+			efi_menu->direct_access_mode = true;
+			return EFI_NOT_READY;
 		default:
 			/* Pressed key is not valid, no need to regenerate the menu */
 			break;
diff --git a/common/menu.c b/common/menu.c
index 8fe00965c0..6ea9f5c9b8 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -557,4 +557,7 @@  void bootmenu_loop(struct bootmenu_data *menu,
 
 	if (c == ' ')
 		*key = KEY_SPACE;
+
+	if (c == '&')
+		*key = KEY_AMPERSAND;
 }
diff --git a/include/efi_config.h b/include/efi_config.h
index 86bc801211..1b84e2d579 100644
--- a/include/efi_config.h
+++ b/include/efi_config.h
@@ -45,6 +45,7 @@  struct eficonfig_entry {
  * @active:		active menu entry index
  * @count:		total count of menu entry
  * @menu_header:	menu header string
+ * @accessor_str:	pointer to the accessor string for entry shortcut
  * @list:		menu entry list structure
  */
 struct efimenu {
@@ -52,6 +53,8 @@  struct efimenu {
 	int active;
 	int count;
 	char *menu_header;
+	bool direct_access_mode;
+	u16 *accessor_str;
 	struct list_head list;
 };
 
diff --git a/include/menu.h b/include/menu.h
index 702aacb170..03bf8dc4f5 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -51,6 +51,7 @@  enum bootmenu_key {
 	KEY_PLUS,
 	KEY_MINUS,
 	KEY_SPACE,
+	KEY_AMPERSAND,
 };
 
 void bootmenu_autoboot_loop(struct bootmenu_data *menu,