[PATCH] 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 --- Kojima-san, I think this might affect your EFI HTTP boot series. I don't mind waiting for this and merging it after your series goes in (and adjust it). The changes should be trivial anyway
cmd/bootefi.c | 4 +- cmd/eficonfig.c | 7 ++- cmd/efidebug.c | 6 +- include/efi_loader.h | 5 +- lib/efi_loader/efi_bootmgr.c | 2 +- lib/efi_loader/efi_boottime.c | 2 +- lib/efi_loader/efi_device_path.c | 66 ++++++---------------- lib/efi_loader/efi_device_path_utilities.c | 2 +- 8 files changed, 32 insertions(+), 62 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33a4..eb839136bf7e 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -513,8 +513,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/cmd/eficonfig.c b/cmd/eficonfig.c index e6e8a0a488e7..470adb7eddbf 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -528,7 +528,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_ p += fp_size; *((struct efi_device_path *)p) = END;
- dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf); + dp = efi_dp_concat(dp_volume, (struct efi_device_path *)buf, false); free(buf);
return dp; @@ -1481,7 +1481,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); }
@@ -1492,7 +1493,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 201531ac19fc..a62298bf5987 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -698,8 +698,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); @@ -841,7 +841,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 e24410505f40..398cd20c7ae6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -808,8 +808,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 */ @@ -928,7 +926,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 a40762c74c83..646c7c7faaad 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -110,7 +110,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;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0b7579cb5af1..709ea07655c2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1821,7 +1821,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 ed7214f3a347..ad79b65843e9 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -272,30 +272,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. + * 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);
- if (concat) - end_size = 2 * sizeof(END); if (!dp1 && !dp2) { /* return an end node */ ret = efi_dp_dup(&END); @@ -307,14 +304,18 @@ 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); + 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); } @@ -328,37 +329,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 a07d9bab3a3c..fa211aee495e 100644 --- a/lib/efi_loader/efi_device_path_utilities.c +++ b/lib/efi_loader/efi_device_path_utilities.c @@ -77,7 +77,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)); }
/* -- 2.40.1
On 11/7/23 18:36, Ilias Apalodimas wrote:
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.
After this patch we still have efi_dp_append_instance(). The only difference to efi_dp_concat(,,true) seems only to be the type of end node used as separator.
Hence the last argument of efi_dp_contat() should be be either of:
* 0 * DEVICE_PATH_SUB_TYPE_INSTANCE_END * DEVICE_PATH_SUB_TYPE_END
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Kojima-san, I think this might affect your EFI HTTP boot series. I don't mind waiting for this and merging it after your series goes in (and adjust it). The changes should be trivial anyway
cmd/bootefi.c | 4 +- cmd/eficonfig.c | 7 ++- cmd/efidebug.c | 6 +- include/efi_loader.h | 5 +- lib/efi_loader/efi_bootmgr.c | 2 +- lib/efi_loader/efi_boottime.c | 2 +- lib/efi_loader/efi_device_path.c | 66 ++++++---------------- lib/efi_loader/efi_device_path_utilities.c | 2 +- 8 files changed, 32 insertions(+), 62 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33a4..eb839136bf7e 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -513,8 +513,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);diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index e6e8a0a488e7..470adb7eddbf 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -528,7 +528,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_ p += fp_size; *((struct efi_device_path *)p) = END;
- dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf);
dp = efi_dp_concat(dp_volume, (struct efi_device_path *)buf, false); free(buf);
return dp;
@@ -1481,7 +1481,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);@@ -1492,7 +1493,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);diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 201531ac19fc..a62298bf5987 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -698,8 +698,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);
@@ -841,7 +841,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 e24410505f40..398cd20c7ae6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -808,8 +808,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);@@ -928,7 +926,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);diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a40762c74c83..646c7c7faaad 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -110,7 +110,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);diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0b7579cb5af1..709ea07655c2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1821,7 +1821,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);diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ed7214f3a347..ad79b65843e9 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -272,30 +272,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.
This description is unclear. Do you mean DEVICE_PATH_SUB_TYPE_END or DEVICE_PATH_SUB_TYPE_INSTANCE_END?
We would better make the parameter a three-state value:
* 0 * DEVICE_PATH_SUB_TYPE_INSTANCE_END * 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)
This assignment is not needed if (!dp1 || !dp2).
- if (concat)
end_size = 2 * sizeof(END);@@ -307,14 +304,18 @@ 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);
We assign end_size twice in this case. We should instead move the first assignment to the if:
if (!split_end_node) end_size = sizeof(END); else end_size = 2 * sizeof(END);
Best regards
Heinrich
p = efi_alloc(sz1 + sz2 + end_size);
if (concat) {
if (split_end_node) { memcpy(p, &END, sizeof(END)); p += sizeof(END);@@ -328,37 +329,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 a07d9bab3a3c..fa211aee495e 100644 --- a/lib/efi_loader/efi_device_path_utilities.c +++ b/lib/efi_loader/efi_device_path_utilities.c @@ -77,7 +77,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)); }
/*
-- 2.40.1
Hi Heinrich
On Mon, 13 Nov 2023 at 09:37, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/7/23 18:36, Ilias Apalodimas wrote:
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.
After this patch we still have efi_dp_append_instance(). The only difference to efi_dp_concat(,,true) seems only to be the type of end node used as separator.
Hence the last argument of efi_dp_contat() should be be either of:
- 0
- DEVICE_PATH_SUB_TYPE_INSTANCE_END
- DEVICE_PATH_SUB_TYPE_END
ah fair enough, I'll clean that up as well. Add an enum as a 3rd argument then?
rd_dp) + sizeof(END);
} else { final_dp = efi_dp_dup(dp);diff --git a/cmd/efidebug.c b/cmd/efidebug.c
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 e24410505f40..398cd20c7ae6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -808,8 +808,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);@@ -928,7 +926,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);diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a40762c74c83..646c7c7faaad 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -110,7 +110,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;diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0b7579cb5af1..709ea07655c2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1821,7 +1821,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 ed7214f3a347..ad79b65843e9 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -272,30 +272,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.This description is unclear. Do you mean DEVICE_PATH_SUB_TYPE_END or DEVICE_PATH_SUB_TYPE_INSTANCE_END?
I think it's DEVICE_PATH_SUB_TYPE_END, I'll double check
We would better make the parameter a three-state value:
- 0
- DEVICE_PATH_SUB_TYPE_INSTANCE_END
- DEVICE_PATH_SUB_TYPE_END
Ok i'll add an enum
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)This assignment is not needed if (!dp1 || !dp2).
if (concat)end_size = 2 * sizeof(END); if (!dp1 && !dp2) { /* return an end node */ ret = efi_dp_dup(&END);@@ -307,14 +304,18 @@ 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);We assign end_size twice in this case. We should instead move the first assignment to the if:
if (!split_end_node) end_size = sizeof(END); else end_size = 2 * sizeof(END);
[...]
Thanks /Ilias
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas