bootctl: unify boot entry loading for "status" and "list"

We must be consistent in the two listings, so let's split out the loading code
and call it from both verb_status() and verb_list(). This effectively makes
verb_status() also call efi_loader_get_entries().

There is still some code duplicated, but that's hard to avoid. Error messages
are made identical for the same errors in various places.

Fixes #22580.

diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c
index c4cb33c705674cc3f72c5ec2533dac7625b663f5..a393370f8d413021b5e2122ac2480458fd6e0f25 100644 (file)
--- a/src/boot/bootctl.c
+++ b/src/boot/bootctl.c
@@ -583,6 +583,35 @@ static const char* const boot_entry_type_table[_BOOT_ENTRY_TYPE_MAX] = {
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(boot_entry_type, BootEntryType);
 
+static int boot_config_load_and_select(
+                BootConfig *config,
+                const char *esp_path,
+                dev_t esp_devid,
+                const char *xbootldr_path,
+                dev_t xbootldr_devid) {
+
+        _cleanup_strv_free_ char **efi_entries = NULL;
+        int r;
+
+        /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would
+         * find the same entries twice. */
+        bool same = esp_path && xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid);
+
+        r = boot_config_load(config, esp_path, same ? NULL : xbootldr_path);
+        if (r < 0)
+                return r;
+
+        r = efi_loader_get_entries(&efi_entries);
+        if (r == -ENOENT || ERRNO_IS_NOT_SUPPORTED(r))
+                log_debug_errno(r, "Boot loader reported no entries.");
+        else if (r < 0)
+                log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m");
+        else
+                (void) boot_config_augment_from_loader(config, efi_entries, /* only_auto= */ false);
+
+        return boot_config_select_special_entries(config);
+}
+
 static int boot_entry_show(
                 const BootEntry *e,
                 bool show_as_default,
@@ -682,16 +711,17 @@ static int boot_entry_show(
 }
 
 static int status_entries(
+                const BootConfig *config,
                 const char *esp_path,
                 sd_id128_t esp_partition_uuid,
                 const char *xbootldr_path,
                 sd_id128_t xbootldr_partition_uuid) {
 
-        _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL;
         sd_id128_t dollar_boot_partition_uuid;
         const char *dollar_boot_path;
         int r;
 
+        assert(config);
         assert(esp_path || xbootldr_path);
 
         if (xbootldr_path) {
@@ -709,21 +739,13 @@ static int status_entries(
                        SD_ID128_FORMAT_VAL(dollar_boot_partition_uuid));
         printf("\n\n");
 
-        r = boot_config_load(&config, esp_path, xbootldr_path);
-        if (r < 0)
-                return r;
-
-        r = boot_config_select_special_entries(&config);
-        if (r < 0)
-                return r;
-
-        if (config.default_entry < 0)
-                printf("%zu entries, no entry could be determined as default.\n", config.n_entries);
+        if (config->default_entry < 0)
+                printf("%zu entries, no entry could be determined as default.\n", config->n_entries);
         else {
                 printf("Default Boot Loader Entry:\n");
 
                 r = boot_entry_show(
-                                boot_config_default_entry(&config),
+                                boot_config_default_entry(config),
                                 /* show_as_default= */ false,
                                 /* show_as_selected= */ false,
                                 /* show_discovered= */ false);
@@ -1641,7 +1663,7 @@ static int verb_status(int argc, char *argv[], void *userdata) {
         r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid, &xbootldr_devid);
         if (arg_print_dollar_boot_path) {
                 if (r == -EACCES)
-                        return log_error_errno(r, "Failed to determine XBOOTLDR location: %m");
+                        return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m");
                 if (r < 0)
                         return r;
 
@@ -1772,16 +1794,20 @@ static int verb_status(int argc, char *argv[], void *userdata) {
         }
 
         if (arg_esp_path || arg_xbootldr_path) {
-                /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */
-                bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid);
-
-                k = status_entries(
-                                arg_esp_path,
-                                esp_uuid,
-                                same ? NULL : arg_xbootldr_path,
-                                same ? SD_ID128_NULL : xbootldr_uuid);
+                _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL;
+
+                k = boot_config_load_and_select(&config,
+                                                arg_esp_path, esp_devid,
+                                                arg_xbootldr_path, xbootldr_devid);
                 if (k < 0)
                         r = k;
+                else {
+                        k = status_entries(&config,
+                                           arg_esp_path, esp_uuid,
+                                           arg_xbootldr_path, xbootldr_uuid);
+                        if (k < 0)
+                                r = k;
+                }
         }
 
         return r;
@@ -1789,17 +1815,17 @@ static int verb_status(int argc, char *argv[], void *userdata) {
 
 static int verb_list(int argc, char *argv[], void *userdata) {
         _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL;
-        _cleanup_strv_free_ char **efi_entries = NULL;
         dev_t esp_devid = 0, xbootldr_devid = 0;
         int r;
 
-        /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two things: turn
-         * off logging about access errors and turn off potentially privileged device probing. Here we're interested in
-         * the latter but not the former, hence request the mode, and log about EACCES. */
+        /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two
+         * things: turn off logging about access errors and turn off potentially privileged device probing.
+         * Here we're interested in the latter but not the former, hence request the mode, and log about
+         * EACCES. */
 
         r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid);
         if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */
-                return log_error_errno(r, "Failed to determine ESP: %m");
+                return log_error_errno(r, "Failed to determine ESP location: %m");
         if (r < 0)
                 return r;
 
@@ -1809,22 +1835,7 @@ static int verb_list(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return r;
 
-        /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */
-        bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid);
-
-        r = boot_config_load(&config, arg_esp_path, same ? NULL : arg_xbootldr_path);
-        if (r < 0)
-                return r;
-
-        r = efi_loader_get_entries(&efi_entries);
-        if (r == -ENOENT || ERRNO_IS_NOT_SUPPORTED(r))
-                log_debug_errno(r, "Boot loader reported no entries.");
-        else if (r < 0)
-                log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m");
-        else
-                (void) boot_config_augment_from_loader(&config, efi_entries, /* only_auto= */ false);
-
-        r = boot_config_select_special_entries(&config);
+        r = boot_config_load_and_select(&config, arg_esp_path, esp_devid, arg_xbootldr_path, xbootldr_devid);
         if (r < 0)
                 return r;
 

diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h
index 99bc5f69a4916886547c867e94c4469d3de5792e..93fb22e9d406c972ed2f75d690a9eb0f3adb9a8d 100644 (file)
--- a/src/shared/bootspec.h
+++ b/src/shared/bootspec.h
@@ -71,7 +71,7 @@ typedef struct BootConfig {
 
 BootEntry* boot_config_find_entry(BootConfig *config, const char *id);
 
-static inline BootEntry* boot_config_default_entry(BootConfig *config) {
+static inline const BootEntry* boot_config_default_entry(const BootConfig *config) {
         assert(config);
 
         if (config->default_entry < 0)