[SeaBIOS] Re: [PATCH 1/2] bootmenu: add support for more than 9 entries

[Kevin O'Connor]

On Thu, Apr 11, 2019 at 08:54:01AM +0200, Gerd Hoffmann wrote:
> In case more than 9 entries are found in the boot menu
> switch into two digit mode, so entries 10 and above can
> actually be selected.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1693031

FWIW, I'm surprised there's enough people that have run into this.

If there's a small set of individuals that need this support then one
possibility is to update the menu to show the actual character - for
example change the printf to:

printf("%c. %s\n", keycode_to_ascii(maxmenu)
           , strtcpy(desc, pos->description, ARRAY_SIZE(desc)));

(And implement the corresponding keycode_to_ascii() in kbd.c.)

If it's felt that a numeric entry is necessary, then my 2 cents would
be that in the greater than 9 case, a prompt followed by <enter> would
be preferable.

[...]
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -465,6 +465,18 @@ get_keystroke(int msec)
>
> #define DEFAULT_BOOTMENU_WAIT 2500
>
> +static int scancode_to_digit(int scan_code)
> +{
> switch (scan_code) {
> case 1 ... 10:
>    return scan_code - 1;
> case 11:
>    return 0;
> default:
>    return -1;
> }

FYI, this could use the scan_to_keycode[] table in kbd.c to make this
more convenient (eg, a keycode_to_ascii() function).
> +}
>
> // Show IPL option menu.
>  void
>  interactive_bootmenu(void)
> @@ -495,14 +507,25 @@ interactive_bootmenu(void)
>      printf("Select boot device:\n\n");
>      wait_threads();
> // Show menu items
> // Count menu items
> int maxmenu = 0;
> struct bootentry_s *pos;
> hlist_for_each_entry(pos, &BootList, node)
>    maxmenu++;
> int twodigits = (maxmenu > 9);
> // Show menu items
> maxmenu = 0;
> hlist_for_each_entry(pos, &BootList, node) {
>    char desc[77];
>    char desc[76];
>    maxmenu++;
>    printf("%d. %s\n", maxmenu
>           , strtcpy(desc, pos->description, ARRAY_SIZE(desc)));
>    if (twodigits) {
>        printf("%d%d. %s\n", maxmenu / 10, maxmenu % 10
>               , strtcpy(desc, pos->description, ARRAY_SIZE(desc)));
>    } else {
>        printf("%d. %s\n", maxmenu
>               , strtcpy(desc, pos->description, ARRAY_SIZE(desc)));
>    }
> }
>    if (tpm_can_show_menu()) {
>        printf("\nt. TPM Configuration\n");
>
> @@ -513,6 +536,7 @@ interactive_bootmenu(void)
>      // repeatedly hitting keys to enter the BIOS) will end up hitting ESC
>      // multiple times and immediately booting the primary boot device.
>      int esc_accepted_time = irqtimer_calc(menukey == 1 ? 1500 : 0);
> int digit, digit1 = -1, choice = 0;
> for (;;) {
>     scan_code = get_keystroke(1000);
>     if (scan_code == 1 && !irqtimer_check(esc_accepted_time))
>
> @@ -521,16 +545,48 @@ interactive_bootmenu(void)
>              printf("\n");
>              tpm_menu();
>          }
>    if (scan_code >= 1 && scan_code <= maxmenu+1)
>        break;
>    if (scan_code < 0) {
>        // timeout
>        continue;
>    }
>    if (scan_code == 0x01) {
>        // ESC
>        printf("\n");
>        return;
>    }
>    digit = scancode_to_digit(scan_code);
>    if (digit < 0) {
>        // key isn't a digit
>        continue;
>    }
>    if (twodigits) {
>        if (digit1 < 0) {
>            // first digit
>            printf("%d", digit);
>            digit1 = digit;
>        } else {
>            // second digit
>            printf("%d", digit);
>            choice = digit1 * 10 + digit;
>            if (choice >= 1 && choice <= maxmenu) {
>                printf(" - good choice, booting ...\n");

I'd avoid the term "good choice" as it could be interpretted as
commentary on the choosen payload.

-Kevin