Re: [RFC 03/14] efi_loader: simplify efi_dp_concat()
On Fri, 26 Apr 2024 at 17:13, Heinrich Schuchardt 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 > --- > 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, >dp, false); > + dp = efi_dp_concat(dp, >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 > *)_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, _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 *)_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, _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
[RFC 03/14] efi_loader: simplify efi_dp_concat()
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. Signed-off-by: Heinrich Schuchardt --- 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, >dp, false); + dp = efi_dp_concat(dp, >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 *)_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, _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 *)_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, _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, -