| Message ID | 20171120223335.45852-11-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | log: Add a new logging feature | expand |
On Mon, 20 Nov 2017 15:33:31 -0700 Simon Glass <sjg@chromium.org> wrote: > Add a command which exercises the logging system. > Reviewed-by: Lukasz Majewski <lukma@denx.de> > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v3: None > Changes in v2: > - Fix function called when test command is selected > - Fix help output for 'log test' > - Rename LOGL_WARN to LOGL_WARNING > > MAINTAINERS | 1 + > cmd/Kconfig | 3 +- > cmd/log.c | 6 ++ > common/Kconfig | 10 +++ > include/log.h | 3 + > test/Makefile | 1 + > test/log/Makefile | 7 ++ > test/log/log_test.c | 203 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, > 233 insertions(+), 1 deletion(-) create mode 100644 test/log/Makefile > create mode 100644 test/log/log_test.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6814c2fc566..47f68651a7c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -297,6 +297,7 @@ S: Maintained > T: git git://git.denx.de/u-boot.git > F: common/log.c > F: cmd/log.c > +F: test/log/log_test.c > > MICROBLAZE > M: Michal Simek <monstr@monstr.eu> > diff --git a/cmd/Kconfig b/cmd/Kconfig > index b745a7e977a..c0332235261 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -1507,7 +1507,8 @@ config CMD_LOG > help > This provides access to logging features. It allows the > output of log data to be controlled to a limited extent (setting up > the default > - maximum log level for emitting of records). > + maximum log level for emitting of records). It also > provides access > + to a command used for testing the log system. > > config CMD_TRACE > bool "trace - Support tracing of function calls and timing" > diff --git a/cmd/log.c b/cmd/log.c > index 44e04ab16a8..abc523b4971 100644 > --- a/cmd/log.c > +++ b/cmd/log.c > @@ -23,6 +23,9 @@ static int do_log_level(cmd_tbl_t *cmdtp, int flag, > int argc, > static cmd_tbl_t log_sub[] = { > U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, > "", ""), +#ifdef CONFIG_LOG_TEST > + U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""), > +#endif > }; > > static int do_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const > argv[]) @@ -46,6 +49,9 @@ static int do_log(cmd_tbl_t *cmdtp, int > flag, int argc, char * const argv[]) #ifdef CONFIG_SYS_LONGHELP > static char log_help_text[] = > "level - get/set log level\n" > +#ifdef CONFIG_LOG_TEST > + "log test - run log tests\n" > +#endif > ; > #endif > > diff --git a/common/Kconfig b/common/Kconfig > index 1b157e47c3d..4da095a4fd7 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -494,6 +494,16 @@ config LOG_SPL_CONSOLE > log message is shown - other details like level, category, > file and line number are omitted. > > +config LOG_TEST > + bool "Provide a test for logging" > + depends on LOG > + default y if SANDBOX > + help > + This enables a 'log test' command to test logging. It is > normally > + executed from a pytest and simply outputs logging > information > + in various different ways to test that the logging system > works > + correctly with varoius settings. > + > endmenu > > config DEFAULT_FDT_FILE > diff --git a/include/log.h b/include/log.h > index 5dced16880b..7f69a17a27b 100644 > --- a/include/log.h > +++ b/include/log.h > @@ -249,6 +249,9 @@ struct log_filter { > #define LOG_DRIVER(_name) \ > ll_entry_declare(struct log_driver, _name, log_driver) > > +/* Handle the 'log test' command */ > +int do_log_test(cmd_tbl_t *cmdtp, int flag, int argc, char *const > argv[]); + > /** > * log_add_filter() - Add a new filter to a log device > * > diff --git a/test/Makefile b/test/Makefile > index 6305afb2119..40f2244b79b 100644 > --- a/test/Makefile > +++ b/test/Makefile > @@ -10,3 +10,4 @@ obj-$(CONFIG_SANDBOX) += command_ut.o > obj-$(CONFIG_SANDBOX) += compression.o > obj-$(CONFIG_SANDBOX) += print_ut.o > obj-$(CONFIG_UT_TIME) += time_ut.o > +obj-$(CONFIG_$(SPL_)LOG) += log/ > diff --git a/test/log/Makefile b/test/log/Makefile > new file mode 100644 > index 00000000000..b0da8dee282 > --- /dev/null > +++ b/test/log/Makefile > @@ -0,0 +1,7 @@ > +# > +# Copyright (c) 2017 Google, Inc > +# > +# SPDX-License-Identifier: GPL-2.0+ > +# > + > +obj-$(CONFIG_LOG_TEST) += log_test.o > diff --git a/test/log/log_test.c b/test/log/log_test.c > new file mode 100644 > index 00000000000..38618a3b139 > --- /dev/null > +++ b/test/log/log_test.c > @@ -0,0 +1,203 @@ > +/* > + * Logging support test program > + * > + * Copyright (c) 2017 Google, Inc > + * Written by Simon Glass <sjg@chromium.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > + > +/* emit some sample log records in different ways, for testing */ > +static int log_run(enum log_category_t cat, const char *file) > +{ > + int i; > + > + debug("debug\n"); > + for (i = LOGL_FIRST; i < LOGL_COUNT; i++) { > + log(cat, i, "log %d\n", i); > + _log(cat, i, file, 100 + i, "func", "_log %d\n", i); > + } > + > + return 0; > +} > + > +static int log_test(int testnum) > +{ > + int ret; > + > + printf("test %d\n", testnum); > + switch (testnum) { > + case 0: { > + /* Check a category filter using the first category > */ > + enum log_category_t cat_list[] = { > + UCLASS_MMC, UCLASS_SPI, LOGC_NONE, LOGC_END > + }; > + > + ret = log_add_filter("console", cat_list, LOGL_MAX, > NULL); > + if (ret < 0) > + return ret; > + log_run(UCLASS_MMC, "file"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 1: { > + /* Check a category filter using the second category > */ > + enum log_category_t cat_list[] = { > + UCLASS_MMC, UCLASS_SPI, LOGC_END > + }; > + > + ret = log_add_filter("console", cat_list, LOGL_MAX, > NULL); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 2: { > + /* Check a category filter that should block log > entries */ > + enum log_category_t cat_list[] = { > + UCLASS_MMC, LOGC_NONE, LOGC_END > + }; > + > + ret = log_add_filter("console", cat_list, LOGL_MAX, > NULL); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 3: { > + /* Check a passing file filter */ > + ret = log_add_filter("console", NULL, LOGL_MAX, > "file"); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 4: { > + /* Check a failing file filter */ > + ret = log_add_filter("console", NULL, LOGL_MAX, > "file"); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file2"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 5: { > + /* Check a passing file filter (second in list) */ > + ret = log_add_filter("console", NULL, LOGL_MAX, > "file,file2"); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file2"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 6: { > + /* Check a passing file filter */ > + ret = log_add_filter("console", NULL, LOGL_MAX, > + "file,file2,log/log_test.c"); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file2"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 7: { > + /* Check a log level filter */ > + ret = log_add_filter("console", NULL, LOGL_WARNING, > NULL); > + if (ret < 0) > + return ret; > + log_run(UCLASS_SPI, "file"); > + ret = log_remove_filter("console", ret); > + if (ret < 0) > + return ret; > + break; > + } > + case 8: { > + /* Check two filters, one of which passes everything > */ > + int filt1, filt2; > + > + ret = log_add_filter("console", NULL, LOGL_WARNING, > NULL); > + if (ret < 0) > + return ret; > + filt1 = ret; > + ret = log_add_filter("console", NULL, LOGL_MAX, > NULL); > + if (ret < 0) > + return ret; > + filt2 = ret; > + log_run(UCLASS_SPI, "file"); > + ret = log_remove_filter("console", filt1); > + if (ret < 0) > + return ret; > + ret = log_remove_filter("console", filt2); > + if (ret < 0) > + return ret; > + break; > + } > + case 9: { > + /* Check three filters, which together pass > everything */ > + int filt1, filt2, filt3; > + > + ret = log_add_filter("console", NULL, LOGL_MAX, > "file)"); > + if (ret < 0) > + return ret; > + filt1 = ret; > + ret = log_add_filter("console", NULL, LOGL_MAX, > "file2"); > + if (ret < 0) > + return ret; > + filt2 = ret; > + ret = log_add_filter("console", NULL, LOGL_MAX, > + "log/log_test.c"); > + if (ret < 0) > + return ret; > + filt3 = ret; > + log_run(UCLASS_SPI, "file2"); > + ret = log_remove_filter("console", filt1); > + if (ret < 0) > + return ret; > + ret = log_remove_filter("console", filt2); > + if (ret < 0) > + return ret; > + ret = log_remove_filter("console", filt3); > + if (ret < 0) > + return ret; > + break; > + } > + } > + > + return 0; > +} > + > +#ifdef CONFIG_LOG_TEST > +int do_log_test(cmd_tbl_t *cmdtp, int flag, int argc, char *const > argv[]) +{ > + int testnum = 0; > + int ret; > + > + if (argc > 1) > + testnum = simple_strtoul(argv[1], NULL, 10); > + > + ret = log_test(testnum); > + if (ret) > + printf("Test failure (err=%d)\n", ret); > + > + return ret ? CMD_RET_FAILURE : 0; > +} > +#endif Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
On Mon, Nov 20, 2017 at 03:33:31PM -0700, Simon Glass wrote: > Add a command which exercises the logging system. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Lukasz Majewski <lukma@denx.de> NAK. With clang-3.8 we see this (and many more): test/log/log_test.c:35:16: warning: implicit conversion from enumeration type 'enum uclass_id' to different enumeration type 'enum log_category_t' [-Wenum-conversion] UCLASS_MMC, UCLASS_SPI, LOGC_NONE, LOGC_END ^~~~~~~~~~ For nearly all instances of UCLASS_MMC and UCLASS_SPI in the code.
Hi Tom, On 29 November 2017 at 20:35, Tom Rini <trini@konsulko.com> wrote: > On Mon, Nov 20, 2017 at 03:33:31PM -0700, Simon Glass wrote: > >> Add a command which exercises the logging system. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> Reviewed-by: Lukasz Majewski <lukma@denx.de> > > NAK. With clang-3.8 we see this (and many more): > test/log/log_test.c:35:16: warning: implicit conversion from > enumeration type 'enum uclass_id' to different enumeration type 'enum > log_category_t' [-Wenum-conversion] > UCLASS_MMC, UCLASS_SPI, LOGC_NONE, LOGC_END > ^~~~~~~~~~ > > For nearly all instances of UCLASS_MMC and UCLASS_SPI in the code. That is unfortunate. I was really hoping to use the uclass as the logging class. I'll have to add a cast or something like that, Regards, Simon
On Thu, Nov 30, 2017 at 09:27:13AM -0700, Simon Glass wrote: > Hi Tom, > > On 29 November 2017 at 20:35, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Nov 20, 2017 at 03:33:31PM -0700, Simon Glass wrote: > > > >> Add a command which exercises the logging system. > >> > >> Signed-off-by: Simon Glass <sjg@chromium.org> > >> Reviewed-by: Lukasz Majewski <lukma@denx.de> > > > > NAK. With clang-3.8 we see this (and many more): > > test/log/log_test.c:35:16: warning: implicit conversion from > > enumeration type 'enum uclass_id' to different enumeration type 'enum > > log_category_t' [-Wenum-conversion] > > UCLASS_MMC, UCLASS_SPI, LOGC_NONE, LOGC_END > > ^~~~~~~~~~ > > > > For nearly all instances of UCLASS_MMC and UCLASS_SPI in the code. > > That is unfortunate. I was really hoping to use the uclass as the > logging class. I'll have to add a cast or something like that, There were a bunch of other clang related warnings in this series too (I wasn't sure which patch, nor for certain it was this series until I dropped 'em all, after sending the email here). Can you please grab clang-3.8 (easy in Debian 9/Ubuntu 16.04 at least) and see what pops up over the whole series? Also, don't move up newer as, sigh, there's a bunch of other warnings showing up unrelated there and I haven't sat down and tried to figure any of those out. Thanks!
diff --git a/MAINTAINERS b/MAINTAINERS index 6814c2fc566..47f68651a7c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -297,6 +297,7 @@ S: Maintained T: git git://git.denx.de/u-boot.git F: common/log.c F: cmd/log.c +F: test/log/log_test.c MICROBLAZE M: Michal Simek <monstr@monstr.eu> diff --git a/cmd/Kconfig b/cmd/Kconfig index b745a7e977a..c0332235261 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1507,7 +1507,8 @@ config CMD_LOG help This provides access to logging features. It allows the output of log data to be controlled to a limited extent (setting up the default - maximum log level for emitting of records). + maximum log level for emitting of records). It also provides access + to a command used for testing the log system. config CMD_TRACE bool "trace - Support tracing of function calls and timing" diff --git a/cmd/log.c b/cmd/log.c index 44e04ab16a8..abc523b4971 100644 --- a/cmd/log.c +++ b/cmd/log.c @@ -23,6 +23,9 @@ static int do_log_level(cmd_tbl_t *cmdtp, int flag, int argc, static cmd_tbl_t log_sub[] = { U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""), +#ifdef CONFIG_LOG_TEST + U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""), +#endif }; static int do_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) @@ -46,6 +49,9 @@ static int do_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #ifdef CONFIG_SYS_LONGHELP static char log_help_text[] = "level - get/set log level\n" +#ifdef CONFIG_LOG_TEST + "log test - run log tests\n" +#endif ; #endif diff --git a/common/Kconfig b/common/Kconfig index 1b157e47c3d..4da095a4fd7 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -494,6 +494,16 @@ config LOG_SPL_CONSOLE log message is shown - other details like level, category, file and line number are omitted. +config LOG_TEST + bool "Provide a test for logging" + depends on LOG + default y if SANDBOX + help + This enables a 'log test' command to test logging. It is normally + executed from a pytest and simply outputs logging information + in various different ways to test that the logging system works + correctly with varoius settings. + endmenu config DEFAULT_FDT_FILE diff --git a/include/log.h b/include/log.h index 5dced16880b..7f69a17a27b 100644 --- a/include/log.h +++ b/include/log.h @@ -249,6 +249,9 @@ struct log_filter { #define LOG_DRIVER(_name) \ ll_entry_declare(struct log_driver, _name, log_driver) +/* Handle the 'log test' command */ +int do_log_test(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]); + /** * log_add_filter() - Add a new filter to a log device * diff --git a/test/Makefile b/test/Makefile index 6305afb2119..40f2244b79b 100644 --- a/test/Makefile +++ b/test/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_SANDBOX) += command_ut.o obj-$(CONFIG_SANDBOX) += compression.o obj-$(CONFIG_SANDBOX) += print_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o +obj-$(CONFIG_$(SPL_)LOG) += log/ diff --git a/test/log/Makefile b/test/log/Makefile new file mode 100644 index 00000000000..b0da8dee282 --- /dev/null +++ b/test/log/Makefile @@ -0,0 +1,7 @@ +# +# Copyright (c) 2017 Google, Inc +# +# SPDX-License-Identifier: GPL-2.0+ +# + +obj-$(CONFIG_LOG_TEST) += log_test.o diff --git a/test/log/log_test.c b/test/log/log_test.c new file mode 100644 index 00000000000..38618a3b139 --- /dev/null +++ b/test/log/log_test.c @@ -0,0 +1,203 @@ +/* + * Logging support test program + * + * Copyright (c) 2017 Google, Inc + * Written by Simon Glass <sjg@chromium.org> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> + +/* emit some sample log records in different ways, for testing */ +static int log_run(enum log_category_t cat, const char *file) +{ + int i; + + debug("debug\n"); + for (i = LOGL_FIRST; i < LOGL_COUNT; i++) { + log(cat, i, "log %d\n", i); + _log(cat, i, file, 100 + i, "func", "_log %d\n", i); + } + + return 0; +} + +static int log_test(int testnum) +{ + int ret; + + printf("test %d\n", testnum); + switch (testnum) { + case 0: { + /* Check a category filter using the first category */ + enum log_category_t cat_list[] = { + UCLASS_MMC, UCLASS_SPI, LOGC_NONE, LOGC_END + }; + + ret = log_add_filter("console", cat_list, LOGL_MAX, NULL); + if (ret < 0) + return ret; + log_run(UCLASS_MMC, "file"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 1: { + /* Check a category filter using the second category */ + enum log_category_t cat_list[] = { + UCLASS_MMC, UCLASS_SPI, LOGC_END + }; + + ret = log_add_filter("console", cat_list, LOGL_MAX, NULL); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 2: { + /* Check a category filter that should block log entries */ + enum log_category_t cat_list[] = { + UCLASS_MMC, LOGC_NONE, LOGC_END + }; + + ret = log_add_filter("console", cat_list, LOGL_MAX, NULL); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 3: { + /* Check a passing file filter */ + ret = log_add_filter("console", NULL, LOGL_MAX, "file"); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 4: { + /* Check a failing file filter */ + ret = log_add_filter("console", NULL, LOGL_MAX, "file"); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file2"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 5: { + /* Check a passing file filter (second in list) */ + ret = log_add_filter("console", NULL, LOGL_MAX, "file,file2"); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file2"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 6: { + /* Check a passing file filter */ + ret = log_add_filter("console", NULL, LOGL_MAX, + "file,file2,log/log_test.c"); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file2"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 7: { + /* Check a log level filter */ + ret = log_add_filter("console", NULL, LOGL_WARNING, NULL); + if (ret < 0) + return ret; + log_run(UCLASS_SPI, "file"); + ret = log_remove_filter("console", ret); + if (ret < 0) + return ret; + break; + } + case 8: { + /* Check two filters, one of which passes everything */ + int filt1, filt2; + + ret = log_add_filter("console", NULL, LOGL_WARNING, NULL); + if (ret < 0) + return ret; + filt1 = ret; + ret = log_add_filter("console", NULL, LOGL_MAX, NULL); + if (ret < 0) + return ret; + filt2 = ret; + log_run(UCLASS_SPI, "file"); + ret = log_remove_filter("console", filt1); + if (ret < 0) + return ret; + ret = log_remove_filter("console", filt2); + if (ret < 0) + return ret; + break; + } + case 9: { + /* Check three filters, which together pass everything */ + int filt1, filt2, filt3; + + ret = log_add_filter("console", NULL, LOGL_MAX, "file)"); + if (ret < 0) + return ret; + filt1 = ret; + ret = log_add_filter("console", NULL, LOGL_MAX, "file2"); + if (ret < 0) + return ret; + filt2 = ret; + ret = log_add_filter("console", NULL, LOGL_MAX, + "log/log_test.c"); + if (ret < 0) + return ret; + filt3 = ret; + log_run(UCLASS_SPI, "file2"); + ret = log_remove_filter("console", filt1); + if (ret < 0) + return ret; + ret = log_remove_filter("console", filt2); + if (ret < 0) + return ret; + ret = log_remove_filter("console", filt3); + if (ret < 0) + return ret; + break; + } + } + + return 0; +} + +#ifdef CONFIG_LOG_TEST +int do_log_test(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +{ + int testnum = 0; + int ret; + + if (argc > 1) + testnum = simple_strtoul(argv[1], NULL, 10); + + ret = log_test(testnum); + if (ret) + printf("Test failure (err=%d)\n", ret); + + return ret ? CMD_RET_FAILURE : 0; +} +#endif
Add a command which exercises the logging system. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v3: None Changes in v2: - Fix function called when test command is selected - Fix help output for 'log test' - Rename LOGL_WARN to LOGL_WARNING MAINTAINERS | 1 + cmd/Kconfig | 3 +- cmd/log.c | 6 ++ common/Kconfig | 10 +++ include/log.h | 3 + test/Makefile | 1 + test/log/Makefile | 7 ++ test/log/log_test.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 test/log/Makefile create mode 100644 test/log/log_test.c