[U-Boot] [PATCH 1/1] test: provide unit test for memory functions
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Jan 24 10:49:50 UTC 2019
On 1/24/19 9:03 AM, Simon Goldschmidt wrote:
> On Thu, Jan 24, 2019 at 7:51 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> Memory functions may have architecture specific implementations. These
>> should be tested.
>>
>> Provide unit tests for memset(), memcpy(), memmove().
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
> Could you explain to me the changes to the files other than test/lib/string.c?
I should have added a line to the commit message that the patch provides
a 'ut lib' sub-command.
The changes are just the usual stuff that we have for all 'ut' sub-commands:
test/py/README.md defines macro LIB_TEST which is only a wrapper around
UNIT_TEST with the test suite set to 'lib'.
In include/test/suites.h the function to handle sub-command 'ut lib' is
defined.
test/Kconfig provides a config option to enable the libary tests.
test/cmd_ut.c calls the sub-commands. So here the function handling 'ut
lib' has to be called.
test/lib/Makefile is needed to compile the test.
test/lib/cmd_ut_lib.c provides the function do_ut_lib() that handles the
'ut lib' command.
> Does this mean they are finally included to 'make qcheck' (etc.)?
Currently make qcheck only executes unit tests defined via DM_TEST. That
does not make sense but I have not yet found out where this restriction
is coded.
Testing memory functions on the sandbox does not replace testing the
architecture specific functions. So the real tests have to be done in QEMU.
>
> If so, do we have to adapt the other tests in test/lib/ somehow (e.g. use
> LIB_TEST)?
We do not have to adapt them. But it would be reasonable to do so in a
later patch. Currently they are part of the device tree tests.
>
>> ---
>> Hello Alex,
>>
>> you already picked up the patch correcting memmove for x86_64.
>> x86: do not use i386 code for x86_64 memory functions
>> https://lists.denx.de/pipermail/u-boot/2019-January/355851.html
>> So I think you could also pick this patch for efi-next.
>>
>> Best regards
>>
>> Heinrich
>> ---
>> include/test/lib.h | 14 ++++++++++
>> include/test/suites.h | 1 +
>> test/Kconfig | 8 ++++++
>> test/cmd_ut.c | 6 ++++
>> test/lib/Makefile | 2 ++
>> test/lib/cmd_ut_lib.c | 20 +++++++++++++
>> test/lib/string.c | 65 +++++++++++++++++++++++++++++++++++++++++++
>> 7 files changed, 116 insertions(+)
>> create mode 100644 include/test/lib.h
>> create mode 100644 test/lib/cmd_ut_lib.c
>> create mode 100644 test/lib/string.c
>>
>> diff --git a/include/test/lib.h b/include/test/lib.h
>> new file mode 100644
>> index 0000000000..12cca61e8b
>> --- /dev/null
>> +++ b/include/test/lib.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2018 Heinrich Schuchardt <xypron.glpk at gmx.de>
>> + */
>> +
>> +#ifndef __TEST_LIB_H__
>> +#define __TEST_LIB_H__
>> +
>> +#include <test/test.h>
>> +
>> +/* Declare a new library function test */
>> +#define LIB_TEST(_name, _flags) UNIT_TEST(_name, _flags, lib_test)
>> +
>> +#endif /* __TEST_LIB_H__ */
>> diff --git a/include/test/suites.h b/include/test/suites.h
>> index 77d863b4a6..01bee09346 100644
>> --- a/include/test/suites.h
>> +++ b/include/test/suites.h
>> @@ -27,6 +27,7 @@ int do_ut_bloblist(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
>> int do_ut_compression(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
>> int do_ut_dm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> int do_ut_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> +int do_ut_lib(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> int do_ut_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> int do_ut_unicode(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>> diff --git a/test/Kconfig b/test/Kconfig
>> index de16d179d0..48a0e501f8 100644
>> --- a/test/Kconfig
>> +++ b/test/Kconfig
>> @@ -6,6 +6,14 @@ menuconfig UNIT_TEST
>> This does not require sandbox to be included, but it is most
>> often used there.
>>
>> +config UT_LIB
>> + bool "Unit tests for library functions"
>> + depends on UNIT_TEST
>> + default y
>> + help
>> + Enables the 'ut lib' command which tests library functions like
>> + memcat(), memcyp(), memmove().
>> +
>> config UT_TIME
>> bool "Unit tests for time functions"
>> depends on UNIT_TEST
>> diff --git a/test/cmd_ut.c b/test/cmd_ut.c
>> index 56924a5272..e3b89504e7 100644
>> --- a/test/cmd_ut.c
>> +++ b/test/cmd_ut.c
>> @@ -46,6 +46,9 @@ static cmd_tbl_t cmd_ut_sub[] = {
>> #ifdef CONFIG_UT_OVERLAY
>> U_BOOT_CMD_MKENT(overlay, CONFIG_SYS_MAXARGS, 1, do_ut_overlay, "", ""),
>> #endif
>> +#ifdef CONFIG_UT_LIB
>> + U_BOOT_CMD_MKENT(lib, CONFIG_SYS_MAXARGS, 1, do_ut_lib, "", ""),
>> +#endif
>> #ifdef CONFIG_UT_TIME
>> U_BOOT_CMD_MKENT(time, CONFIG_SYS_MAXARGS, 1, do_ut_time, "", ""),
>> #endif
>> @@ -108,6 +111,9 @@ static char ut_help_text[] =
>> #ifdef CONFIG_UT_ENV
>> "ut env [test-name]\n"
>> #endif
>> +#ifdef CONFIG_UT_LIB
>> + "ut lib [test-name] - test library functions\n"
>> +#endif
>> #ifdef CONFIG_UT_OVERLAY
>> "ut overlay [test-name]\n"
>> #endif
>> diff --git a/test/lib/Makefile b/test/lib/Makefile
>> index 5a636aac74..308c61708e 100644
>> --- a/test/lib/Makefile
>> +++ b/test/lib/Makefile
>> @@ -2,5 +2,7 @@
>> #
>> # (C) Copyright 2018
>> # Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
>> +obj-y += cmd_ut_lib.o
>> obj-y += hexdump.o
>> obj-y += lmb.o
>> +obj-y += string.o
>> diff --git a/test/lib/cmd_ut_lib.c b/test/lib/cmd_ut_lib.c
>> new file mode 100644
>> index 0000000000..eb90e53914
>> --- /dev/null
>> +++ b/test/lib/cmd_ut_lib.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 Heinrich Schuchardt <xypron.glpk at gmx.de>
>> + *
>> + * Unit tests for library functions
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <test/lib.h>
>> +#include <test/suites.h>
>> +#include <test/ut.h>
>> +
>> +int do_ut_lib(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> + struct unit_test *tests = ll_entry_start(struct unit_test, lib_test);
>> + const int n_ents = ll_entry_count(struct unit_test, lib_test);
>> +
>> + return cmd_ut_category("lib", tests, n_ents, argc, argv);
>> +}
>> diff --git a/test/lib/string.c b/test/lib/string.c
>> new file mode 100644
>> index 0000000000..52ac4809ac
>> --- /dev/null
>> +++ b/test/lib/string.c
>> @@ -0,0 +1,65 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2019 Heinrich Schuchardt <xypron.glpk at gmx.de>
>> + *
>> + * Unit tests for memory functions
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <test/lib.h>
>> +#include <test/test.h>
>> +#include <test/ut.h>
>> +
>> +static int lib_memset(struct unit_test_state *uts)
>> +{
>> + char work[] = "submarine";
>> + const char expected[] = "suUUUUine";
>> + char *ptr;
>> +
>> + ptr = memset(work + 2, 'U', 4);
>> + ut_asserteq_ptr(work + 2, ptr);
>> + ut_asserteq_str(expected, work);
>> +
>> + return 0;
>> +}
>> +
>> +LIB_TEST(lib_memset, 0);
>> +
>> +static int lib_memcpy(struct unit_test_state *uts)
>> +{
>> + char work[] = "elephants";
>> + const char expected[] = "elelephas";
>> + char *ptr;
>> +
>> + /* Overlapping forward copy */
>> + ptr = memcpy(work + 2, "elepha", 6);
>> + ut_asserteq_ptr(work + 2, ptr);
>> + ut_asserteq_str(expected, work);
>> +
>> + return 0;
>> +}
>> +
>> +LIB_TEST(lib_memcpy, 0);
>> +
>> +static int lib_memmove(struct unit_test_state *uts)
>> +{
>> + char work[] = "elephants";
>> + const char expected1[] = "eleelents";
>> + const char expected2[] = "ellentnts";
>> + char *ptr;
>> +
>> + /* Overlapping forward copy */
>> + ptr = memcpy(work + 3, work, 3);
>
> Should that be 'memmove()'?
yes
>
>> + ut_asserteq_ptr(work + 3, ptr);
>> + ut_asserteq_str(expected1, work);
>> +
>> + /* Overlapping backward copy */
>> + ptr = memcpy(work + 2, work + 4, 4);
>
> Should that be 'memmove()'?
>
>> + ut_asserteq_ptr(work + 2, ptr);
>> + ut_asserteq_str(expected2, work);
>> +
>> + return 0;
>> +}
>> +
>> +LIB_TEST(lib_memmove, 0);
>> --
>> 2.20.1
>>
>
> I could imagine some of these functions having different implementations
> for alignment or bigger vs. smaller blocks, maybe that should be tested
> as well?
That is reasonable. I will look into it.
Thanks for reviewing.
Best regards
Heinrich
>
> Regards,
> Simon
>
More information about the U-Boot
mailing list