Re: [RFC PATCH 2/2] cmd: replace efi_create_handle/add_protocol with InstallMultipleProtocol

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Oct 05, 2022 at 06:26:02PM +0300, Ilias Apalodimas wrote:
> In general handles should only be deleted if the last remaining protocol
> is removed.  Instead of explicitly calling
> efi_create_handle -> efi_add_protocol -> efi_delete_handle which blindly
> removes all protocols from a handle before removing it,  use
> InstallMultiple/UninstallMultiple which adheres to the EFI spec and only
> deletes a handle if there are no additional protocols present
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  cmd/bootefi.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 3041873afbbc..4ab68868cc7e 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -509,12 +509,9 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
>  		 * Make sure that device for device_path exist
>  		 * in load_image(). Otherwise, shell and grub will fail.
>  		 */
> -		ret = efi_create_handle(&mem_handle);
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -
> -		ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
> -				       file_path);
> +		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
> +							       &efi_guid_device_path,
> +							       file_path, NULL);

nitpick: NULL -> NULL, NULL
UEFI spec seems to require "A variable argument list containing pairs of
protocol GUIDs and protocol interfaces" even if a protocol interface won't be
used with GUID as NULL.

-Takahiro Akashi

>  		if (ret != EFI_SUCCESS)
>  			goto out;
>  		msg_path = file_path;
> @@ -542,7 +539,9 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
>  	ret = do_bootefi_exec(handle, load_options);
>  
>  out:
> -	efi_delete_handle(mem_handle);
> +	efi_uninstall_multiple_protocol_interfaces(mem_handle,
> +						   &efi_guid_device_path,
> +						   file_path, NULL);
>  	efi_free_pool(file_path);
>  	return ret;
>  }
> -- 
> 2.34.1
>