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

[Heinrich Schuchardt]

On 11/06/2017 05:58 PM, Simon Glass wrote:
> 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?

If some part of the EFI spec is not yet inspected this is not an error 
but a TODO. Furthermore In this case I will not cause the Travis CI test 
to fail.

Best regards

Heinrich

> 
>> + *
>> + * @...        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.
>