efi_loader: Clean up efi_dp_append and efi_dp_concat

Looking back at the initrd storing functionality, we introduced three
functions, efi_dp_append_or_concatenate(), efi_dp_append/concat(). In
hindsight we could have simplified that by a lot. First of all none of
the functions append anything. They all allocate a new device path and
concatenate the contents of two device paths in one. A boolean parameter
controls the final device path -- if that's true an end node is injected
between the two device paths.

So let's rewrite this and make it a bit easier to read. Get rid of
efi_dp_append(), efi_dp_concat() and rename
efi_dp_append_or_concatenate() to efi_dp_concat(). This is far more
intuitive and the only adjustment that is needed is an extra boolean
argument on all callsites.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 34a59cb15d0df94bee204fb773f004a42727b0a0..8234e602b8fbe1384a03a1cb832ad7d13426af3a 100644 (file)
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -531,7 +531,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_
        dp = efi_dp_shorten(dp_volume);
        if (!dp)
                dp = dp_volume;
-       dp = efi_dp_append(dp, &fp->dp);
+       dp = efi_dp_concat(dp, &fp->dp, false);
        free(buf);
 
        return dp;
@@ -1484,7 +1484,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
                        ret = EFI_OUT_OF_RESOURCES;
                        goto out;
                }
-               initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp, dp);
+               initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp,
+                                         dp, false);
                efi_free_pool(dp);
        }
 
@@ -1495,7 +1496,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
        }
        final_dp_size = efi_dp_size(dp) + sizeof(END);
        if (initrd_dp) {
-               final_dp = efi_dp_concat(dp, initrd_dp);
+               final_dp = efi_dp_concat(dp, initrd_dp, true);
                final_dp_size += efi_dp_size(initrd_dp) + sizeof(END);
        } else {
                final_dp = efi_dp_dup(dp);

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index e10fbf891a42967126e091eabeedbf738681e90a..836014ab8eabc031c18ef80191a3d1f9596ccc93 100644 (file)
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -699,8 +699,8 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part,
        if (!short_fp)
                short_fp = tmp_fp;
 
-       initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,
-                                 short_fp);
+       initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp,
+                                 short_fp, false);
 
 out:
        efi_free_pool(tmp_dp);
@@ -916,7 +916,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
                goto out;
        }
 
-       final_fp = efi_dp_concat(file_path, initrd_dp);
+       final_fp = efi_dp_concat(file_path, initrd_dp, true);
        if (!final_fp) {
                printf("Cannot create final device path\n");
                r = CMD_RET_FAILURE;

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 34e7fbbf184087a0b04ce6b1357375a275eb9c7e..d3725041b20da91613cd0ffed5ecb8e9a39553e3 100644 (file)
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -819,8 +819,6 @@ efi_uintn_t efi_dp_instance_size(const struct efi_device_path *dp);
 /* size of multi-instance device path excluding end node */
 efi_uintn_t efi_dp_size(const struct efi_device_path *dp);
 struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp);
-struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
-                                     const struct efi_device_path *dp2);
 struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
                                           const struct efi_device_path *node);
 /* Create a device path node of given type, sub-type, length */
@@ -937,7 +935,8 @@ struct efi_load_option {
 struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
                                       const efi_guid_t *guid);
 struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
