From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0ED56C3A59D for ; Mon, 24 Oct 2022 06:34:41 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 69AD984CDF; Mon, 24 Oct 2022 08:34:38 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="SlgwBOAK"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 38E6684CDF; Mon, 24 Oct 2022 08:34:37 +0200 (CEST) Received: from mail-vk1-xa36.google.com (mail-vk1-xa36.google.com [IPv6:2607:f8b0:4864:20::a36]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9B23984E9E for ; Mon, 24 Oct 2022 08:34:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=masahisa.kojima@linaro.org Received: by mail-vk1-xa36.google.com with SMTP id m18so1126095vka.10 for ; Sun, 23 Oct 2022 23:34:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=FjXtIZMvfIvJHNLDIr26GwGUgBhsFN3Ge5TbTInEToY=; b=SlgwBOAKK2iZzyBScv1Tc64y1l0aoL7wtdCMN+rnH042dq0bAV1KEJSK61lkosKz8B S2NeZKZvHSBFb9JHl+HJKBqwRMHGCNyWTDYvQMwPbWngCwbDkn2t1YY1itLWwln+3Tqk orTxp+a96TTn9XMXo8I9iO/dgp1qQGSPzs0IfCIIuZ1e3el7yDCjSoEMOizBoJpHdQOE UfVmTaMcnObZQ2b1A8B3SVHG/tqaVGxsx9xFqJmmxZYElII43aKdYfOKX9PSMYDAn0sK V+bKuQzKmmCxmcPix2B28/Cs1YAMW23VFec8G6P62H8gaYu8d25udABFhBcP13uGLv6B 1LqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FjXtIZMvfIvJHNLDIr26GwGUgBhsFN3Ge5TbTInEToY=; b=YadNQpVh5hN5AoWqtIN3BjPVB0TOwGFW+5+1ucxrb6JNvZ4l7Z1WMd2J1nrv2izjWa suo8VXuNhfqhP7TTIVtrnO4qGWxMxbfzGkdp6hx210JYtWl9DX7QVQzkiPimLYUJxrdY boyGA1MnoAmhPfuHxeaLZCjM0tQyTfsloeFmboL+bxaDnlmiiROryFDD2P9ximeU1Khv S0yautBgjS/oQBHDO+h52oLznXR07woRyM7UnPt3Cnb142VHjMHqxA4YVZ0fXA6j9Oal Cfcmoow1IhEtWyEhRDaSqiDlSiAyWhL9tEvhLjDmTKq36r1PAIf095VEav8hsjoXlclf lFtw== X-Gm-Message-State: ACrzQf3nmRHCxvowEaJRqJxu+8W4k+Ep13qaZnx4j3ZQybHNhSDrXCI4 ZtcOI9Cnn/t1RCK7RxpkJhnaei1GEM29ZVwrQO88/vccbg/PWA== X-Google-Smtp-Source: AMsMyM6cBydVrvbDuHmEGvlhVDeK1kLeuQXCEJgmlw+at6zFdfcGM5e6IJeE+bT2F8yNWqMQ5t31dktHxUFiHI4AIxA= X-Received: by 2002:a17:903:40d1:b0:17f:4e94:b747 with SMTP id t17-20020a17090340d100b0017f4e94b747mr30909986pld.44.1666593260749; Sun, 23 Oct 2022 23:34:20 -0700 (PDT) MIME-Version: 1.0 References: <20221024044804.3351-1-masahisa.kojima@linaro.org> <20221024044804.3351-4-masahisa.kojima@linaro.org> In-Reply-To: From: Masahisa Kojima Date: Mon, 24 Oct 2022 15:34:08 +0900 Message-ID: Subject: Re: [PATCH v4 3/7] eficonfig: add direct menu entry access mode To: Heinrich Schuchardt Cc: Ilias Apalodimas , Simon Glass , Takahiro Akashi , Stefan Roese , u-boot@lists.denx.de Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean Hi Heinrich, On Mon, 24 Oct 2022 at 14:40, Heinrich Schuchardt 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 > > --- > > 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, >