boot: Unify idx type handling

This replaces several inconsistent use of types and unsound
signed comparisons.
The added max entry count is just for paranoia. Anyone with that
many boot entries deserves a medal.

1 files changed, 34 insertions, 29 deletions

diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c
index af24d1e788..f054ad18b9 100644
--- a/src/boot/efi/boot.c
+++ b/src/boot/efi/boot.c
@@ -71,8 +71,8 @@ typedef struct {
 typedef struct {
         ConfigEntry **entries;
         UINTN entry_count;
-        INTN idx_default;
-        INTN idx_default_efivar;
+        UINTN idx_default;
+        UINTN idx_default_efivar;
         UINT32 timeout_sec; /* Actual timeout used (efi_main() override > efivar > config). */
         UINT32 timeout_sec_config;
         UINT32 timeout_sec_efivar;
@@ -104,6 +104,11 @@ enum {
         TIMEOUT_TYPE_MAX    = UINT32_MAX,
 };
 
+enum {
+        IDX_MAX = INT16_MAX,
+        IDX_INVALID,
+};
+
 static void cursor_left(UINTN *cursor, UINTN *first) {
         assert(cursor);
         assert(first);
@@ -375,7 +380,7 @@ static UINTN entry_lookup_key(Config *config, UINTN start, CHAR16 key) {
         assert(config);
 
         if (key == 0)
-                return -1;
+                return IDX_INVALID;
 
         /* select entry by number key */
         if (key >= '1' && key <= '9') {
@@ -394,7 +399,7 @@ static UINTN entry_lookup_key(Config *config, UINTN start, CHAR16 key) {
                 if (config->entries[i]->key == key)
                         return i;
 
-        return -1;
+        return IDX_INVALID;
 }
 
 static CHAR16 *update_timeout_efivar(UINT32 *t, BOOLEAN inc) {
@@ -583,7 +588,7 @@ static BOOLEAN menu_run(
         UINTN visible_max = 0;
         UINTN idx_highlight = config->idx_default;
         UINTN idx_highlight_prev = 0;
-        UINTN idx_first = 0, idx_last = 0;
+        UINTN idx, idx_first = 0, idx_last = 0;
         BOOLEAN new_mode = TRUE, clear = TRUE;
         BOOLEAN refresh = TRUE, highlight = FALSE;
         UINTN x_start = 0, y_start = 0, y_status = 0;
@@ -592,10 +597,9 @@ static BOOLEAN menu_run(
         _cleanup_freepool_ CHAR16 *clearline = NULL, *status = NULL;
         UINT32 timeout_efivar_saved = config->timeout_sec_efivar;
         UINT32 timeout_remain = config->timeout_sec == TIMEOUT_MENU_FORCE ? 0 : config->timeout_sec;
-        INT16 idx;
         BOOLEAN exit = FALSE, run = TRUE, firmware_setup = FALSE;
         INT64 console_mode_initial = ST->ConOut->Mode->Mode, console_mode_efivar_saved = config->console_mode_efivar;
-        INTN default_efivar_saved = config->idx_default_efivar;
+        UINTN default_efivar_saved = config->idx_default_efivar;
 
         graphics_mode(FALSE);
         ST->ConIn->Reset(ST->ConIn, FALSE);
@@ -693,7 +697,7 @@ static BOOLEAN menu_run(
                                 print_at(x_start, y_start + i - idx_first,
                                          (i == idx_highlight) ? COLOR_HIGHLIGHT : COLOR_ENTRY,
                                          lines[i]);
-                                if ((INTN)i == config->idx_default_efivar)
+                                if (i == config->idx_default_efivar)
                                         print_at(x_start, y_start + i - idx_first,
                                                  (i == idx_highlight) ? COLOR_HIGHLIGHT : COLOR_ENTRY,
                                                  (CHAR16*) L"=>");
@@ -702,9 +706,9 @@ static BOOLEAN menu_run(
                 } else if (highlight) {
                         print_at(x_start, y_start + idx_highlight_prev - idx_first, COLOR_ENTRY, lines[idx_highlight_prev]);
                         print_at(x_start, y_start + idx_highlight - idx_first, COLOR_HIGHLIGHT, lines[idx_highlight]);
-                        if ((INTN)idx_highlight_prev == config->idx_default_efivar)
+                        if (idx_highlight_prev == config->idx_default_efivar)
                                 print_at(x_start , y_start + idx_highlight_prev - idx_first, COLOR_ENTRY, (CHAR16*) L"=>");
-                        if ((INTN)idx_highlight == config->idx_default_efivar)
+                        if (idx_highlight == config->idx_default_efivar)
                                 print_at(x_start, y_start + idx_highlight - idx_first, COLOR_HIGHLIGHT, (CHAR16*) L"=>");
                         highlight = FALSE;
                 }
@@ -826,14 +830,14 @@ static BOOLEAN menu_run(
 
                 case KEYPRESS(0, 0, 'd'):
                 case KEYPRESS(0, 0, 'D'):
-                        if (config->idx_default_efivar != (INTN)idx_highlight) {
+                        if (config->idx_default_efivar != idx_highlight) {
                                 FreePool(config->entry_default_efivar);
                                 config->entry_default_efivar = xstrdup(config->entries[idx_highlight]->id);
                                 config->idx_default_efivar = idx_highlight;
                                 status = xstrdup(L"Default boot entry selected.");
                         } else {
                                 config->entry_default_efivar = mfree(config->entry_default_efivar);
-                                config->idx_default_efivar = -1;
+                                config->idx_default_efivar = IDX_INVALID;
                                 status = xstrdup(L"Default boot entry cleared.");
                         }
                         config->use_saved_entry_efivar = FALSE;
@@ -923,7 +927,7 @@ static BOOLEAN menu_run(
                 default:
                         /* jump with a hotkey directly to a matching entry */
                         idx = entry_lookup_key(config, idx_highlight+1, KEYCHAR(key));
-                        if (idx < 0)
+                        if (idx == IDX_INVALID)
                                 break;
                         idx_highlight = idx;
                         refresh = TRUE;
@@ -974,6 +978,9 @@ static void config_add_entry(Config *config, ConfigEntry *entry) {
         assert(config);
         assert(entry);
 
+        /* This is just for paranoia. */
+        assert(config->entry_count < IDX_MAX);
+
         if ((config->entry_count & 15) == 0) {
                 UINTN i = config->entry_count + 16;
                 config->entries = xreallocate_pool(
@@ -1507,7 +1514,7 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) {
                 .auto_entries = TRUE,
                 .auto_firmware = TRUE,
                 .random_seed_mode = RANDOM_SEED_WITH_SYSTEM_TOKEN,
-                .idx_default_efivar = -1,
+                .idx_default_efivar = IDX_INVALID,
                 .console_mode = CONSOLE_MODE_KEEP,
                 .console_mode_efivar = CONSOLE_MODE_KEEP,
                 .timeout_sec_config = TIMEOUT_UNSET,
@@ -1633,32 +1640,32 @@ static void config_sort_entries(Config *config) {
         sort_pointer_array((void**) config->entries, config->entry_count, (compare_pointer_func_t) config_entry_compare);
 }
 
-static INTN config_entry_find(Config *config, const CHAR16 *needle) {
+static UINTN config_entry_find(Config *config, const CHAR16 *needle) {
         assert(config);
 
         if (!needle)
-                return -1;
+                return IDX_INVALID;
 
         for (UINTN i = 0; i < config->entry_count; i++)
                 if (MetaiMatch(config->entries[i]->id, (CHAR16*) needle))
-                        return (INTN) i;
+                        return i;
 
-        return -1;
+        return IDX_INVALID;
 }
 
 static void config_default_entry_select(Config *config) {
-        INTN i;
+        UINTN i;
 
         assert(config);
 
         i = config_entry_find(config, config->entry_oneshot);
-        if (i >= 0) {
+        if (i != IDX_INVALID) {
                 config->idx_default = i;
                 return;
         }
 
         i = config_entry_find(config, config->use_saved_entry_efivar ? config->entry_saved : config->entry_default_efivar);
-        if (i >= 0) {
+        if (i != IDX_INVALID) {
                 config->idx_default = i;
                 config->idx_default_efivar = i;
                 return;
@@ -1666,10 +1673,10 @@ static void config_default_entry_select(Config *config) {
 
         if (config->use_saved_entry)
                 /* No need to do the same thing twice. */
-                i = config->use_saved_entry_efivar ? -1 : config_entry_find(config, config->entry_saved);
+                i = config->use_saved_entry_efivar ? IDX_INVALID : config_entry_find(config, config->entry_saved);
         else
                 i = config_entry_find(config, config->entry_default_config);
-        if (i >= 0) {
+        if (i != IDX_INVALID) {
                 config->idx_default = i;
                 return;
         }
@@ -1684,7 +1691,7 @@ static void config_default_entry_select(Config *config) {
         }
 
         /* no entry found */
-        config->idx_default = -1;
+        config->idx_default = IDX_INVALID;
 }
 
 static BOOLEAN find_nonunique(ConfigEntry **entries, UINTN entry_count) {
@@ -2397,7 +2404,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
         config_default_entry_select(&config);
 
         /* if no configured entry to select from was found, enable the menu */
-        if (config.idx_default == -1) {
+        if (config.idx_default == IDX_INVALID) {
                 config.idx_default = 0;
                 if (config.timeout_sec == 0)
                         config.timeout_sec = 10;
@@ -2412,11 +2419,9 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
                 /* Block up to 100ms to give firmware time to get input working. */
                 err = console_key_read(&key, 100 * 1000);
                 if (!EFI_ERROR(err)) {
-                        INT16 idx;
-
                         /* find matching key in config entries */
-                        idx = entry_lookup_key(&config, config.idx_default, KEYCHAR(key));
-                        if (idx >= 0)
+                        UINTN idx = entry_lookup_key(&config, config.idx_default, KEYCHAR(key));
+                        if (idx != IDX_INVALID)
                                 config.idx_default = idx;
                         else
                                 menu = TRUE;