On 1/24/19 9:03 AM, Simon Goldschmidt wrote:
On Thu, Jan 24, 2019 at 7:51 AM Heinrich Schuchardt xypron.glpk@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@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@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_TESTdefault yhelpEnables the 'ut lib' command which tests library functions likememcat(), 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@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@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@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