On Fri, 26 Apr 2024 at 17:13, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
>
> As we now have efi_dp_merge() we can use this function to replace
> efi_dp_concat(,,true) and remove the last parameter from efi_dp_concat()
> otherwise.
>
This patch looks correct, but I prefer keeping the existing efi_dp_concat as-is
Thanks
/Ilias
> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> ---
> cmd/eficonfig.c | 22 +++++++++---------
> cmd/efidebug.c | 17 +++++++++-----
> include/efi_loader.h | 3 +--
> lib/efi_loader/efi_bootbin.c | 2 +-
> lib/efi_loader/efi_bootmgr.c | 2 +-
> lib/efi_loader/efi_boottime.c | 2 +-
> lib/efi_loader/efi_device_path.c | 26 ++++------------------
> lib/efi_loader/efi_device_path_utilities.c | 2 +-
> 8 files changed, 31 insertions(+), 45 deletions(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 0ba92c60e03..1c57e66040b 100644
> --- 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_concat(dp, &fp->dp, false);
> + dp = efi_dp_concat(dp, &fp->dp);
> free(buf);
>
> return dp;
> @@ -1485,7 +1485,7 @@ static efi_status_t eficonfig_edit_boot_option(u16
> *varname, struct eficonfig_bo
> goto out;
> }
> initrd_dp = efi_dp_concat((const struct efi_device_path
> *)&id_dp,
> - dp, false);
> + dp);
> efi_free_pool(dp);
> }
>
> @@ -1494,17 +1494,17 @@ static efi_status_t eficonfig_edit_boot_option(u16
> *varname, struct eficonfig_bo
> ret = EFI_OUT_OF_RESOURCES;
> goto out;
> }
> - final_dp_size = efi_dp_size(dp) + sizeof(END);
> +
> + final_dp = dp;
> + final_dp_size = efi_dp_size(final_dp) + sizeof(END);
> +
> if (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);
> + final_dp = efi_dp_merge(final_dp, &final_dp_size, initrd_dp);
> + if (!final_dp) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> }
> - efi_free_pool(dp);
> -
> - if (!final_dp)
> - goto out;
>
> if (utf16_utf8_strlen(bo->optional_data)) {
> len = utf16_utf8_strlen(bo->optional_data) + 1;
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index a587860e2a5..93ba16efc7d 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -697,7 +697,7 @@ struct efi_device_path *create_initrd_dp(const char *dev,
> const char *part,
> short_fp = tmp_fp;
>
> initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp,
> - short_fp, false);
> + short_fp);
>
> out:
> efi_free_pool(tmp_dp);
> @@ -917,11 +917,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int
> flag,
> goto out;
> }
>
> - final_fp = efi_dp_concat(file_path, initrd_dp, true);
> - if (!final_fp) {
> - printf("Cannot create final device path\n");
> - r = CMD_RET_FAILURE;
> - goto out;
> + final_fp = file_path;
> + fp_size = efi_dp_size(final_fp) + sizeof(END);
> +
> + if (initrd_dp) {
> + final_fp = efi_dp_merge(final_fp, &fp_size, initrd_dp);
> + if (!final_fp) {
> + printf("Cannot create final device path\n");
> + r = CMD_RET_FAILURE;
> + goto out;
> + }
> }
>
> lo.file_path = final_fp;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index a7d7b8324f1..0ac04990b8e 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -948,8 +948,7 @@ struct efi_device_path *efi_dp_merge(const struct
> efi_device_path *dp1,
> efi_uintn_t *size,
> const struct efi_device_path *dp2);
> struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> - const struct efi_device_path *dp2,
> - bool split_end_node);
> + const struct efi_device_path *dp2);
> 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_bootbin.c b/lib/efi_loader/efi_bootbin.c
> index b7910f78fb6..745b941b6ca 100644
> --- a/lib/efi_loader/efi_bootbin.c
> +++ b/lib/efi_loader/efi_bootbin.c
> @@ -150,7 +150,7 @@ efi_status_t efi_run_image(void *source_buffer,
> efi_uintn_t source_size)
> msg_path = file_path;
> } else {
> file_path = efi_dp_concat(bootefi_device_path,
> - bootefi_image_path, false);
> + bootefi_image_path);
> msg_path = bootefi_image_path;
> log_debug("Loaded from disk\n");
> }
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 4ac519228a6..db394df6bf4 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -130,7 +130,7 @@ static efi_status_t try_load_from_file_path(efi_handle_t
> *fs_handles,
> if (!dp)
> continue;
>
> - dp = efi_dp_concat(dp, fp, false);
> + dp = efi_dp_concat(dp, fp);
> if (!dp)
> continue;
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 1951291747c..33f03c0cb0f 100644
> --- 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_concat(device_path, file_path, false);
> + dp = efi_dp_concat(device_path, file_path);
> 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 16cbe41d32f..75fe95c9c1e 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -307,21 +307,15 @@ struct efi_device_path *efi_dp_merge(const struct
> efi_device_path *dp1,
> *
> * @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
> */
> struct
> efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> - const struct efi_device_path *dp2,
> - bool split_end_node)
> + const struct efi_device_path *dp2)
> {
> struct efi_device_path *ret;
> - size_t end_size;
>
> if (!dp1 && !dp2) {
> /* return an end node */
> @@ -333,29 +327,17 @@ efi_device_path *efi_dp_concat(const struct
> efi_device_path *dp1,
> } else {
> /* both dp1 and dp2 are non-null */
> unsigned sz1 = efi_dp_size(dp1);
> - unsigned sz2 = efi_dp_size(dp2);
> + /* the end node of the second device path has to be retained
> */
> + unsigned int sz2 = efi_dp_size(dp2) + sizeof(END);
> void *p;
>
> - if (split_end_node)
> - end_size = 2 * sizeof(END);
> - else
> - end_size = sizeof(END);
> - p = efi_alloc(sz1 + sz2 + end_size);
> + p = efi_alloc(sz1 + sz2);
> if (!p)
> return NULL;
> ret = p;
> memcpy(p, dp1, sz1);
> p += sz1;
> -
> - if (split_end_node) {
> - memcpy(p, &END, sizeof(END));
> - p += sizeof(END);
> - }
> -
> - /* the end node of the second device path has to be retained
> */
> memcpy(p, dp2, sz2);
> - p += sz2;
> - memcpy(p, &END, sizeof(END));
> }
>
> return ret;
> diff --git a/lib/efi_loader/efi_device_path_utilities.c
> b/lib/efi_loader/efi_device_path_utilities.c
> index c95dbfa9b5f..5f06602dba6 100644
> --- 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_concat(src1, src2, false));
> + return EFI_EXIT(efi_dp_concat(src1, src2));
> }
>
> /*
> --
> 2.43.0
>
