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

[Gerd Hoffmann]

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
Signed-off-by: Gerd Hoffmann 
---
 src/boot.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 10 deletions(-)

diff --git a/src/boot.c b/src/boot.c
index 9f82f3ca0c3e..a4795b9cb9a2 100644
--- 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;
+    }
+}
+
 // 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");
+                    break;
+                }
+                printf(" - invalid choice, try again\n");
+                digit1 = -1;
+            }
+        } else {
+            if (digit >= 1 && digit <= maxmenu) {
+                choice = digit;
+                break;
+            }
+        }
     }
     printf("\n");
-    if (scan_code == 0x01)
-        // ESC
-        return;

// Find entry and make top priority.
-    int choice = scan_code - 1;
     hlist_for_each_entry(pos, &BootList, node) {
         if (! --choice)
             break;

[PATCH 2/2] bootmenu: also accept keypad keys

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 src/boot.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/boot.c b/src/boot.c
index a4795b9cb9a2..620060460647 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -472,6 +472,15 @@ static int scancode_to_digit(int scan_code)
         return scan_code - 1;
     case 11:
         return 0;
+    // keypad
+    case 71 ... 73:
+        return scan_code - 71 + 7;
+    case 75 ... 77:
+        return scan_code - 75 + 4;
+    case 79 ... 81:
+        return scan_code - 79 + 1;
+    case 82:
+        return 0;
     default:
         return -1;
     }

[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

[Gerd Hoffmann]

On Thu, Apr 11, 2019 at 11:53:42AM -0400, Kevin O'Connor wrote:
> 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.

People ask rarely, but it keeps coming up now and then ...
> 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,

Using letter keys works for me too, and keeping the one keypress
selection logic is easier.  v2 follows in a moment ...

cheers,
  Gerd

[Kevin O'Connor]

On Thu, Apr 11, 2019 at 10:00:51PM +0200, Gerd Hoffmann wrote:
> On Thu, Apr 11, 2019 at 11:53:42AM -0400, Kevin O'Connor wrote:
> > 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.
>
> People ask rarely, but it keeps coming up now and then ...
> > 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,
>
> Using letter keys works for me too, and keeping the one keypress
> selection logic is easier.  v2 follows in a moment ...

Unfortunately, it looks like 13 and 14 are unprintable characters
(backspace and tab).  :-(

-Kevin