[U-Boot] [PATCH 2/9] efi_selftest: test protocol management

[Simon Glass]

Hi Heinrich,

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> This unit test checks the following protocol services:
> InstallProtocolInterface, UninstallProtocolInterface,
> InstallMultipleProtocolsInterfaces,
> UninstallMultipleProtocolsInterfaces,
> HandleProtocol, ProtocolsPerHandle,
> LocateHandle, LocateHandleBuffer.
>
> As UninstallProtocolInterface and UninstallMultipleProtocolsInterfaces
> are not completely implemented a TODO message will shown for
> their failure.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  include/efi_selftest.h                          |   9 +
>  lib/efi_selftest/Makefile                       |   3 +
>  lib/efi_selftest/efi_selftest_manageprotocols.c | 381 ++++++++++++++++++++++++
>  3 files changed, 393 insertions(+)
>  create mode 100644 lib/efi_selftest/efi_selftest_manageprotocols.c
>

Reviewed-by: Simon Glass <sjg at chromium.org>

nits below

> diff --git a/include/efi_selftest.h b/include/efi_selftest.h
> index 5cc8d4f600..be5ba4bfa9 100644
> --- a/include/efi_selftest.h
> +++ b/include/efi_selftest.h
> @@ -27,6 +27,15 @@
>         (efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
>         efi_st_printf(__VA_ARGS__)) \
>
> +/*
> + * Prints a TODO message.

Why is this called a TODO message rather than an error?

> + *
> + * @...        format string followed by fields to print
> + */
> +#define efi_st_todo(...) \
> +       (efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
> +       efi_st_printf(__VA_ARGS__)) \
> +
>  /*
>   * A test may be setup and executed at boottime,
>   * it may be setup at boottime and executed at runtime,
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 88b998bd4c..acd184db4b 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -15,6 +15,8 @@ CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI)
> +CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI)
> +CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI)
> @@ -31,6 +33,7 @@ efi_selftest.o \
>  efi_selftest_console.o \
>  efi_selftest_events.o \
>  efi_selftest_exitbootservices.o \
> +efi_selftest_manageprotocols.o \
>  efi_selftest_snp.o \
>  efi_selftest_textoutput.o \
>  efi_selftest_tpl.o \
> diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
> new file mode 100644
> index 0000000000..0a297f79ba
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
> @@ -0,0 +1,381 @@
> +/*
> + * efi_selftest_manageprotocols
> + *
> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk at gmx.de>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + *
> + * This unit test checks the following protocol services:
> + * InstallProtocolInterface, UninstallProtocolInterface,
> + * InstallMultipleProtocolsInterfaces, UninstallMultipleProtocolsInterfaces,
> + * HandleProtocol, ProtocolsPerHandle,
> + * LocateHandle, LocateHandleBuffer.
> + */
> +
> +#include <efi_selftest.h>
> +
> +static struct efi_boot_services *boottime;
> +
> +static efi_handle_t handle1;
> +static efi_handle_t handle2;
> +
> +struct interface {
> +       void (EFIAPI * inc)(void);
> +};
> +
> +static efi_guid_t guid1 =
> +       EFI_GUID(0x2e7ca819, 0x21d3, 0x0a3a,
> +                0xf7, 0x91, 0x82, 0x1f, 0x7a, 0x83, 0x67, 0xaf);
> +static unsigned int counter1;

blank line before function

> +void EFIAPI inc1(void)
> +{
> +       ++counter1;

This is just checking that the function is called, right? I suggest
adding a comment explaining that you are incrementing so the test can
detect success.

> +}
> +
> +static struct interface interface1 = {
> +       inc1,
> +};
> +
> +static struct interface interface4 = {
> +       inc1,
> +};
> +
> +static efi_guid_t guid2 =
> +       EFI_GUID(0xf909f2bb, 0x90a8, 0x0d77,
> +                0x94, 0x0c, 0x3e, 0xa8, 0xea, 0x38, 0xd6, 0x6f);

blank line here

> +static unsigned int counter2;
> +void EFIAPI inc2(void)
> +{
> +       ++counter2;
> +}
> +
> +static struct interface interface2 = {
> +       inc2,
> +};
> +
> +static efi_guid_t guid3 =
> +       EFI_GUID(0x06d641a3, 0xf4e7, 0xe0c9,
> +                0xe7, 0x8d, 0x41, 0x2d, 0x72, 0xa6, 0xb1, 0x24);
> +static unsigned int counter3;

blank line here

> +void EFIAPI inc3(void)
> +{
> +       ++counter3;
> +}
> +
> +static struct interface interface3 = {
> +       inc3,
> +};
> +
> +/*
> + * Find a handle in an array.
> + *
> + * @handle:    handle to find
> + * @count:     number of entries in the array
> + * @buffer:    array to search

@return

Please check in rest of file too.