-                                     const struct efi_device_path *dp2);
+                                     const struct efi_device_path *dp2,
+                                     bool split_end_node);
 struct efi_device_path *search_gpt_dp_node(struct efi_device_path *device_path);
 efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
                                         efi_uintn_t *size);

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a032d3ae04e255cd14a67fc2a5198b9d2f73cacb..dd2dd23c7b5aeadb9bbaf88c2f1035e1ef4b89a1 100644 (file)
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -143,7 +143,7 @@ static efi_status_t try_load_from_file_path(efi_handle_t *fs_handles,
                if (!dp)
                        continue;
 
-               dp = efi_dp_append(dp, fp);
+               dp = efi_dp_concat(dp, fp, false);
                if (!dp)
                        continue;
 
@@ -1579,8 +1579,8 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
                        goto out;
                msg_path = file_path;
        } else {
-               file_path = efi_dp_append(bootefi_device_path,
-                                         bootefi_image_path);
+               file_path = efi_dp_concat(bootefi_device_path,
+                                         bootefi_image_path, false);
                msg_path = bootefi_image_path;
                log_debug("Loaded from disk\n");
        }

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c579d89211be9cccb6095a0a1ea98ab6e5d3b0bb..1951291747cd5f245d7f73ccafe2083af2888dac 100644 (file)
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1816,7 +1816,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
        if (device_path) {
                info->device_handle = efi_dp_find_obj(device_path, NULL, NULL);
 
-               dp = efi_dp_append(device_path, file_path);
+               dp = efi_dp_concat(device_path, file_path, false);
                if (!dp) {
                        ret = EFI_OUT_OF_RESOURCES;
                        goto failure;

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 8dbd8105ae26dd3dec5e7b5bffc6cd8ac61f326d..500fa10d405fd504c1bf34e36bfd7037bdc464f9 100644 (file)
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -271,30 +271,27 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp)
 }
 
 /**
- * efi_dp_append_or_concatenate() - Append or concatenate two device paths.
- *                                 Concatenated device path will be separated
- *                                 by a sub-type 0xff end node
+ * efi_dp_concat() - Concatenate two device paths and add and terminate them
+ *                   with an end node.
  *
- * @dp1:       First device path
- * @dp2:       Second device path
- * @concat:    If true the two device paths will be concatenated and separated
- *             by an end of entrire device path sub-type 0xff end node.
- *             If true the second device path will be appended to the first and
- *             terminated by an end node
+ * @dp1:           First device path
+ * @dp2:           Second device path
+ * @split_end_node: If true the two device paths will be concatenated and
+ *                  separated by an end node (DEVICE_PATH_SUB_TYPE_END).
+ *                 If false the second device path will be concatenated to the
+ *                 first one as-is.
  *
  * Return:
  * concatenated device path or NULL. Caller must free the returned value
  */
-static struct
-efi_device_path *efi_dp_append_or_concatenate(const struct efi_device_path *dp1,
-                                             const struct efi_device_path *dp2,
-                                             bool concat)
+struct
+efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+                              const struct efi_device_path *dp2,
+                              bool split_end_node)
 {
        struct efi_device_path *ret;
-       size_t end_size = sizeof(END);
+       size_t end_size;
 
-       if (concat)
-               end_size = 2 * sizeof(END);
        if (!dp1 && !dp2) {
                /* return an end node */
                ret = efi_dp_dup(&END);
@@ -306,14 +303,20 @@ efi_device_path *efi_dp_append_or_concatenate(const struct efi_device_path *dp1,
                /* both dp1 and dp2 are non-null */
                unsigned sz1 = efi_dp_size(dp1);
                unsigned sz2 = efi_dp_size(dp2);
-               void *p = efi_alloc(sz1 + sz2 + end_size);
+               void *p;
+
+               if (split_end_node)
+                       end_size = 2 * sizeof(END);
+               else
+                       end_size = sizeof(END);
+               p = efi_alloc(sz1 + sz2 + end_size);
                if (!p)
                        return NULL;
                ret = p;
                memcpy(p, dp1, sz1);
                p += sz1;
 
-               if (concat) {
+               if (split_end_node) {
                        memcpy(p, &END, sizeof(END));
                        p += sizeof(END);
                }
@@ -327,37 +330,6 @@ efi_device_path *efi_dp_append_or_concatenate(const struct efi_device_path *dp1,
        return ret;
 }
 
-/**
- * efi_dp_append() - Append a device to an existing device path.
- *
- * @dp1:       First device path
- * @dp2:       Second device path
- *
- * Return:
- * concatenated device path or NULL. Caller must free the returned value
- */
-struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
-                                     const struct efi_device_path *dp2)
-{
-       return efi_dp_append_or_concatenate(dp1, dp2, false);
-}
-
-/**
- * efi_dp_concat() - Concatenate 2 device paths. The final device path will
- *                   contain two device paths separated by and end node (0xff).
- *
- * @dp1:       First device path
- * @dp2:       Second device path
- *
- * Return:
- * concatenated device path or NULL. Caller must free the returned value
- */
-struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
-                                     const struct efi_device_path *dp2)
-{
-       return efi_dp_append_or_concatenate(dp1, dp2, true);
-}
-
 struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
                                           const struct efi_device_path *node)
 {

diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
index 844d8acd67c0d659da7c7eb86bac712a85f9c4fc..c95dbfa9b5f6ef6cddbd9a5094e9d32cce138c17 100644 (file)
--- a/lib/efi_loader/efi_device_path_utilities.c
+++ b/lib/efi_loader/efi_device_path_utilities.c
@@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path(
        const struct efi_device_path *src2)
 {
        EFI_ENTRY("%pD, %pD", src1, src2);
-       return EFI_EXIT(efi_dp_append(src1, src2));
+       return EFI_EXIT(efi_dp_concat(src1, src2, false));
 }
 
 /*