bootctl: additional fixes for local/global UKI PE addons

Fix various memory leaks and names used in
https://github.com/systemd/systemd/pull/28761.

src/shared/bootspec.c

@@ -43,15 +43,6 @@ static const char* const boot_entry_type_json_table[_BOOT_ENTRY_TYPE_MAX] = {
 
 DEFINE_STRING_TABLE_LOOKUP_TO_STRING(boot_entry_type_json, BootEntryType);
 
-BootEntryAddon* boot_entry_addon_free(BootEntryAddon *addon) {
-        if (!addon)
-                return NULL;
-
-        free(addon->location);
-        free(addon->cmdline);
-        return mfree(addon);
-}
-
 static void boot_entry_free(BootEntry *entry) {
         assert(entry);
 
@@ -783,7 +774,10 @@ static int find_cmdline_section(
                 char **ret_cmdline) {
 
         int r;
-        char *cmdline = NULL;
+        char *cmdline = NULL, *t = NULL;
+        _cleanup_free_ char *word = NULL;
+
+        assert(path);
 
         if (!ret_cmdline)
                 return 0;
@@ -793,7 +787,16 @@ static int find_cmdline_section(
                 *ret_cmdline = NULL;
         else if (r < 0)
                 return log_warning_errno(r, "Failed to read .cmdline section of '%s': %m", path);
-        else if (ret_cmdline)
+
+        word = strdup(cmdline);
+        if (!word)
+                return log_oom();
+
+        /* Quick test to check if there is actual content in the addon cmdline */
+        t = delete_chars(word, NULL);
+        if (t[0] == 0)
+                *ret_cmdline = NULL;
+        else
                 *ret_cmdline = TAKE_PTR(cmdline);
 
         return 0;
@@ -859,7 +862,15 @@ static int find_addon_sections(
         if (r < 0)
                 return r;
 
-        return find_cmdline_section(fd, path, sections, pe_header, ret_cmdline);
+        r = find_cmdline_section(fd, path, sections, pe_header, ret_cmdline);
+        /* If addon cmdline is empty or contains just separators,
+         * don't bother tracking it.
+         * Don't check r because it cannot return <0 if cmdline is empty,
+         * as cmdline is always optional. */
+        if (!ret_cmdline)
+                return log_warning_errno(SYNTHETIC_ERRNO(ENOENT), "Addon %s contains empty cmdline and will be therefore ignored.", path);
+
+        return r;
 }
 
 static int insert_boot_entry_addon(
@@ -867,30 +878,40 @@ static int insert_boot_entry_addon(
                 char *location,
                 char *cmdline) {
 
-        if (!GREEDY_REALLOC(addons->items, addons->count + 1))
+        if (!GREEDY_REALLOC(addons->items, addons->n_items + 1))
                 return log_oom();
 
-        addons->items[addons->count] = (BootEntryAddon) {
+        addons->items[addons->n_items] = (BootEntryAddon) {
                 .location = location,
                 .cmdline = cmdline,
         };
-        addons->count++;
+        addons->n_items++;
 
         return 0;
 }
 
+static void boot_entry_addons_done(BootEntryAddons *addons) {
+        FOREACH_ARRAY(addon, addons->items, addons->n_items) {
+                free(addon->cmdline);
+                free(addon->location);
+        }
+        addons->items = mfree(addons->items);
+        addons->n_items = 0;
+}
+
 static int boot_entries_find_unified_addons(
                 BootConfig *config,
                 int d_fd,
                 const char *addon_dir,
                 const char *root,
-                BootEntryAddons *addons) {
+                BootEntryAddons *ret_addons) {
 
         _cleanup_closedir_ DIR *d = NULL;
         _cleanup_free_ char *full = NULL;
+        _cleanup_(boot_entry_addons_done) BootEntryAddons addons = {};
         int r;
 
-        assert(addons);
+        assert(ret_addons);
         assert(config);
 
         r = chase_and_opendirat(d_fd, addon_dir, CHASE_AT_RESOLVE_IN_ROOT, &full, &d);
@@ -899,8 +920,6 @@ static int boot_entries_find_unified_addons(
         if (r < 0)
                 return log_error_errno(r, "Failed to open '%s/%s': %m", root, addon_dir);
 
-        *addons = (BootEntryAddons) {};
-
         FOREACH_DIRENT(de, d, return log_error_errno(errno, "Failed to read %s: %m", full)) {
                 _cleanup_free_ char *j = NULL, *cmdline = NULL, *location = NULL;
                 _cleanup_close_ int fd = -EBADF;
@@ -934,12 +953,15 @@ static int boot_entries_find_unified_addons(
                 if (!location)
                         return log_oom();
 
-                r = insert_boot_entry_addon(addons, TAKE_PTR(location), TAKE_PTR(cmdline));
-                if (r < 0) {
-                        free(addons);
+                r = insert_boot_entry_addon(&addons, location, cmdline);
+                if (r < 0)
                         return r;
-                }
+
+                TAKE_PTR(location);
+                TAKE_PTR(cmdline);
         }
+
+        *ret_addons = TAKE_STRUCT(addons);
         return 0;
 }
 
@@ -1439,52 +1461,74 @@ static void print_addon(
                 const char *addon_str) {
 
         printf("  %s: %s\n", addon_str, addon->location);
-        printf("      cmdline: %s%s\n", special_glyph(SPECIAL_GLYPH_TREE_RIGHT), addon->cmdline);
+        printf("      options: %s%s\n", special_glyph(SPECIAL_GLYPH_TREE_RIGHT), addon->cmdline);
+}
+
+static int indent_embedded_newlines(char *cmdline, char **ret_cmdline) {
+        _cleanup_free_ char *t = NULL;
+        _cleanup_strv_free_ char **ts = NULL;
+
+        assert(ret_cmdline);
+
+        ts = strv_split_newlines(cmdline);
+        if (!ts)
+                return -ENOMEM;
+
+        t = strv_join(ts, "\n              ");
+        if (!t)
+                return -ENOMEM;
+
+        *ret_cmdline = TAKE_PTR(t);
+
+        return 0;
 }
 
 static int print_cmdline(
                 const BootEntry *e,
                 const BootEntryAddons *global_arr) {
 
-        _cleanup_free_ char *final_cmdline = NULL;
+        _cleanup_free_ char *options = NULL, *combined_cmdline = NULL, *t2 = NULL;
 
         assert(e);
 
         if (!strv_isempty(e->options)) {
-                _cleanup_free_ char *t = NULL, *t2 = NULL;
-                _cleanup_strv_free_ char **ts = NULL;
+                _cleanup_free_ char *t = NULL;
 
-                t = strv_join(e->options, " ");
-                if (!t)
+                options = strv_join(e->options, " ");
+                if (!options)
                         return log_oom();
 
-                ts = strv_split_newlines(t);
-                if (!ts)
+                if (indent_embedded_newlines(options, &t) < 0)
                         return log_oom();
 
-                t2 = strv_join(ts, "\n              ");
+                printf("      options: %s\n", t);
+                t2 = strdup(options);
                 if (!t2)
                         return log_oom();
-
-                printf("  ukiCmdline: %s\n", t2);
-                final_cmdline = TAKE_PTR(t2);
         }
 
-        FOREACH_ARRAY(addon, global_arr->items, global_arr->count) {
-                print_addon(addon, "globalAddon");
-                if (!strextend(&final_cmdline, " ", addon->cmdline))
+        FOREACH_ARRAY(addon, global_arr->items, global_arr->n_items) {
+                print_addon(addon, "global-addon");
+                if (!strextend(&t2, " ", addon->cmdline))
                         return log_oom();
         }
 
-        FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.count) {
+        FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.n_items) {
                 /* Add space at the beginning of addon_str to align it correctly */
-                print_addon(addon, " localAddon");
-                if (!strextend(&final_cmdline, " ", addon->cmdline))
+                print_addon(addon, " local-addon");
+                if (!strextend(&t2, " ", addon->cmdline))
                         return log_oom();
         }
 
-        if (final_cmdline)
-                printf("      options: %s\n", final_cmdline);
+        /* Don't print the combined cmdline if it's same as options. */
+        if (streq_ptr(t2, options))
+                return 0;
+
+        if (indent_embedded_newlines(t2, &combined_cmdline) < 0)
+                return log_oom();
+
+        if (combined_cmdline)
+                printf("      cmdline: %s\n", combined_cmdline);
 
         return 0;
 }
@@ -1499,7 +1543,7 @@ static int json_addon(
         r = json_variant_append_arrayb(array,
                         JSON_BUILD_OBJECT(
                                 JSON_BUILD_PAIR(addon_str, JSON_BUILD_STRING(addon->location)),
-                                JSON_BUILD_PAIR("cmdline", JSON_BUILD_STRING(addon->cmdline))));
+                                JSON_BUILD_PAIR("options", JSON_BUILD_STRING(addon->cmdline))));
         if (r < 0)
                 return log_oom();
 
@@ -1509,42 +1553,41 @@ static int json_addon(
 static int json_cmdline(
                 const BootEntry *e,
                 const BootEntryAddons *global_arr,
+                const char *def_cmdline,
                 JsonVariant **v) {
 
-        _cleanup_free_ char *final_cmdline = NULL, *def_cmdline = NULL;
+        _cleanup_free_ char *combined_cmdline = NULL;
         _cleanup_(json_variant_unrefp) JsonVariant *addons_array = NULL;
         int r;
 
         assert(e);
 
-        if (!strv_isempty(e->options)) {
-                def_cmdline = strv_join(e->options, " ");
-                if (!def_cmdline)
+        if (def_cmdline) {
+                combined_cmdline = strdup(def_cmdline);
+                if (!combined_cmdline)
                         return log_oom();
-                final_cmdline = TAKE_PTR(def_cmdline);
         }
 
-        FOREACH_ARRAY(addon, global_arr->items, global_arr->count) {
+        FOREACH_ARRAY(addon, global_arr->items, global_arr->n_items) {
                 r = json_addon(addon, "globalAddon", &addons_array);
                 if (r < 0)
                         return r;
-                if (!strextend(&final_cmdline, " ", addon->cmdline))
+                if (!strextend(&combined_cmdline, " ", addon->cmdline))
                         return log_oom();
         }
 
-        FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.count) {
+        FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.n_items) {
                 r = json_addon(addon, "localAddon", &addons_array);
                 if (r < 0)
                         return r;
-                if (!strextend(&final_cmdline, " ", addon->cmdline))
+                if (!strextend(&combined_cmdline, " ", addon->cmdline))
                         return log_oom();
         }
 
         r = json_variant_merge_objectb(
                 v, JSON_BUILD_OBJECT(
-                                JSON_BUILD_PAIR_CONDITION(def_cmdline, "ukiCmdline", JSON_BUILD_STRING(def_cmdline)),
                                 JSON_BUILD_PAIR("addons", JSON_BUILD_VARIANT(addons_array)),
-                                JSON_BUILD_PAIR_CONDITION(final_cmdline, "options", JSON_BUILD_STRING(final_cmdline))));
+                                JSON_BUILD_PAIR_CONDITION(combined_cmdline, "cmdline", JSON_BUILD_STRING(combined_cmdline))));
         if (r < 0)
                 return log_oom();
         return 0;
@@ -1654,6 +1697,7 @@ int show_boot_entry(
 
 int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) {
         _cleanup_(json_variant_unrefp) JsonVariant *v = NULL;
+        _cleanup_free_ char *opts = NULL;
         const BootEntry *e;
         int r;
 
@@ -1667,6 +1711,12 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) {
 
         e = c->entries + i;
 
+        if (!strv_isempty(e->options)) {
+                opts = strv_join(e->options, " ");
+                if (!opts)
+                        return log_oom();
+        }
+
         r = json_variant_merge_objectb(
                         &v, JSON_BUILD_OBJECT(
                                         JSON_BUILD_PAIR("type", JSON_BUILD_STRING(boot_entry_type_json_to_string(e->type))),
@@ -1679,6 +1729,7 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) {
                                         JSON_BUILD_PAIR_CONDITION(e->version, "version", JSON_BUILD_STRING(e->version)),
                                         JSON_BUILD_PAIR_CONDITION(e->machine_id, "machineId", JSON_BUILD_STRING(e->machine_id)),
                                         JSON_BUILD_PAIR_CONDITION(e->architecture, "architecture", JSON_BUILD_STRING(e->architecture)),
+                                        JSON_BUILD_PAIR_CONDITION(opts, "options", JSON_BUILD_STRING(opts)),
                                         JSON_BUILD_PAIR_CONDITION(e->kernel, "linux", JSON_BUILD_STRING(e->kernel)),
                                         JSON_BUILD_PAIR_CONDITION(e->efi, "efi", JSON_BUILD_STRING(e->efi)),
                                         JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->initrd), "initrd", JSON_BUILD_STRV(e->initrd)),
@@ -1701,7 +1752,7 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) {
         if (r < 0)
                 return log_oom();
 
-        r = json_cmdline(e, &c->global_addons, &v);
+        r = json_cmdline(e, &c->global_addons, opts, &v);
         if (r < 0)
                 return log_oom();
 

src/shared/bootspec.h

@@ -27,7 +27,7 @@ typedef struct BootEntryAddon {
 
 typedef struct BootEntryAddons {
         BootEntryAddon *items;
-        size_t count;
+        size_t n_items;
 } BootEntryAddons;
 
 BootEntryAddon* boot_entry_addon_free(BootEntryAddon *t);