[PATCH v3 1/3] efi: Drop the memset() from efi_alloc()

[Heinrich Schuchardt]

On 02.09.24 00:22, Simon Glass wrote:
>  From my inspection none of the users need the memory to be zeroed. It
> is somewhat unexpected that it does so, since the name gives no clue to
> this.

Though the UEFI specification does not require it, EDK II uses
AllocateZeroPool() to implement
EFI_DEVICE_PATH_UTILITIES_PROTOCOL.CreateDeviceNode(). We should not
deviate from it.

>
> Drop the memset() so that it effectively becomes a wrapper around the
> normal EFI-pool allocator.
>
> Another option would be to drop this function and call
> efi_allocate_pool() directly, but that increase code size a little.
>
> Move the function comment to the header file like most other exported
> functions in U-Boot.

Yes, we should move all non-static EFI function descriptions to headers.
But it makes no sense to move the comments of a single function and
leave the other functions unchanged. Have a look at doc/api/efi.rst.

Best regards

Heinrich

>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v3:
> - Add new patch to drop the memset() from efi_alloc()
> - Drop patch to convert device_path allocation to use malloc()
>
>   include/efi_loader.h        | 11 ++++++++++-
>   lib/efi_loader/efi_memory.c |  9 ---------
>   2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f84852e384f..38971d01442 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -758,8 +758,17 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
>    * Return:	size in pages
>    */
>   #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT)
> -/* Allocate boot service data pool memory */
> +
> +/**
> + * efi_alloc() - allocate boot services data pool memory
> + *
> + * Allocate memory from pool with type EFI_BOOT_SERVICES_DATA
> + *
> + * @size:	number of bytes to allocate
> + * Return:	pointer to allocated memory, or NULL if out of memory
> + */
>   void *efi_alloc(size_t len);
> +
>   /* Allocate pages on the specified alignment */
>   void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align);
>   /* More specific EFI memory allocator, called by EFI payloads */
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index c6f1dd09456..50cb2f3898b 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -664,14 +664,6 @@ efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size,
>   	return r;
>   }
>
> -/**
> - * efi_alloc() - allocate boot services data pool memory
> - *
> - * Allocate memory from pool and zero it out.
> - *
> - * @size:	number of bytes to allocate
> - * Return:	pointer to allocated memory or NULL
> - */
>   void *efi_alloc(size_t size)
>   {
>   	void *buf;
> @@ -681,7 +673,6 @@ void *efi_alloc(size_t size)
>   		log_err("out of memory");
>   		return NULL;
>   	}
> -	memset(buf, 0, size);
>
>   	return buf;
>   }