On 10/17/2018 09:32 AM, AKASHI Takahiro wrote:
> Factor out efi_set_bootdev() and extract efi_dp_from_name().
> This function will be used to set a boot device in efishell command.
>
> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> ---
> cmd/bootefi.c | 42 ++++++----------------------
> include/efi_loader.h | 4 +++
> lib/efi_loader/efi_device_path.c | 47 ++++++++++++++++++++++++++++++++
> 3 files changed, 59 insertions(+), 34 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 99f5b2b95706..b3e2845120fe 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -647,45 +647,19 @@ U_BOOT_CMD(
>
> void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
> {
> - char filename[32] = { 0 }; /* dp->str is u16[32] long */
> - char *s;
> + struct efi_device_path *device, *image;
> + efi_status_t ret;
>
> /* efi_set_bootdev is typically called repeatedly, recover memory */
> efi_free_pool(bootefi_device_path);
> efi_free_pool(bootefi_image_path);
> - /* If blk_get_device_part_str fails, avoid duplicate free. */
> - bootefi_device_path = NULL;
> - bootefi_image_path = NULL;
> -
> - if (strcmp(dev, "Net")) {
> - struct blk_desc *desc;
> - disk_partition_t fs_partition;
> - int part;
> -
> - part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
> - 1);
> - if (part < 0)
> - return;
> -
> - bootefi_device_path = efi_dp_from_part(desc, part);
> - } else {
> -#ifdef CONFIG_NET
> - bootefi_device_path = efi_dp_from_eth();
> -#endif
> - }
> -
> - if (!path)
> - return;
>
> - if (strcmp(dev, "Net")) {
> - /* Add leading / to fs paths, because they're absolute */
> - snprintf(filename, sizeof(filename), "/%s", path);
> + ret = efi_dp_from_name(dev, devnr, path, &device, &image);
> + if (ret == EFI_SUCCESS) {
> + bootefi_device_path = device;
> + bootefi_image_path = image;
> } else {
> - snprintf(filename, sizeof(filename), "%s", path);
> + bootefi_device_path = NULL;
> + bootefi_image_path = NULL;
> }
> - /* DOS style file path: */
> - s = filename;
> - while ((s = strchr(s, '/')))
> - *s++ = '\\';
> - bootefi_image_path = efi_dp_from_file(NULL, 0, filename);
> }
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 74df070316a0..b73fbb6de23f 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -425,6 +425,10 @@ const struct efi_device_path *efi_dp_last_node(
> efi_status_t efi_dp_split_file_path(struct efi_device_path *full_path,
> struct efi_device_path **device_path,
> struct efi_device_path **file_path);
> +efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
> + const char *path,
> + struct efi_device_path **device,
> + struct efi_device_path **file);
>
> #define EFI_DP_TYPE(_dp, _type, _subtype) \
> (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index 5a61a1c1dcf9..3af444147283 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -942,3 +942,50 @@ efi_status_t efi_dp_split_file_path(struct
> efi_device_path *full_path,
> *file_path = fp;
> return EFI_SUCCESS;
> }
> +
> +efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
> + const char *path,
> + struct efi_device_path **device,
> + struct efi_device_path **file)
> +{
> + int is_net;
> + struct blk_desc *desc = NULL;
> + disk_partition_t fs_partition;
> + int part = 0;
> + char filename[32] = { 0 }; /* dp->str is u16[32] long */
> + char *s;
> +
> + if (!device || (path && !file))
> + return EFI_INVALID_PARAMETER;
> +
> + is_net = !strcmp(dev, "Net");
> + if (!is_net) {
> + part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
> + 1);
> + if (part < 0)
> + return EFI_INVALID_PARAMETER;
> +
> + *device = efi_dp_from_part(desc, part);
> + } else {
> +#ifdef CONFIG_NET
> + *device = efi_dp_from_eth();
> +#endif
> + }
> +
> + if (!path)
> + return EFI_SUCCESS;
> +
> + if (!is_net) {
> + /* Add leading / to fs paths, because they're absolute */
> + snprintf(filename, sizeof(filename), "/%s", path);
This coding creates undefined behavior. You have to use two different
variables.
> + } else {
> + snprintf(filename, sizeof(filename), "%s", path);
Same here.
@Alex:
Please, remove the patch from efi-next until it is fixed.
Best regards
Heinrich
> + }
> + /* DOS style file path: */
> + s = filename;
> + while ((s = strchr(s, '/')))
> + *s++ = '\\';
> + *file = efi_dp_from_file(NULL, 0, filename);
> +
> + return EFI_SUCCESS;
> +}
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
