Merge pull request #29502 from keszybz/sd-boot-config-tweaks

Tweaks to sd-boot UX

1 files changed, 210 insertions, 246 deletions

diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c
index c28ec01975..2c2d0bf735 100644
--- a/src/boot/efi/boot.c
+++ b/src/boot/efi/boot.c
@@ -72,10 +72,10 @@ typedef struct {
         char16_t *path;
         char16_t *current_name;
         char16_t *next_name;
-} ConfigEntry;
+} BootEntry;
 
 typedef struct {
-        ConfigEntry **entries;
+        BootEntry **entries;
         size_t n_entries;
         size_t idx_default;
         size_t idx_default_efivar;
@@ -142,6 +142,8 @@ static bool line_edit(char16_t **line_in, size_t x_max, size_t y_pos) {
         _cleanup_free_ char16_t *line = NULL, *print = NULL;
         size_t size, len, first = 0, cursor = 0, clear = 0;
 
+        /* Edit the line and return true if it should be executed, false if not. */
+
         assert(line_in);
 
         len = strlen16(*line_in);
@@ -379,7 +381,7 @@ static size_t entry_lookup_key(Config *config, size_t start, char16_t key) {
                 return i-1;
         }
 
-        /* find matching key in config entries */
+        /* find matching key in boot entries */
         for (size_t i = start; i < config->n_entries; i++)
                 if (config->entries[i]->key == key)
                         return i;
@@ -391,27 +393,29 @@ static size_t entry_lookup_key(Config *config, size_t start, char16_t key) {
         return IDX_INVALID;
 }
 
-static char16_t *update_timeout_efivar(uint32_t *t, bool inc) {
-        assert(t);
+static char16_t* update_timeout_efivar(Config *config, bool inc) {
+        assert(config);
 
-        switch (*t) {
+        switch (config->timeout_sec) {
         case TIMEOUT_MAX:
-                *t = inc ? TIMEOUT_MAX : (*t - 1);
+                config->timeout_sec = inc ? TIMEOUT_MAX : config->timeout_sec - 1;
                 break;
         case TIMEOUT_UNSET:
-                *t = inc ? TIMEOUT_MENU_FORCE : TIMEOUT_UNSET;
+                config->timeout_sec = inc ? TIMEOUT_MENU_FORCE : TIMEOUT_UNSET;
                 break;
         case TIMEOUT_MENU_FORCE:
-                *t = inc ? TIMEOUT_MENU_HIDDEN : TIMEOUT_UNSET;
+                config->timeout_sec = inc ? TIMEOUT_MENU_HIDDEN : TIMEOUT_MENU_FORCE;
                 break;
         case TIMEOUT_MENU_HIDDEN:
-                *t = inc ? TIMEOUT_MIN : TIMEOUT_MENU_FORCE;
+                config->timeout_sec = inc ? TIMEOUT_MIN : TIMEOUT_MENU_FORCE;
                 break;
         default:
-                *t += inc ? 1 : -1;
+                config->timeout_sec = config->timeout_sec + (inc ? 1 : -1);
         }
 
-        switch (*t) {
+        config->timeout_sec_efivar = config->timeout_sec;
+
+        switch (config->timeout_sec) {
         case TIMEOUT_UNSET:
                 return xstrdup16(u"Menu timeout defined by configuration file.");
         case TIMEOUT_MENU_FORCE:
@@ -419,7 +423,7 @@ static char16_t *update_timeout_efivar(uint32_t *t, bool inc) {
         case TIMEOUT_MENU_HIDDEN:
                 return xstrdup16(u"Menu disabled. Hold down key at bootup to show menu.");
         default:
-                return xasprintf("Menu timeout set to %u s.", *t);
+                return xasprintf("Menu timeout set to %u s.", config->timeout_sec_efivar);
         }
 }
 
@@ -443,6 +447,19 @@ static bool ps_continue(void) {
                         !IN_SET(key, KEYPRESS(0, SCAN_ESC, 0), KEYPRESS(0, 0, 'q'), KEYPRESS(0, 0, 'Q'));
 }
 
+static void print_timeout_status(const char *label, uint32_t t) {
+        switch (t) {
+        case TIMEOUT_UNSET:
+                return;
+        case TIMEOUT_MENU_FORCE:
+                return (void) printf("%s: menu-force\n", label);
+        case TIMEOUT_MENU_HIDDEN:
+                return (void) printf("%s: menu-hidden\n", label);
+        default:
+                return (void) printf("%s: %u s\n", label, t);
+        }
+}
+
 static void print_status(Config *config, char16_t *loaded_image_path) {
         size_t x_max, y_max;
         uint32_t screen_width = 0, screen_height = 0;
@@ -480,31 +497,8 @@ static void print_status(Config *config, char16_t *loaded_image_path) {
         if (!ps_continue())
                 return;
 
-        switch (config->timeout_sec_config) {
-        case TIMEOUT_UNSET:
-                break;
-        case TIMEOUT_MENU_FORCE:
-                printf("      timeout (config): menu-force\n");
-                break;
-        case TIMEOUT_MENU_HIDDEN:
-                printf("      timeout (config): menu-hidden\n");
-                break;
-        default:
-                printf("      timeout (config): %u s\n", config->timeout_sec_config);
-        }
-
-        switch (config->timeout_sec_efivar) {
-        case TIMEOUT_UNSET:
-                break;
-        case TIMEOUT_MENU_FORCE:
-                printf("     timeout (EFI var): menu-force\n");
-                break;
-        case TIMEOUT_MENU_HIDDEN:
-                printf("     timeout (EFI var): menu-hidden\n");
-                break;
-        default:
-                printf("     timeout (EFI var): %u s\n", config->timeout_sec_efivar);
-        }
+        print_timeout_status("      timeout (config)", config->timeout_sec_config);
+        print_timeout_status("     timeout (EFI var)", config->timeout_sec_efivar);
 
         if (config->entry_default_config)
                 printf("      default (config): %ls\n", config->entry_default_config);
@@ -562,7 +556,7 @@ static void print_status(Config *config, char16_t *loaded_image_path) {
                 return;
 
         for (size_t i = 0; i < config->n_entries; i++) {
-                ConfigEntry *entry = config->entries[i];
+                BootEntry *entry = config->entries[i];
                 EFI_DEVICE_PATH *dp = NULL;
                 _cleanup_free_ char16_t *dp_str = NULL;
 
@@ -571,7 +565,7 @@ static void print_status(Config *config, char16_t *loaded_image_path) {
                                     EFI_SUCCESS)
                         (void) device_path_to_str(dp, &dp_str);
 
-                printf("  config entry: %zu/%zu\n", i + 1, config->n_entries);
+                printf("    boot entry: %zu/%zu\n", i + 1, config->n_entries);
                 printf("            id: %ls\n", entry->id);
                 if (entry->title)
                         printf("         title: %ls\n", entry->title);
@@ -607,37 +601,42 @@ static void print_status(Config *config, char16_t *loaded_image_path) {
         }
 }
 
-static EFI_STATUS reboot_into_firmware(void) {
+static EFI_STATUS set_reboot_into_firmware(void) {
         uint64_t osind = 0;
         EFI_STATUS err;
 
-        if (!FLAGS_SET(get_os_indications_supported(), EFI_OS_INDICATIONS_BOOT_TO_FW_UI))
-                return log_error_status(EFI_UNSUPPORTED, "Reboot to firmware interface not supported.");
-
         (void) efivar_get_uint64_le(MAKE_GUID_PTR(EFI_GLOBAL_VARIABLE), u"OsIndications", &osind);
         osind |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
 
         err = efivar_set_uint64_le(MAKE_GUID_PTR(EFI_GLOBAL_VARIABLE), u"OsIndications", osind, EFI_VARIABLE_NON_VOLATILE);
         if (err != EFI_SUCCESS)
-                return log_error_status(err, "Error setting OsIndications: %m");
-
-        RT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
-        assert_not_reached();
+                log_error_status(err, "Error setting OsIndications: %m");
+        return err;
 }
 
-static EFI_STATUS poweroff_system(void) {
+_noreturn_ static EFI_STATUS poweroff_system(void) {
         RT->ResetSystem(EfiResetShutdown, EFI_SUCCESS, 0, NULL);
         assert_not_reached();
 }
 
-static EFI_STATUS reboot_system(void) {
+_noreturn_ static EFI_STATUS reboot_system(void) {
         RT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
         assert_not_reached();
 }
 
+static EFI_STATUS reboot_into_firmware(void) {
+        EFI_STATUS err;
+
+        err = set_reboot_into_firmware();
+        if (err != EFI_SUCCESS)
+                return err;
+
+        return reboot_system();
+}
+
 static bool menu_run(
                 Config *config,
-                ConfigEntry **chosen_entry,
+                BootEntry **chosen_entry,
                 char16_t *loaded_image_path) {
 
         assert(config);
@@ -654,10 +653,18 @@ static bool menu_run(
         _cleanup_free_ char16_t *clearline = NULL, *separator = NULL, *status = NULL;
         uint32_t timeout_efivar_saved = config->timeout_sec_efivar;
         uint32_t timeout_remain = config->timeout_sec == TIMEOUT_MENU_FORCE ? 0 : config->timeout_sec;
-        bool exit = false, run = true, firmware_setup = false;
         int64_t console_mode_initial = ST->ConOut->Mode->Mode, console_mode_efivar_saved = config->console_mode_efivar;
         size_t default_efivar_saved = config->idx_default_efivar;
 
+        enum {
+                ACTION_CONTINUE,        /* Continue with loop over user input */
+                ACTION_FIRMWARE_SETUP,  /* Ask for confirmation and reboot into firmware setup */
+                ACTION_POWEROFF,        /* Power off the machine */
+                ACTION_REBOOT,          /* Reboot the machine */
+                ACTION_RUN,             /* Execute a boot entry */
+                ACTION_QUIT,            /* Return to the firmware */
+        } action = ACTION_CONTINUE;
+
         graphics_mode(false);
         ST->ConIn->Reset(ST->ConIn, false);
         ST->ConOut->EnableCursor(ST->ConOut, false);
@@ -673,7 +680,7 @@ static bool menu_run(
         }
 
         size_t line_width = 0, entry_padding = 3;
-        while (!exit) {
+        while (IN_SET(action, ACTION_CONTINUE, ACTION_FIRMWARE_SETUP)) {
                 uint64_t key;
 
                 if (new_mode) {
@@ -820,7 +827,7 @@ static bool menu_run(
                         assert(timeout_remain > 0);
                         timeout_remain--;
                         if (timeout_remain == 0) {
-                                exit = true;
+                                action = ACTION_RUN;
                                 break;
                         }
 
@@ -828,7 +835,7 @@ static bool menu_run(
                         continue;
                 }
                 if (err != EFI_SUCCESS) {
-                        exit = true;
+                        action = ACTION_RUN;
                         break;
                 }
 
@@ -839,10 +846,13 @@ static bool menu_run(
 
                 idx_highlight_prev = idx_highlight;
 
-                if (firmware_setup) {
-                        firmware_setup = false;
-                        if (IN_SET(key, KEYPRESS(0, 0, '\r'), KEYPRESS(0, 0, '\n')))
-                                (void) reboot_into_firmware();
+                if (action == ACTION_FIRMWARE_SETUP) {
+                        if (IN_SET(key, KEYPRESS(0, 0, '\r'), KEYPRESS(0, 0, '\n')) &&
+                            set_reboot_into_firmware() == EFI_SUCCESS)
+                                break;
+
+                        /* Any key other than newline or a failed attempt cancel the request. */
+                        action = ACTION_CONTINUE;
                         continue;
                 }
 
@@ -895,7 +905,7 @@ static bool menu_run(
                 case KEYPRESS(0, SCAN_F3, 0): /* EZpad Mini 4s firmware sends malformed events */
                 case KEYPRESS(0, SCAN_F3, '\r'): /* Teclast X98+ II firmware sends malformed events */
                 case KEYPRESS(0, SCAN_RIGHT, 0):
-                        exit = true;
+                        action = ACTION_RUN;
                         break;
 
                 case KEYPRESS(0, SCAN_F1, 0):
@@ -909,8 +919,7 @@ static bool menu_run(
                         break;
 
                 case KEYPRESS(0, 0, 'Q'):
-                        exit = true;
-                        run = false;
+                        action = ACTION_QUIT;
                         break;
 
                 case KEYPRESS(0, 0, 'd'):
@@ -931,12 +940,12 @@ static bool menu_run(
 
                 case KEYPRESS(0, 0, '-'):
                 case KEYPRESS(0, 0, 'T'):
-                        status = update_timeout_efivar(&config->timeout_sec_efivar, false);
+                        status = update_timeout_efivar(config, false);
                         break;
 
                 case KEYPRESS(0, 0, '+'):
                 case KEYPRESS(0, 0, 't'):
-                        status = update_timeout_efivar(&config->timeout_sec_efivar, true);
+                        status = update_timeout_efivar(config, true);
                         break;
 
                 case KEYPRESS(0, 0, 'e'):
@@ -964,7 +973,8 @@ static bool menu_run(
                          * Since we cannot paint the last character of the edit line, we simply start
                          * at x-offset 1 for symmetry. */
                         print_at(1, y_status, COLOR_EDIT, clearline + 2);
-                        exit = line_edit(&config->entries[idx_highlight]->options, x_max - 2, y_status);
+                        if (line_edit(&config->entries[idx_highlight]->options, x_max - 2, y_status))
+                                action = ACTION_RUN;
                         print_at(1, y_status, COLOR_NORMAL, clearline + 2);
 
                         /* The options string was now edited, hence we have to pass it to the invoked
@@ -1030,19 +1040,20 @@ static bool menu_run(
                 case KEYPRESS(0, SCAN_DELETE, 0): /* Same as F2. */
                 case KEYPRESS(0, SCAN_ESC, 0):    /* HP. */
                         if (FLAGS_SET(get_os_indications_supported(), EFI_OS_INDICATIONS_BOOT_TO_FW_UI)) {
-                                firmware_setup = true;
+                                action = ACTION_FIRMWARE_SETUP;
                                 /* Let's make sure the user really wants to do this. */
                                 status = xstrdup16(u"Press Enter to reboot into firmware interface.");
                         } else
                                 status = xstrdup16(u"Reboot into firmware interface not supported.");
                         break;
 
-                case KEYPRESS(0, 0, 'O'): /* Only uppercase, so that it can't be hit so easily fat-fingered, but still works safely over serial */
-                        (void) poweroff_system();
+                case KEYPRESS(0, 0, 'O'): /* Only uppercase, so that it can't be hit so easily fat-fingered,
+                                           * but still works safely over serial. */
+                        action = ACTION_POWEROFF;
                         break;
 
                 case KEYPRESS(0, 0, 'B'): /* ditto */
-                        (void) reboot_system();
+                        action = ACTION_REBOOT;
                         break;
 
                 default:
@@ -1068,8 +1079,6 @@ static bool menu_run(
                         highlight = true;
         }
 
-        *chosen_entry = config->entries[idx_highlight];
-
         /* Update EFI vars after we left the menu to reduce NVRAM writes. */
 
         if (default_efivar_saved != config->idx_default_efivar)
@@ -1100,11 +1109,25 @@ static bool menu_run(
                 }
         }
 
+        switch (action) {
+        case ACTION_CONTINUE:
+                assert_not_reached();
+        case ACTION_POWEROFF:
+                poweroff_system();
+        case ACTION_REBOOT:
+        case ACTION_FIRMWARE_SETUP:
+                reboot_system();
+        case ACTION_RUN:
+        case ACTION_QUIT:
+                break;
+        }
+
+        *chosen_entry = config->entries[idx_highlight];
         clear_screen(COLOR_NORMAL);
-        return run;
+        return action == ACTION_RUN;
 }
 
-static void config_add_entry(Config *config, ConfigEntry *entry) {
+static void config_add_entry(Config *config, BootEntry *entry) {
         assert(config);
         assert(entry);
 
@@ -1120,9 +1143,9 @@ static void config_add_entry(Config *config, ConfigEntry *entry) {
         config->entries[config->n_entries++] = entry;
 }
 
-static void config_entry_free(ConfigEntry *entry) {
+static BootEntry* boot_entry_free(BootEntry *entry) {
         if (!entry)
-                return;
+                return NULL;
 
         free(entry->id);
         free(entry->title_show);
@@ -1137,13 +1160,12 @@ static void config_entry_free(ConfigEntry *entry) {
         free(entry->path);
         free(entry->current_name);
         free(entry->next_name);
-        free(entry);
-}
 
-static void config_entry_freep(ConfigEntry **entry) {
-        config_entry_free(*entry);
+        return mfree(entry);
 }
 
+DEFINE_TRIVIAL_CLEANUP_FUNC(BootEntry *, boot_entry_free);
+
 static char *line_get_key_value(
                 char *content,
                 const char *sep,
@@ -1227,7 +1249,7 @@ static void config_defaults_load_from_file(Config *config, char *content) {
         assert(config);
         assert(content);
 
-        while ((line = line_get_key_value(content, " \t", &pos, &key, &value))) {
+        while ((line = line_get_key_value(content, " \t", &pos, &key, &value)))
                 if (streq8(key, "timeout")) {
                         if (streq8( value, "menu-force"))
                                 config->timeout_sec_config = TIMEOUT_MENU_FORCE;
@@ -1243,69 +1265,52 @@ static void config_defaults_load_from_file(Config *config, char *content) {
                                 config->timeout_sec_config = u;
                         }
                         config->timeout_sec = config->timeout_sec_config;
-                        continue;
-                }
 
-                if (streq8(key, "default")) {
+                } else if (streq8(key, "default")) {
                         if (value[0] == '@' && !strcaseeq8(value, "@saved")) {
                                 log_error("Unsupported special entry identifier, ignoring: %s", value);
                                 continue;
                         }
                         free(config->entry_default_config);
                         config->entry_default_config = xstr8_to_16(value);
-                        continue;
-                }
 
-                if (streq8(key, "editor")) {
+                } else if (streq8(key, "editor")) {
                         err = parse_boolean(value, &config->editor);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'editor' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "auto-entries")) {
+                } else if (streq8(key, "auto-entries")) {
                         err = parse_boolean(value, &config->auto_entries);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'auto-entries' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "auto-firmware")) {
+                } else if (streq8(key, "auto-firmware")) {
                         err = parse_boolean(value, &config->auto_firmware);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'auto-firmware' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "auto-poweroff")) {
+                } else if (streq8(key, "auto-poweroff")) {
                         err = parse_boolean(value, &config->auto_poweroff);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'auto-poweroff' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "auto-reboot")) {
+                } else if (streq8(key, "auto-reboot")) {
                         err = parse_boolean(value, &config->auto_reboot);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'auto-reboot' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "beep")) {
+                } else if (streq8(key, "beep")) {
                         err = parse_boolean(value, &config->beep);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'beep' config option, ignoring: %s", value);
-                        continue;
-                }
 
-                if (streq8(key, "reboot-for-bitlocker")) {
+                } else if (streq8(key, "reboot-for-bitlocker")) {
                         err = parse_boolean(value, &config->reboot_for_bitlocker);
                         if (err != EFI_SUCCESS)
                                 log_error("Error parsing 'reboot-for-bitlocker' config option, ignoring: %s",
                                           value);
-                }
 
-                if (streq8(key, "secure-boot-enroll")) {
+                } else if (streq8(key, "secure-boot-enroll")) {
                         if (streq8(value, "manual"))
                                 config->secure_boot_enroll = ENROLL_MANUAL;
                         else if (streq8(value, "force"))
@@ -1317,10 +1322,8 @@ static void config_defaults_load_from_file(Config *config, char *content) {
                         else
                                 log_error("Error parsing 'secure-boot-enroll' config option, ignoring: %s",
                                           value);
-                        continue;
-                }
 
-                if (streq8(key, "console-mode")) {
+                } else if (streq8(key, "console-mode")) {
                         if (streq8(value, "auto"))
                                 config->console_mode = CONSOLE_MODE_AUTO;
                         else if (streq8(value, "max"))
@@ -1336,13 +1339,11 @@ static void config_defaults_load_from_file(Config *config, char *content) {
                                 }
                                 config->console_mode = u;
                         }
-                        continue;
                 }
-        }
 }
 
-static void config_entry_parse_tries(
-                ConfigEntry *entry,
+static void boot_entry_parse_tries(
+                BootEntry *entry,
                 const char16_t *path,
                 const char16_t *file,
                 const char16_t *suffix) {
@@ -1405,7 +1406,7 @@ static void config_entry_parse_tries(
                         suffix);
 }
 
-static EFI_STATUS config_entry_bump_counters(ConfigEntry *entry) {
+static EFI_STATUS boot_entry_bump_counters(BootEntry *entry) {
         _cleanup_free_ char16_t* old_path = NULL, *new_path = NULL;
         _cleanup_(file_closep) EFI_FILE *handle = NULL;
         _cleanup_free_ EFI_FILE_INFO *file_info = NULL;
@@ -1461,7 +1462,7 @@ static EFI_STATUS config_entry_bump_counters(ConfigEntry *entry) {
         return EFI_SUCCESS;
 }
 
-static void config_entry_add_type1(
+static void boot_entry_add_type1(
                 Config *config,
                 EFI_HANDLE *device,
                 EFI_FILE *root_dir,
@@ -1470,7 +1471,7 @@ static void config_entry_add_type1(
                 char *content,
                 const char16_t *loaded_image_path) {
 
-        _cleanup_(config_entry_freep) ConfigEntry *entry = NULL;
+        _cleanup_(boot_entry_freep) BootEntry *entry = NULL;
         char *line;
         size_t pos = 0, n_initrd = 0;
         char *key, *value;
@@ -1483,46 +1484,36 @@ static void config_entry_add_type1(
         assert(file);
         assert(content);
 
-        entry = xnew(ConfigEntry, 1);
-        *entry = (ConfigEntry) {
+        entry = xnew(BootEntry, 1);
+        *entry = (BootEntry) {
                 .tries_done = -1,
                 .tries_left = -1,
         };
 
-        while ((line = line_get_key_value(content, " \t", &pos, &key, &value))) {
+        while ((line = line_get_key_value(content, " \t", &pos, &key, &value)))
                 if (streq8(key, "title")) {
                         free(entry->title);
                         entry->title = xstr8_to_16(value);
-                        continue;
-                }
 
-                if (streq8(key, "sort-key")) {
+                } else if (streq8(key, "sort-key")) {
                         free(entry->sort_key);
                         entry->sort_key = xstr8_to_16(value);
-                        continue;
-                }
 
-                if (streq8(key, "version")) {
+                } else if (streq8(key, "version")) {
                         free(entry->version);
                         entry->version = xstr8_to_16(value);
-                        continue;
-                }
 
-                if (streq8(key, "machine-id")) {
+                } else if (streq8(key, "machine-id")) {
                         free(entry->machine_id);
                         entry->machine_id = xstr8_to_16(value);
-                        continue;
-                }
 
-                if (streq8(key, "linux")) {
+                } else if (streq8(key, "linux")) {
                         free(entry->loader);
                         entry->type = LOADER_LINUX;
                         entry->loader = xstr8_to_path(value);
                         entry->key = 'l';
-                        continue;
-                }
 
-                if (streq8(key, "efi")) {
+                } else if (streq8(key, "efi")) {
                         entry->type = LOADER_EFI;
                         free(entry->loader);
                         entry->loader = xstr8_to_path(value);
@@ -1532,35 +1523,27 @@ static void config_entry_add_type1(
                                 entry->type = LOADER_UNDEFINED;
                                 break;
                         }
-                        continue;
-                }
 
-                if (streq8(key, "architecture")) {
+                } else if (streq8(key, "architecture")) {
                         /* do not add an entry for an EFI image of architecture not matching with that of the image */
                         if (!streq8(value, EFI_MACHINE_TYPE_NAME)) {
                                 entry->type = LOADER_UNDEFINED;
                                 break;
                         }
-                        continue;
-                }
 
-                if (streq8(key, "devicetree")) {
+                } else if (streq8(key, "devicetree")) {
                         free(entry->devicetree);
                         entry->devicetree = xstr8_to_path(value);
-                        continue;
-                }
 
-                if (streq8(key, "initrd")) {
+                } else if (streq8(key, "initrd")) {
                         entry->initrd = xrealloc(
                                 entry->initrd,
                                 n_initrd == 0 ? 0 : (n_initrd + 1) * sizeof(uint16_t *),
                                 (n_initrd + 2) * sizeof(uint16_t *));
                         entry->initrd[n_initrd++] = xstr8_to_path(value);
                         entry->initrd[n_initrd] = NULL;
-                        continue;
-                }
 
-                if (streq8(key, "options")) {
+                } else if (streq8(key, "options")) {
                         _cleanup_free_ char16_t *new = NULL;
 
                         new = xstr8_to_16(value);
@@ -1570,10 +1553,7 @@ static void config_entry_add_type1(
                                 entry->options = s;
                         } else
                                 entry->options = TAKE_PTR(new);
-
-                        continue;
                 }
-        }
 
         if (entry->type == LOADER_UNDEFINED)
                 return;
@@ -1590,7 +1570,7 @@ static void config_entry_add_type1(
 
         config_add_entry(config, entry);
 
-        config_entry_parse_tries(entry, path, file, u".conf");
+        boot_entry_parse_tries(entry, path, file, u".conf");
         TAKE_PTR(entry);
 }
 
@@ -1692,7 +1672,7 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) {
                 (void) efivar_get(MAKE_GUID_PTR(LOADER), u"LoaderEntryLastBooted", &config->entry_saved);
 }
 
-static void config_load_entries(
+static void config_load_type1_entries(
                 Config *config,
                 EFI_HANDLE *device,
                 EFI_FILE *root_dir,
@@ -1732,11 +1712,11 @@ static void config_load_entries(
 
                 err = file_read(entries_dir, f->FileName, 0, 0, &content, NULL);
                 if (err == EFI_SUCCESS)
-                        config_entry_add_type1(config, device, root_dir, u"\\loader\\entries", f->FileName, content, loaded_image_path);
+                        boot_entry_add_type1(config, device, root_dir, u"\\loader\\entries", f->FileName, content, loaded_image_path);
         }
 }
 
-static int config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
+static int boot_entry_compare(const BootEntry *a, const BootEntry *b) {
         int r;
 
         assert(a);
@@ -1790,7 +1770,7 @@ static int config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
         return CMP(a->tries_done, b->tries_done);
 }
 
-static size_t config_entry_find(Config *config, const char16_t *pattern) {
+static size_t config_find_entry(Config *config, const char16_t *pattern) {
         assert(config);
 
         /* We expect pattern and entry IDs to be already case folded. */
@@ -1805,18 +1785,18 @@ static size_t config_entry_find(Config *config, const char16_t *pattern) {
         return IDX_INVALID;
 }
 
-static void config_default_entry_select(Config *config) {
+static void config_select_default_entry(Config *config) {
         size_t i;
 
         assert(config);
 
-        i = config_entry_find(config, config->entry_oneshot);
+        i = config_find_entry(config, config->entry_oneshot);
         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);
+        i = config_find_entry(config, config->use_saved_entry_efivar ? config->entry_saved : config->entry_default_efivar);
         if (i != IDX_INVALID) {
                 config->idx_default = i;
                 config->idx_default_efivar = i;
@@ -1825,21 +1805,20 @@ 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 ? IDX_INVALID : config_entry_find(config, config->entry_saved);
+                i = config->use_saved_entry_efivar ? IDX_INVALID : config_find_entry(config, config->entry_saved);
         else
-                i = config_entry_find(config, config->entry_default_config);
+                i = config_find_entry(config, config->entry_default_config);
         if (i != IDX_INVALID) {
                 config->idx_default = i;
                 return;
         }
 
         /* select the first suitable entry */
-        for (i = 0; i < config->n_entries; i++) {
-                if (config->entries[i]->type == LOADER_AUTO || config->entries[i]->call)
-                        continue;
-                config->idx_default = i;
-                return;
-        }
+        for (i = 0; i < config->n_entries; i++)
+                if (config->entries[i]->type != LOADER_AUTO && !config->entries[i]->call) {
+                        config->idx_default = i;
+                        return;
+                }
 
         /* If no configured entry to select from was found, enable the menu. */
         config->idx_default = 0;
@@ -1847,7 +1826,7 @@ static void config_default_entry_select(Config *config) {
                 config->timeout_sec = 10;
 }
 
-static bool entries_unique(ConfigEntry **entries, bool *unique, size_t n_entries) {
+static bool entries_unique(BootEntry **entries, bool *unique, size_t n_entries) {
         bool is_unique = true;
 
         assert(entries);
@@ -1864,8 +1843,8 @@ static bool entries_unique(ConfigEntry **entries, bool *unique, size_t n_entries
         return is_unique;
 }
 
-/* generate a unique title, avoiding non-distinguishable menu entries */
-static void config_title_generate(Config *config) {
+/* generate unique titles, avoiding non-distinguishable menu entries */
+static void generate_boot_entry_titles(Config *config) {
         assert(config);
 
         bool unique[config->n_entries];
@@ -1947,7 +1926,7 @@ static bool is_sd_boot(EFI_FILE *root_dir, const char16_t *loader_path) {
         return memcmp(content, SD_MAGIC, sizeof(SD_MAGIC)) == 0;
 }
 
-static ConfigEntry *config_entry_add_loader_auto(
+static BootEntry* config_add_entry_loader_auto(
                 Config *config,
                 EFI_HANDLE *device,
                 EFI_FILE *root_dir,
@@ -1986,8 +1965,8 @@ static ConfigEntry *config_entry_add_loader_auto(
         if (err != EFI_SUCCESS)
                 return NULL;
 
-        ConfigEntry *entry = xnew(ConfigEntry, 1);
-        *entry = (ConfigEntry) {
+        BootEntry *entry = xnew(BootEntry, 1);
+        *entry = (BootEntry) {
                 .id = xstrdup16(id),
                 .type = LOADER_AUTO,
                 .title = xstrdup16(title),
@@ -2002,7 +1981,7 @@ static ConfigEntry *config_entry_add_loader_auto(
         return entry;
 }
 
-static void config_entry_add_osx(Config *config) {
+static void config_add_entry_osx(Config *config) {
         EFI_STATUS err;
         size_t n_handles = 0;
         _cleanup_free_ EFI_HANDLE *handles = NULL;
@@ -2023,7 +2002,7 @@ static void config_entry_add_osx(Config *config) {
                 if (open_volume(handles[i], &root) != EFI_SUCCESS)
                         continue;
 
-                if (config_entry_add_loader_auto(
+                if (config_add_entry_loader_auto(
                                 config,
                                 handles[i],
                                 root,
@@ -2119,7 +2098,7 @@ static EFI_STATUS boot_windows_bitlocker(void) {
 }
 #endif
 
-static void config_entry_add_windows(Config *config, EFI_HANDLE *device, EFI_FILE *root_dir) {
+static void config_add_entry_windows(Config *config, EFI_HANDLE *device, EFI_FILE *root_dir) {
 #if defined(__i386__) || defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)
         _cleanup_free_ char *bcd = NULL;
         char16_t *title = NULL;
@@ -2138,16 +2117,16 @@ static void config_entry_add_windows(Config *config, EFI_HANDLE *device, EFI_FIL
         if (err == EFI_SUCCESS)
                 title = get_bcd_title((uint8_t *) bcd, len);
 
-        ConfigEntry *e = config_entry_add_loader_auto(config, device, root_dir, NULL,
-                                                      u"auto-windows", 'w', title ?: u"Windows Boot Manager",
-                                                      u"\\EFI\\Microsoft\\Boot\\bootmgfw.efi");
+        BootEntry *e = config_add_entry_loader_auto(config, device, root_dir, NULL,
+                                                    u"auto-windows", 'w', title ?: u"Windows Boot Manager",
+                                                    u"\\EFI\\Microsoft\\Boot\\bootmgfw.efi");
 
         if (config->reboot_for_bitlocker)
                 e->call = boot_windows_bitlocker;
 #endif
 }
 
-static void config_entry_add_unified(
+static void config_load_type2_entries(
                 Config *config,
                 EFI_HANDLE *device,
                 EFI_FILE *root_dir) {
@@ -2210,55 +2189,39 @@ static void config_entry_add_unified(
                         continue;
 
                 /* read properties from the embedded os-release file */
-                while ((line = line_get_key_value(content, "=", &pos, &key, &value))) {
+                while ((line = line_get_key_value(content, "=", &pos, &key, &value)))
                         if (streq8(key, "PRETTY_NAME")) {
                                 free(os_pretty_name);
                                 os_pretty_name = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "IMAGE_ID")) {
+                        } else if (streq8(key, "IMAGE_ID")) {
                                 free(os_image_id);
                                 os_image_id = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "NAME")) {
+                        } else if (streq8(key, "NAME")) {
                                 free(os_name);
                                 os_name = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "ID")) {
+                        } else if (streq8(key, "ID")) {
                                 free(os_id);
                                 os_id = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "IMAGE_VERSION")) {
+                        } else if (streq8(key, "IMAGE_VERSION")) {
                                 free(os_image_version);
                                 os_image_version = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "VERSION")) {
+                        } else if (streq8(key, "VERSION")) {
                                 free(os_version);
                                 os_version = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "VERSION_ID")) {
+                        } else if (streq8(key, "VERSION_ID")) {
                                 free(os_version_id);
                                 os_version_id = xstr8_to_16(value);
-                                continue;
-                        }
 
-                        if (streq8(key, "BUILD_ID")) {
+                        } else if (streq8(key, "BUILD_ID")) {
                                 free(os_build_id);
                                 os_build_id = xstr8_to_16(value);
-                                continue;
                         }
-                }
 
                 if (!bootspec_pick_name_version_sort_key(
                                     os_pretty_name,
@@ -2274,8 +2237,8 @@ static void config_entry_add_unified(
                                     &good_sort_key))
                         continue;
 
-                ConfigEntry *entry = xnew(ConfigEntry, 1);
-                *entry = (ConfigEntry) {
+                BootEntry *entry = xnew(BootEntry, 1);
+                *entry = (BootEntry) {
                         .id = xstrdup16(f->FileName),
                         .type = LOADER_UNIFIED_LINUX,
                         .title = xstrdup16(good_name),
@@ -2290,7 +2253,7 @@ static void config_entry_add_unified(
 
                 strtolower16(entry->id);
                 config_add_entry(config, entry);
-                config_entry_parse_tries(entry, u"\\EFI\\Linux", f->FileName, u".efi");
+                boot_entry_parse_tries(entry, u"\\EFI\\Linux", f->FileName, u".efi");
 
                 if (szs[SECTION_CMDLINE] == 0)
                         continue;
@@ -2323,13 +2286,13 @@ static void config_load_xbootldr(
         if (err != EFI_SUCCESS)
                 return;
 
-        config_entry_add_unified(config, new_device, root_dir);
-        config_load_entries(config, new_device, root_dir, NULL);
+        config_load_type2_entries(config, new_device, root_dir);
+        config_load_type1_entries(config, new_device, root_dir, NULL);
 }
 
 static EFI_STATUS initrd_prepare(
                 EFI_FILE *root,
-                const ConfigEntry *entry,
+                const BootEntry *entry,
                 char16_t **ret_options,
                 void **ret_initrd,
                 size_t *ret_initrd_size) {
@@ -2405,7 +2368,7 @@ static EFI_STATUS initrd_prepare(
 
 static EFI_STATUS image_start(
                 EFI_HANDLE parent_image,
-                const ConfigEntry *entry) {
+                const BootEntry *entry) {
 
         _cleanup_(devicetree_cleanup) struct devicetree_state dtstate = {};
         _cleanup_(unload_imagep) EFI_HANDLE image = NULL;
@@ -2439,7 +2402,8 @@ static EFI_STATUS image_start(
                 return log_error_status(err, "Error loading %ls: %m", entry->loader);
 
         /* DTBs are loaded by the kernel before ExitBootServices, and they can be used to map and assign
-         * arbitrary memory ranges, so skip it when secure boot is enabled as the DTB here is unverified. */
+         * arbitrary memory ranges, so skip them when secure boot is enabled as the DTB here is unverified.
+         */
         if (entry->devicetree && !secure_boot_enabled()) {
                 err = devicetree_install(&dtstate, image_root, entry->devicetree);
                 if (err != EFI_SUCCESS)
@@ -2456,8 +2420,8 @@ static EFI_STATUS image_start(
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Error getting LoadedImageProtocol handle: %m");
 
-        /* If we had to append an initrd= entry to the command line, we have to pass it, and measure
-         * it. Otherwise, only pass/measure it if it is not implicit anyway (i.e. embedded into the UKI or
+        /* If we had to append an initrd= entry to the command line, we have to pass it, and measure it.
+         * Otherwise, only pass/measure it if it is not implicit anyway (i.e. embedded into the UKI or
          * so). */
         char16_t *options = options_initrd ?: entry->options_implied ? NULL : entry->options;
         if (options) {
@@ -2500,7 +2464,7 @@ static EFI_STATUS image_start(
 static void config_free(Config *config) {
         assert(config);
         for (size_t i = 0; i < config->n_entries; i++)
-                config_entry_free(config->entries[i]);
+                boot_entry_free(config->entries[i]);
         free(config->entries);
         free(config->entry_default_config);
         free(config->entry_default_efivar);
@@ -2529,7 +2493,7 @@ static void config_write_entries_to_variable(Config *config) {
         (void) efivar_set_raw(MAKE_GUID_PTR(LOADER), u"LoaderEntries", buffer, sz, 0);
 }
 
-static void save_selected_entry(const Config *config, const ConfigEntry *entry) {
+static void save_selected_entry(const Config *config, const BootEntry *entry) {
         assert(config);
         assert(entry);
         assert(entry->loader || !entry->call);
@@ -2569,7 +2533,7 @@ static EFI_STATUS secure_boot_discover_keys(Config *config, EFI_FILE *root_dir)
         for (;;) {
                 _cleanup_free_ EFI_FILE_INFO *dirent = NULL;
                 size_t dirent_size = 0;
-                ConfigEntry *entry = NULL;
+                BootEntry *entry = NULL;
 
                 err = readdir(keys_basedir, &dirent, &dirent_size);
                 if (err != EFI_SUCCESS || !dirent)
@@ -2581,8 +2545,8 @@ static EFI_STATUS secure_boot_discover_keys(Config *config, EFI_FILE *root_dir)
                 if (!FLAGS_SET(dirent->Attribute, EFI_FILE_DIRECTORY))
                         continue;
 
-                entry = xnew(ConfigEntry, 1);
-                *entry = (ConfigEntry) {
+                entry = xnew(BootEntry, 1);
+                *entry = (BootEntry) {
                         .id = xasprintf("secure-boot-keys-%ls", dirent->FileName),
                         .title = xasprintf("Enroll Secure Boot keys: %ls", dirent->FileName),
                         .path = xasprintf("\\loader\\keys\\%ls", dirent->FileName),
@@ -2594,8 +2558,8 @@ static EFI_STATUS secure_boot_discover_keys(Config *config, EFI_FILE *root_dir)
 
                 if (IN_SET(config->secure_boot_enroll, ENROLL_IF_SAFE, ENROLL_FORCE) &&
                     strcaseeq16(dirent->FileName, u"auto"))
-                        /* if we auto enroll successfully this call does not return, if it fails we still
-                         * want to add other potential entries to the menu */
+                        /* If we auto enroll successfully this call does not return.
+                         * If it fails we still want to add other potential entries to the menu. */
                         secure_boot_enroll_at(root_dir, entry->path, config->secure_boot_enroll == ENROLL_FORCE);
         }
 
@@ -2659,29 +2623,29 @@ static void config_load_all_entries(
 
         config_load_defaults(config, root_dir);
 
-        /* scan /EFI/Linux/ directory */
-        config_entry_add_unified(config, loaded_image->DeviceHandle, root_dir);
+        /* Scan /EFI/Linux/ directory */
+        config_load_type2_entries(config, loaded_image->DeviceHandle, root_dir);
 
-        /* scan /loader/entries/\*.conf files */
-        config_load_entries(config, loaded_image->DeviceHandle, root_dir, loaded_image_path);
+        /* Scan /loader/entries/\*.conf files */
+        config_load_type1_entries(config, loaded_image->DeviceHandle, root_dir, loaded_image_path);
 
         /* Similar, but on any XBOOTLDR partition */
         config_load_xbootldr(config, loaded_image->DeviceHandle);
 
-        /* sort entries after version number */
-        sort_pointer_array((void **) config->entries, config->n_entries, (compare_pointer_func_t) config_entry_compare);
+        /* Sort entries after version number */
+        sort_pointer_array((void **) config->entries, config->n_entries, (compare_pointer_func_t) boot_entry_compare);
 
-        /* if we find some well-known loaders, add them to the end of the list */
-        config_entry_add_osx(config);
-        config_entry_add_windows(config, loaded_image->DeviceHandle, root_dir);
-        config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, NULL,
+        /* If we find some well-known loaders, add them to the end of the list */
+        config_add_entry_osx(config);
+        config_add_entry_windows(config, loaded_image->DeviceHandle, root_dir);
+        config_add_entry_loader_auto(config, loaded_image->DeviceHandle, root_dir, NULL,
                                      u"auto-efi-shell", 's', u"EFI Shell", u"\\shell" EFI_MACHINE_TYPE_NAME ".efi");
-        config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
+        config_add_entry_loader_auto(config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
                                      u"auto-efi-default", '\0', u"EFI Default Loader", NULL);
 
         if (config->auto_firmware && FLAGS_SET(get_os_indications_supported(), EFI_OS_INDICATIONS_BOOT_TO_FW_UI)) {
-                ConfigEntry *entry = xnew(ConfigEntry, 1);
-                *entry = (ConfigEntry) {
+                BootEntry *entry = xnew(BootEntry, 1);
+                *entry = (BootEntry) {
                         .id = xstrdup16(u"auto-reboot-to-firmware-setup"),
                         .title = xstrdup16(u"Reboot Into Firmware Interface"),
                         .call = reboot_into_firmware,
@@ -2692,8 +2656,8 @@ static void config_load_all_entries(
         }
 
         if (config->auto_poweroff) {
-                ConfigEntry *entry = xnew(ConfigEntry, 1);
-                *entry = (ConfigEntry) {
+                BootEntry *entry = xnew(BootEntry, 1);
+                *entry = (BootEntry) {
                         .id = xstrdup16(u"auto-poweroff"),
                         .title = xstrdup16(u"Power Off The System"),
                         .call = poweroff_system,
@@ -2704,8 +2668,8 @@ static void config_load_all_entries(
         }
 
         if (config->auto_reboot) {
-                ConfigEntry *entry = xnew(ConfigEntry, 1);
-                *entry = (ConfigEntry) {
+                BootEntry *entry = xnew(BootEntry, 1);
+                *entry = (BootEntry) {
                         .id = xstrdup16(u"auto-reboot"),
                         .title = xstrdup16(u"Reboot The System"),
                         .call = reboot_system,
@@ -2715,10 +2679,10 @@ static void config_load_all_entries(
                 config_add_entry(config, entry);
         }
 
-        /* find if secure boot signing keys exist and autoload them if necessary
-        otherwise creates menu entries so that the user can load them manually
-        if the secure-boot-enroll variable is set to no (the default), we do not
-        even search for keys on the ESP */
+        /* Find secure boot signing keys and autoload them if configured.
+         * Otherwise, create menu entries so that the user can load them manually.
+         * If the secure-boot-enroll variable is set to no (the default), we do not
+         * even search for keys on the ESP */
         if (config->secure_boot_enroll != ENROLL_OFF)
                 secure_boot_discover_keys(config, root_dir);
 
@@ -2727,10 +2691,10 @@ static void config_load_all_entries(
 
         config_write_entries_to_variable(config);
 
-        config_title_generate(config);
+        generate_boot_entry_titles(config);
 
-        /* select entry by configured pattern or EFI LoaderDefaultEntry= variable */
-        config_default_entry_select(config);
+        /* Select entry by configured pattern or EFI LoaderDefaultEntry= variable */
+        config_select_default_entry(config);
 }
 
 static EFI_STATUS discover_root_dir(EFI_LOADED_IMAGE_PROTOCOL *loaded_image, EFI_FILE **ret_dir) {
@@ -2785,7 +2749,7 @@ static EFI_STATUS run(EFI_HANDLE image) {
                 /* Block up to 100ms to give firmware time to get input working. */
                 err = console_key_read(&key, 100 * 1000);
                 if (err == EFI_SUCCESS) {
-                        /* find matching key in config entries */
+                        /* find matching key in boot entries */
                         size_t idx = entry_lookup_key(&config, config.idx_default, KEYCHAR(key));
                         if (idx != IDX_INVALID)
                                 config.idx_default = idx;
@@ -2795,7 +2759,7 @@ static EFI_STATUS run(EFI_HANDLE image) {
         }
 
         for (;;) {
-                ConfigEntry *entry;
+                BootEntry *entry;
 
                 entry = config.entries[config.idx_default];
                 if (menu) {
@@ -2819,7 +2783,7 @@ static EFI_STATUS run(EFI_HANDLE image) {
                         continue;
                 }
 
-                (void) config_entry_bump_counters(entry);
+                (void) boot_entry_bump_counters(entry);
                 save_selected_entry(&config, entry);
 
                 /* Optionally, read a random seed off the ESP and pass it to the OS */