[U-Boot] [PATCH 6/6] efi_selftest: add HII database protocols test

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

On Thu, Nov 01, 2018 at 08:33:09AM +0100, Heinrich Schuchardt wrote:
> On 11/01/2018 05:47 AM, AKASHI Takahiro wrote:
> > This efi_selftest tests HII database protocol and HII string protocol.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  lib/efi_selftest/Makefile                |   1 +
> >  lib/efi_selftest/efi_selftest_hii.c      | 882 +++++++++++++++++++++++
> >  lib/efi_selftest/efi_selftest_hii_data.c | 450 ++++++++++++
> >  3 files changed, 1333 insertions(+)
> >  create mode 100644 lib/efi_selftest/efi_selftest_hii.c
> >  create mode 100644 lib/efi_selftest/efi_selftest_hii_data.c
> > 
> > diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> > index 4b1c0bb84b18..1209b54b07cd 100644
> > --- a/lib/efi_selftest/Makefile
> > +++ b/lib/efi_selftest/Makefile
> > @@ -25,6 +25,7 @@ efi_selftest_exception.o \
> >  efi_selftest_exitbootservices.o \
> >  efi_selftest_fdt.o \
> >  efi_selftest_gop.o \
> > +efi_selftest_hii.o \
> >  efi_selftest_loaded_image.o \
> >  efi_selftest_manageprotocols.o \
> >  efi_selftest_rtc.o \
> > diff --git a/lib/efi_selftest/efi_selftest_hii.c b/lib/efi_selftest/efi_selftest_hii.c
> > new file mode 100644
> > index 000000000000..2d5fe095a9b2
> > --- /dev/null
> > +++ b/lib/efi_selftest/efi_selftest_hii.c
> > @@ -0,0 +1,882 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * efi_selftest_hii
> > + *
> > + * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> > + *
> > + * Test HII database protocols
> > + */
> > +
> > +#include <efi_selftest.h>
> > +#include <malloc.h>
> > +#include "efi_selftest_hii_data.c"
> > +
> > +#define PRINT_TESTNAME efi_st_printf("%s:\n", __func__)
> > +
> > +static struct efi_boot_services *boottime;
> > +
> > +static const efi_guid_t hii_database_protocol_guid =
> > +	EFI_HII_DATABASE_PROTOCOL_GUID;
> > +static const efi_guid_t hii_string_protocol_guid =
> > +	EFI_HII_STRING_PROTOCOL_GUID;
> > +
> > +static struct efi_hii_database_protocol *hii_database_protocol;
> > +static struct efi_hii_string_protocol *hii_string_protocol;
> > +
> > +/*
> > + * Setup unit test.
> > + *
> > + * @handle:	handle of the loaded image
> > + * @systable:	system table
> > + *
> > + * @return:	EFI_ST_SUCCESS for success
> > + */
> > +static int setup(const efi_handle_t handle,
> > +		 const struct efi_system_table *systable)
> > +{
> > +	efi_status_t ret;
> > +
> > +	boottime = systable->boottime;
> > +
> > +	/* HII database protocol */
> > +	ret = boottime->locate_protocol(&hii_database_protocol_guid, NULL,
> > +					(void **)&hii_database_protocol);
> > +	if (ret != EFI_SUCCESS) {
> > +		hii_database_protocol = NULL;
> > +		efi_st_error("HII database protocol is not available.\n");
> > +		return EFI_ST_FAILURE;
> > +	}
> > +
> > +	/* HII string protocol */
> > +	ret = boottime->locate_protocol(&hii_string_protocol_guid, NULL,
> > +					(void **)&hii_string_protocol);
> > +	if (ret != EFI_SUCCESS) {
> > +		hii_string_protocol = NULL;
> > +		efi_st_error("HII string protocol is not available.\n");
> > +		return EFI_ST_FAILURE;
> > +	}
> > +
> > +	return EFI_ST_SUCCESS;
> > +}
> > +
> > +/*
> > + * HII database protocol Tests
> 
> Thanks a lot for providing unit tests for the protocols.
> 
> %s/Tests/tests/

Fixed.

> > + */
> > +
> 
> Please, add a comment to each test describing what it is doing, e.g.

OK, but it's a quite painful task.

> /**
>  * test_hii_database_new_package_list() - test creation and removal of
> package lists
>  *
>  * This test adds a new package list and then tries to remove it using
> the provided handle.
>  *
>  * @Return:	status code
>  */
> 
> > +static int test_hii_database_new_package_list(void)
> > +{
> > +	efi_hii_handle_t handle;
> > +	efi_status_t ret;
> > +
> > +	PRINT_TESTNAME;
> > +	ret = hii_database_protocol->new_package_list(hii_database_protocol,
> > +			(struct efi_hii_package_list_header *)packagelist1,
> > +			NULL, &handle);
> > +	if (ret != EFI_SUCCESS || !handle) {
> > +		efi_st_error("new_package_list returned %u\n", (int)ret);
> 
> %s/int/unsigned int/
> 
> > +		return EFI_ST_FAILURE;
> > +	}
> > +
> > +	ret = hii_database_protocol->remove_package_list(hii_database_protocol,
> > +			handle);
> > +	if (ret != EFI_SUCCESS) {
> > +		efi_st_error("remove_package_list returned %u\n", (int)ret);
> 
> %s/int/unsigned int/
> 
> Same change needed all over the file.

OK, but the root issue is that efi_st_printf/error() doesn't support
'l' specifier in "%lx" format.

-Takahiro Akashi

> Best regards
> 
> Heinrich