On 10/17/2018 12:46 PM, Heinrich Schuchardt wrote:
> 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.
Sorry I must be blind.
Heinrich
>
>> + } 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
