[PATCH v3 1/3] efi_loader: Add guidcpy function

[Heinrich Schuchardt]

On 12/27/19 3:26 PM, Sughosh Ganu wrote:
> Add guidcpy function to copy the source guid to the destination
> guid. Use this function instead of memcpy for copying to the
> destination guid.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>   include/efi_loader.h          | 5 +++++
>   lib/efi_loader/efi_boottime.c | 4 ++--
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 16a1b25..bec7873 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -17,6 +17,11 @@ static inline int guidcmp(const void *g1, const void *g2)
>   	return memcmp(g1, g2, sizeof(efi_guid_t));
>   }
>
> +static inline void *guidcpy(efi_guid_t *dst, const efi_guid_t *src)
> +{
> +	return memcpy(dst, src, sizeof(*dst));
> +}
> +
>   /* No need for efi loader support in SPL */
>   #if CONFIG_IS_ENABLED(EFI_LOADER)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 88a7604..3103a50 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1401,7 +1401,7 @@ static efi_status_t EFIAPI efi_register_protocol_notify(
>   	}
>
>   	item->event = event;
> -	memcpy(&item->protocol, protocol, sizeof(efi_guid_t));
> +	guidcpy(&item->protocol, protocol);

This gives me an error with GCC 9:

lib/efi_loader/efi_boottime.c: In function
‘efi_install_configuration_table’:
lib/efi_loader/efi_boottime.c:1635:10: error: taking address of packed
member of ‘struct efi_configuration_table’ may result in an unaligned
pointer value [-Werror=address-of-packed-member]
  1635 |  guidcpy(&systab.tables[i].guid, guid);
       |          ^~~~~~~~~~~~~~~~~~~~~~

efi_configuration_table is naturally packed. There is not reason to have
a __packed attribute. I will provide a patch to remove the __packed
attribute.

So nothing to change in your patch.

Best regards

Heinrich

>   	INIT_LIST_HEAD(&item->handles);
>
>   	list_add_tail(&item->link, &efi_register_notify_events);
> @@ -1632,7 +1632,7 @@ efi_status_t efi_install_configuration_table(const efi_guid_t *guid,
>   		return EFI_OUT_OF_RESOURCES;
>
>   	/* Add a new entry */
> -	memcpy(&systab.tables[i].guid, guid, sizeof(*guid));
> +	guidcpy(&systab.tables[i].guid, guid);
>   	systab.tables[i].table = table;
>   	systab.nr_tables = i + 1;
>
>