Re: [PATCH v4 3/3] Load option with short device path for boot vars

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Raymond,

On Tue, May 23, 2023 at 12:18:22PM -0700, Raymond Mao wrote:
> The boot variables automatically generated for removable medias
> should be with short form of device path without device nodes.
> This is a requirement for the case that a removable media is
> plugged into a different port but is still able to work with the
> existing boot variables.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
> Changes in v2
> - Ignore EFI_NOT_FOUND returned from
>   efi_bootmgr_update_media_device_boot_option which means no boot
>   options scanned.
> Changes in v3
> - Split the patch into moving and renaming functions and
>   individual patches for each changed functionality
> Changes in v4
> - Revert the change of introducing a bool parameter when updating
>   the boot option. Use short-form of device path by default
>
>  lib/efi_loader/efi_bootmgr.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index c329428973..336b9d2d8f 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -387,6 +387,11 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
>  		p = dev_name;
>  		utf8_utf16_strncpy(&p, buf, strlen(buf));
>
> +		/* use short form device path */
> +		device_path = efi_dp_shorten(device_path);
> +		if (!device_path)

One nit here.  I think we should print a warning if shortening the path
fails.  Heinrich are you ok with this?  I think not falling back and adding
the long form DP is fine.

Thanks
/Ilias
> +			continue;
> +
>  		lo.label = dev_name;
>  		lo.attributes = LOAD_OPTION_ACTIVE;
>  		lo.file_path = device_path;
> --
> 2.25.1
>