From c1a2bb4f836a1c96c8e39a67be9795d462ec3356 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 06:59:56 -0600 Subject: [PATCH 01/13] console: Report an error when output buffer is exhausted If the console output buffer is exhausted, characters are silently dropped from the end. Detect this condition and report an error when reading back the characters. Signed-off-by: Simon Glass <sjg@chromium.org> --- common/console.c | 18 +++++++++++---- include/asm-generic/global_data.h | 16 ++++++++----- include/console.h | 3 ++- test/ut.c | 37 ++++++++++++++++++++++++++----- 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/common/console.c b/common/console.c index 561cdf36a7..73edb28799 100644 --- a/common/console.c +++ b/common/console.c @@ -95,16 +95,22 @@ static void console_record_putc(const char c) { if (!(gd->flags & GD_FLG_RECORD)) return; - if (gd->console_out.start) - membuff_putbyte((struct membuff *)&gd->console_out, c); + if (gd->console_out.start && + !membuff_putbyte((struct membuff *)&gd->console_out, c)) + gd->flags |= GD_FLG_RECORD_OVF; } static void console_record_puts(const char *s) { if (!(gd->flags & GD_FLG_RECORD)) return; - if (gd->console_out.start) - membuff_put((struct membuff *)&gd->console_out, s, strlen(s)); + if (gd->console_out.start) { + int len = strlen(s); + + if (membuff_put((struct membuff *)&gd->console_out, s, len) != + len) + gd->flags |= GD_FLG_RECORD_OVF; + } } static int console_record_getc(void) @@ -742,6 +748,7 @@ void console_record_reset(void) { membuff_purge((struct membuff *)&gd->console_out); membuff_purge((struct membuff *)&gd->console_in); + gd->flags &= ~GD_FLG_RECORD_OVF; } int console_record_reset_enable(void) @@ -754,6 +761,9 @@ int console_record_reset_enable(void) int console_record_readline(char *str, int maxlen) { + if (gd->flags & GD_FLG_RECORD_OVF) + return -ENOSPC; + return membuff_readline((struct membuff *)&gd->console_out, str, maxlen, ' '); } diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 47921d27b1..e278d4c941 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -571,30 +571,34 @@ enum gd_flags { * @GD_FLG_RECORD: record console */ GD_FLG_RECORD = 0x01000, + /** + * @GD_FLG_RECORD_OVF: record console overflow + */ + GD_FLG_RECORD_OVF = 0x02000, /** * @GD_FLG_ENV_DEFAULT: default variable flag */ - GD_FLG_ENV_DEFAULT = 0x02000, + GD_FLG_ENV_DEFAULT = 0x04000, /** * @GD_FLG_SPL_EARLY_INIT: early SPL initialization is done */ - GD_FLG_SPL_EARLY_INIT = 0x04000, + GD_FLG_SPL_EARLY_INIT = 0x08000, /** * @GD_FLG_LOG_READY: log system is ready for use */ - GD_FLG_LOG_READY = 0x08000, + GD_FLG_LOG_READY = 0x10000, /** * @GD_FLG_WDT_READY: watchdog is ready for use */ - GD_FLG_WDT_READY = 0x10000, + GD_FLG_WDT_READY = 0x20000, /** * @GD_FLG_SKIP_LL_INIT: don't perform low-level initialization */ - GD_FLG_SKIP_LL_INIT = 0x20000, + GD_FLG_SKIP_LL_INIT = 0x40000, /** * @GD_FLG_SMP_READY: SMP initialization is complete */ - GD_FLG_SMP_READY = 0x40000, + GD_FLG_SMP_READY = 0x80000, }; #endif /* __ASSEMBLY__ */ diff --git a/include/console.h b/include/console.h index 7e628c0cf8..f848bcbf03 100644 --- a/include/console.h +++ b/include/console.h @@ -72,7 +72,8 @@ int console_record_reset_enable(void); * * @str: Place to put string * @maxlen: Maximum length of @str including nul terminator - * @return length of string returned + * @return length of string returned, or -ENOSPC if the console buffer was + * overflowed by the output */ int console_record_readline(char *str, int maxlen); diff --git a/test/ut.c b/test/ut.c index ea0af153e4..a0fe5facac 100644 --- a/test/ut.c +++ b/test/ut.c @@ -51,14 +51,31 @@ long ut_check_delta(ulong last) return ut_check_free() - last; } +static int readline_check(struct unit_test_state *uts) +{ + int ret; + + ret = console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + if (ret == -ENOSPC) { + ut_fail(uts, __FILE__, __LINE__, __func__, + "Console record buffer too small - increase CONFIG_CONSOLE_RECORD_OUT_SIZE"); + return ret; + } + + return 0; +} + int ut_check_console_line(struct unit_test_state *uts, const char *fmt, ...) { va_list args; + int ret; va_start(args, fmt); vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); va_end(args); - console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ret = readline_check(uts); + if (ret < 0) + return ret; return strcmp(uts->expect_str, uts->actual_str); } @@ -66,11 +83,14 @@ int ut_check_console_line(struct unit_test_state *uts, const char *fmt, ...) int ut_check_console_linen(struct unit_test_state *uts, const char *fmt, ...) { va_list args; + int ret; va_start(args, fmt); vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); va_end(args); - console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ret = readline_check(uts); + if (ret < 0) + return ret; return strncmp(uts->expect_str, uts->actual_str, strlen(uts->expect_str)); @@ -78,19 +98,26 @@ int ut_check_console_linen(struct unit_test_state *uts, const char *fmt, ...) int ut_check_skipline(struct unit_test_state *uts) { + int ret; + if (!console_record_avail()) return -ENFILE; - console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ret = readline_check(uts); + if (ret < 0) + return ret; return 0; } int ut_check_console_end(struct unit_test_state *uts) { + int ret; + if (!console_record_avail()) return 0; - - console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ret = readline_check(uts); + if (ret < 0) + return ret; return 1; } From 090d664eac6ff3bdb39c871bb9079df8e22873fc Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 06:59:57 -0600 Subject: [PATCH 02/13] test: Detect when expect_str is too small If a line of more than 256 bytes is generated, the test will fail but the reason is not clear. Add a check for this condition and print a helpful message. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/ut.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/ut.c b/test/ut.c index a0fe5facac..350509a292 100644 --- a/test/ut.c +++ b/test/ut.c @@ -68,11 +68,17 @@ static int readline_check(struct unit_test_state *uts) int ut_check_console_line(struct unit_test_state *uts, const char *fmt, ...) { va_list args; + int len; int ret; va_start(args, fmt); - vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); + len = vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); va_end(args); + if (len >= sizeof(uts->expect_str)) { + ut_fail(uts, __FILE__, __LINE__, __func__, + "unit_test_state->expect_str too small"); + return -EOVERFLOW; + } ret = readline_check(uts); if (ret < 0) return ret; @@ -83,11 +89,17 @@ int ut_check_console_line(struct unit_test_state *uts, const char *fmt, ...) int ut_check_console_linen(struct unit_test_state *uts, const char *fmt, ...) { va_list args; + int len; int ret; va_start(args, fmt); - vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); + len = vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args); va_end(args); + if (len >= sizeof(uts->expect_str)) { + ut_fail(uts, __FILE__, __LINE__, __func__, + "unit_test_state->expect_str too small"); + return -EOVERFLOW; + } ret = readline_check(uts); if (ret < 0) return ret; From fbb99dcec099254f77efd7e16cdc5b31c46cc888 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 06:59:58 -0600 Subject: [PATCH 03/13] test: Convert print tests to use ut framework This test predates the test framework in U-Boot. It uses #define DEBUG and assert() to check the result. Update it to use the framework so it can report failure constitent with other tests. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/test/suites.h | 1 + test/cmd_ut.c | 2 + test/print_ut.c | 113 ++++++++++++++++++++++-------------------- 3 files changed, 61 insertions(+), 55 deletions(-) diff --git a/include/test/suites.h b/include/test/suites.h index f5d8e139ce..80b41f188c 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -41,6 +41,7 @@ int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_str(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); diff --git a/test/cmd_ut.c b/test/cmd_ut.c index b9c166045d..6f174c6a07 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -51,6 +51,7 @@ static struct cmd_tbl cmd_ut_sub[] = { U_BOOT_CMD_MKENT(setexpr, CONFIG_SYS_MAXARGS, 1, do_ut_setexpr, "", ""), #endif + U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_ut_print, "", ""), #ifdef CONFIG_UT_TIME U_BOOT_CMD_MKENT(time, CONFIG_SYS_MAXARGS, 1, do_ut_time, "", ""), #endif @@ -132,6 +133,7 @@ static char ut_help_text[] = #ifdef CONFIG_UT_OVERLAY "ut overlay [test-name]\n" #endif + "ut print [test-name] - test printing\n" "ut setexpr [test-name] - test setexpr command\n" #ifdef CONFIG_SANDBOX "ut str - Basic test of string functions\n" diff --git a/test/print_ut.c b/test/print_ut.c index 5b0a46de9c..5fabdf8428 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -3,42 +3,49 @@ * Copyright (c) 2012, The Chromium Authors */ -#define DEBUG - #include <common.h> #include <command.h> #include <efi_api.h> #include <display_options.h> #include <log.h> #include <version.h> +#include <test/suites.h> +#include <test/test.h> +#include <test/ut.h> #define FAKE_BUILD_TAG "jenkins-u-boot-denx_uboot_dm-master-build-aarch64" \ "and a lot more text to come" -/* Test printing GUIDs */ -static void guid_ut_print(void) -{ +/* Declare a new print test */ +#define PRINT_TEST(_name, _flags) UNIT_TEST(_name, _flags, print_test) + #if CONFIG_IS_ENABLED(LIB_UUID) +/* Test printing GUIDs */ +static int print_guid(struct unit_test_state *uts) +{ unsigned char guid[16] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; char str[40]; sprintf(str, "%pUb", guid); - assert(!strcmp("01020304-0506-0708-090a-0b0c0d0e0f10", str)); + ut_assertok(strcmp("01020304-0506-0708-090a-0b0c0d0e0f10", str)); sprintf(str, "%pUB", guid); - assert(!strcmp("01020304-0506-0708-090A-0B0C0D0E0F10", str)); + ut_assertok(strcmp("01020304-0506-0708-090A-0B0C0D0E0F10", str)); sprintf(str, "%pUl", guid); - assert(!strcmp("04030201-0605-0807-090a-0b0c0d0e0f10", str)); + ut_assertok(strcmp("04030201-0605-0807-090a-0b0c0d0e0f10", str)); sprintf(str, "%pUL", guid); - assert(!strcmp("04030201-0605-0807-090A-0B0C0D0E0F10", str)); -#endif -} + ut_assertok(strcmp("04030201-0605-0807-090A-0B0C0D0E0F10", str)); + + return 0; +} +PRINT_TEST(print_guid, 0); +#endif -/* Test efi_loader specific printing */ -static void efi_ut_print(void) -{ #if CONFIG_IS_ENABLED(EFI_LOADER) && !defined(API_BUILD) +/* Test efi_loader specific printing */ +static int print_efi_ut(struct unit_test_state *uts) +{ char str[10]; u8 buf[sizeof(struct efi_device_path_sd_mmc_path) + sizeof(struct efi_device_path)]; @@ -60,92 +67,88 @@ static void efi_ut_print(void) dp_end->length = sizeof(struct efi_device_path); snprintf(str, sizeof(str), "_%pD_", buf); - assert(!strcmp("_/SD(3)_", str)); + ut_assertok(strcmp("_/SD(3)_", str)); /* NULL device path */ snprintf(str, sizeof(str), "_%pD_", NULL); - assert(!strcmp("_<NULL>_", str)); -#endif -} + ut_assertok(strcmp("_<NULL>_", str)); -static int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) + return 0; +} +PRINT_TEST(print_efi_ut, 0); +#endif + +static int print_printf(struct unit_test_state *uts) { char big_str[400]; int big_str_len; char str[10], *s; int len; - printf("%s: Testing print\n", __func__); - snprintf(str, sizeof(str), "testing"); - assert(!strcmp("testing", str)); + ut_assertok(strcmp("testing", str)); snprintf(str, sizeof(str), "testing but too long"); - assert(!strcmp("testing b", str)); + ut_assertok(strcmp("testing b", str)); snprintf(str, 1, "testing none"); - assert(!strcmp("", str)); + ut_assertok(strcmp("", str)); *str = 'x'; snprintf(str, 0, "testing none"); - assert(*str == 'x'); + ut_asserteq('x', *str); sprintf(big_str, "_%ls_", L"foo"); - assert(!strcmp("_foo_", big_str)); + ut_assertok(strcmp("_foo_", big_str)); /* Test the banner function */ s = display_options_get_banner(true, str, sizeof(str)); - assert(s == str); - assert(!strcmp("\n\nU-Boo\n\n", s)); + ut_asserteq_ptr(str, s); + ut_assertok(strcmp("\n\nU-Boo\n\n", s)); /* Assert that we do not overwrite memory before the buffer */ str[0] = '`'; s = display_options_get_banner(true, str + 1, 1); - assert(s == str + 1); - assert(!strcmp("`", str)); + ut_asserteq_ptr(str + 1, s); + ut_assertok(strcmp("`", str)); str[0] = '~'; s = display_options_get_banner(true, str + 1, 2); - assert(s == str + 1); - assert(!strcmp("~\n", str)); + ut_asserteq_ptr(str + 1, s); + ut_assertok(strcmp("~\n", str)); /* The last two characters are set to \n\n for all buffer sizes > 2 */ s = display_options_get_banner(false, str, sizeof(str)); - assert(s == str); - assert(!strcmp("U-Boot \n\n", s)); + ut_asserteq_ptr(str, s); + ut_assertok(strcmp("U-Boot \n\n", s)); /* Give it enough space for some of the version */ big_str_len = strlen(version_string) - 5; s = display_options_get_banner_priv(false, FAKE_BUILD_TAG, big_str, big_str_len); - assert(s == big_str); - assert(!strncmp(version_string, s, big_str_len - 3)); - assert(!strcmp("\n\n", s + big_str_len - 3)); + ut_asserteq_ptr(big_str, s); + ut_assertok(strncmp(version_string, s, big_str_len - 3)); + ut_assertok(strcmp("\n\n", s + big_str_len - 3)); /* Give it enough space for the version and some of the build tag */ big_str_len = strlen(version_string) + 9 + 20; s = display_options_get_banner_priv(false, FAKE_BUILD_TAG, big_str, big_str_len); - assert(s == big_str); + ut_asserteq_ptr(big_str, s); len = strlen(version_string); - assert(!strncmp(version_string, s, len)); - assert(!strncmp(", Build: ", s + len, 9)); - assert(!strncmp(FAKE_BUILD_TAG, s + 9 + len, 12)); - assert(!strcmp("\n\n", s + big_str_len - 3)); + ut_assertok(strncmp(version_string, s, len)); + ut_assertok(strncmp(", Build: ", s + len, 9)); + ut_assertok(strncmp(FAKE_BUILD_TAG, s + 9 + len, 12)); + ut_assertok(strcmp("\n\n", s + big_str_len - 3)); - /* Test efi_loader specific printing */ - efi_ut_print(); - - /* Test printing GUIDs */ - guid_ut_print(); - - printf("%s: Everything went swimmingly\n", __func__); return 0; } +PRINT_TEST(print_printf, 0); -U_BOOT_CMD( - ut_print, 1, 1, do_ut_print, - "Very basic test of printf(), etc.", - "" -); +int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct unit_test *tests = UNIT_TEST_SUITE_START(print_test); + const int n_ents = UNIT_TEST_SUITE_COUNT(print_test); + + return cmd_ut_category("print", "print_", tests, n_ents, argc, argv); +} From c614ddf28b064d1de18b97edb6d0f5b91ce70376 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 06:59:59 -0600 Subject: [PATCH 04/13] test: Add a test for print_buffer() Add a test for this function, to cover the various features. Expand the expect_str length to take acount of the ~300-bytes lines generated in one case. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/test/test.h | 4 +-- test/print_ut.c | 82 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/include/test/test.h b/include/test/test.h index bf7d785d8e..0104e189f6 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -32,8 +32,8 @@ struct unit_test_state { struct udevice *testdev; int force_fail_alloc; int skip_post_probe; - char expect_str[256]; - char actual_str[256]; + char expect_str[512]; + char actual_str[512]; }; /* Test flags for each test */ diff --git a/test/print_ut.c b/test/print_ut.c index 5fabdf8428..b4dc902d0d 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -8,11 +8,14 @@ #include <efi_api.h> #include <display_options.h> #include <log.h> +#include <mapmem.h> #include <version.h> #include <test/suites.h> #include <test/test.h> #include <test/ut.h> +#define BUF_SIZE 0x100 + #define FAKE_BUILD_TAG "jenkins-u-boot-denx_uboot_dm-master-build-aarch64" \ "and a lot more text to come" @@ -145,6 +148,85 @@ static int print_printf(struct unit_test_state *uts) } PRINT_TEST(print_printf, 0); +static int print_display_buffer(struct unit_test_state *uts) +{ + u8 *buf; + int i; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\0', BUF_SIZE); + for (i = 0; i < 0x11; i++) + buf[i] = i * 0x11; + + /* bytes */ + console_record_reset(); + print_buffer(0, buf, 1, 0x12, 0); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("00000010: 10 00 .."); + ut_assert_console_end(); + + /* line length */ + console_record_reset(); + print_buffer(0, buf, 1, 0x12, 8); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 ..\"3DUfw"); + ut_assert_nextline("00000008: 88 99 aa bb cc dd ee ff ........"); + ut_assert_nextline("00000010: 10 00 .."); + ut_assert_console_end(); + + /* long line */ + console_record_reset(); + buf[0x41] = 0x41; + print_buffer(0, buf, 1, 0x42, 0x40); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................"); + ut_assert_nextline("00000040: 00 41 .A"); + ut_assert_console_end(); + + /* address */ + console_record_reset(); + print_buffer(0x12345678, buf, 1, 0x12, 0); + ut_assert_nextline("12345678: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("12345688: 10 00 .."); + ut_assert_console_end(); + + /* 16-bit */ + console_record_reset(); + print_buffer(0, buf, 2, 9, 0); + ut_assert_nextline("00000000: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........"); + ut_assert_nextline("00000010: 0010 .."); + ut_assert_console_end(); + + /* 32-bit */ + console_record_reset(); + print_buffer(0, buf, 4, 5, 0); + ut_assert_nextline("00000000: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........"); + ut_assert_nextline("00000010: 00000010 ...."); + ut_assert_console_end(); + + /* 64-bit */ + console_record_reset(); + print_buffer(0, buf, 8, 3, 0); + ut_assert_nextline("00000000: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........"); + ut_assert_nextline("00000010: 0000000000000010 ........"); + ut_assert_console_end(); + + /* ASCII */ + console_record_reset(); + buf[1] = 31; + buf[2] = 32; + buf[3] = 33; + for (i = 0; i < 4; i++) + buf[4 + i] = 126 + i; + buf[8] = 255; + print_buffer(0, buf, 1, 10, 0); + ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~....."); + ut_assert_console_end(); + + unmap_sysmem(buf); + + return 0; +} +PRINT_TEST(print_display_buffer, UT_TESTF_CONSOLE_REC); + int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = UNIT_TEST_SUITE_START(print_test); From c7b16d830e72372a1af54af94f0e83fcc2b1a0fb Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:00 -0600 Subject: [PATCH 05/13] display_options: Drop two spaces before the ASCII column At present with print_buffer() U-Boot shows four spaces between the hex and ASCII data. Two seems enough and matches print_hex_dump(). Change it. Signed-off-by: Simon Glass <sjg@chromium.org> --- lib/display_options.c | 2 +- test/cmd/mem_search.c | 26 +++++++++++++------------- test/dm/rtc.c | 6 +++--- test/print_ut.c | 32 ++++++++++++++++---------------- test/ut.c | 2 +- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/display_options.c b/lib/display_options.c index cd48998b6d..7752baba2b 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -189,7 +189,7 @@ int print_buffer(ulong addr, const void *data, uint width, uint count, lb.uc[i] = '.'; } lb.uc[i] = '\0'; - printf(" %s\n", lb.uc); + printf(" %s\n", lb.uc); /* update references */ addr += thislinelen * width; diff --git a/test/cmd/mem_search.c b/test/cmd/mem_search.c index 94942793a4..f80c9c4068 100644 --- a/test/cmd/mem_search.c +++ b/test/cmd/mem_search.c @@ -30,9 +30,9 @@ static int mem_test_ms_b(struct unit_test_state *uts) buf[0x100] = 0x12; ut_assertok(console_record_reset_enable()); run_command("ms.b 1 ff 12", 0); - ut_assert_nextline("00000030: 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................"); + ut_assert_nextline("00000030: 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................"); ut_assert_nextline("--"); - ut_assert_nextline("000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 12 ................"); + ut_assert_nextline("000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 12 ................"); ut_assert_nextline("2 matches"); ut_assert_console_end(); @@ -57,7 +57,7 @@ static int mem_test_ms_w(struct unit_test_state *uts) buf[BUF_SIZE / 2] = 0x1234; ut_assertok(console_record_reset_enable()); run_command("ms.w 0 80 1234", 0); - ut_assert_nextline("00000030: 0000 0000 1234 0000 0000 0000 0000 0000 ....4..........."); + ut_assert_nextline("00000030: 0000 0000 1234 0000 0000 0000 0000 0000 ....4..........."); ut_assert_nextline("1 match"); ut_assert_console_end(); @@ -82,7 +82,7 @@ static int mem_test_ms_l(struct unit_test_state *uts) buf[BUF_SIZE / 4] = 0x12345678; ut_assertok(console_record_reset_enable()); run_command("ms 0 40 12345678", 0); - ut_assert_nextline("00000030: 00000000 00000000 12345678 00000000 ........xV4....."); + ut_assert_nextline("00000030: 00000000 00000000 12345678 00000000 ........xV4....."); ut_assert_nextline("1 match"); ut_assert_console_end(); @@ -212,10 +212,10 @@ static int mem_test_ms_mult(struct unit_test_state *uts) strcpy(buf + BUF_SIZE - strlen(str) + 1, str); ut_assertok(console_record_reset_enable()); run_command("ms.b 0 100 68 65 6c 6c 6f", 0); - ut_assert_nextline("00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 68 65 ..............he"); - ut_assert_nextline("00000020: 6c 6c 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 llo............."); + ut_assert_nextline("00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 68 65 ..............he"); + ut_assert_nextline("00000020: 6c 6c 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 llo............."); ut_assert_nextline("--"); - ut_assert_nextline("00000060: 00 00 00 68 65 6c 6c 6f 00 00 00 00 00 00 00 00 ...hello........"); + ut_assert_nextline("00000060: 00 00 00 68 65 6c 6c 6f 00 00 00 00 00 00 00 00 ...hello........"); ut_assert_nextline("2 matches"); ut_assert_console_end(); unmap_sysmem(buf); @@ -242,12 +242,12 @@ static int mem_test_ms_s(struct unit_test_state *uts) strcpy(buf + 0xa1, str2); ut_assertok(console_record_reset_enable()); run_command("ms.s 0 100 hello", 0); - ut_assert_nextline("00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 68 65 ..............he"); - ut_assert_nextline("00000020: 6c 6c 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 llo............."); + ut_assert_nextline("00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 68 65 ..............he"); + ut_assert_nextline("00000020: 6c 6c 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 llo............."); ut_assert_nextline("--"); - ut_assert_nextline("00000060: 00 00 00 68 65 6c 6c 6f 00 00 00 00 00 00 00 00 ...hello........"); + ut_assert_nextline("00000060: 00 00 00 68 65 6c 6c 6f 00 00 00 00 00 00 00 00 ...hello........"); ut_assert_nextline("--"); - ut_assert_nextline("000000a0: 00 68 65 6c 6c 6f 74 68 65 72 65 00 00 00 00 00 .hellothere....."); + ut_assert_nextline("000000a0: 00 68 65 6c 6c 6f 74 68 65 72 65 00 00 00 00 00 .hellothere....."); ut_assert_nextline("3 matches"); ut_assert_console_end(); @@ -257,7 +257,7 @@ static int mem_test_ms_s(struct unit_test_state *uts) ut_assertok(console_record_reset_enable()); run_command("ms.s 0 100 hello there", 0); - ut_assert_nextline("000000a0: 00 68 65 6c 6c 6f 74 68 65 72 65 00 00 00 00 00 .hellothere....."); + ut_assert_nextline("000000a0: 00 68 65 6c 6c 6f 74 68 65 72 65 00 00 00 00 00 .hellothere....."); ut_assert_nextline("1 match"); ut_assert_console_end(); @@ -284,7 +284,7 @@ static int mem_test_ms_limit(struct unit_test_state *uts) buf[0x76] = 0x12; ut_assertok(console_record_reset_enable()); run_command("ms.b -l2 1 ff 12", 0); - ut_assert_nextline("00000030: 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................"); + ut_assert_nextline("00000030: 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................"); ut_assert_nextline("--"); ut_assert_nextlinen("00000060: 00 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00"); ut_assert_nextline("2 matches (repeat command to check for more)"); diff --git a/test/dm/rtc.c b/test/dm/rtc.c index 8ab997c87d..c7f9f8f0ce 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -204,7 +204,7 @@ static int dm_test_rtc_cmd_rw(struct unit_test_state *uts) ut_assert_console_end(); run_command("rtc read 0x30 2", 0); - ut_assert_nextline("00000030: aa bb .."); + ut_assert_nextline("00000030: aa bb .."); ut_assert_console_end(); run_command("rtc dev 1", 0); @@ -215,7 +215,7 @@ static int dm_test_rtc_cmd_rw(struct unit_test_state *uts) ut_assert_console_end(); run_command("rtc read 0x30 2", 0); - ut_assert_nextline("00000030: cc dd .."); + ut_assert_nextline("00000030: cc dd .."); ut_assert_console_end(); /* @@ -227,7 +227,7 @@ static int dm_test_rtc_cmd_rw(struct unit_test_state *uts) ut_assert_console_end(); run_command("rtc read 0x30 2", 0); - ut_assert_nextline("00000030: aa bb .."); + ut_assert_nextline("00000030: aa bb .."); ut_assert_console_end(); return 0; diff --git a/test/print_ut.c b/test/print_ut.c index b4dc902d0d..9562db6ec3 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -161,52 +161,52 @@ static int print_display_buffer(struct unit_test_state *uts) /* bytes */ console_record_reset(); print_buffer(0, buf, 1, 0x12, 0); - ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); - ut_assert_nextline("00000010: 10 00 .."); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("00000010: 10 00 .."); ut_assert_console_end(); /* line length */ console_record_reset(); print_buffer(0, buf, 1, 0x12, 8); - ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 ..\"3DUfw"); - ut_assert_nextline("00000008: 88 99 aa bb cc dd ee ff ........"); - ut_assert_nextline("00000010: 10 00 .."); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 ..\"3DUfw"); + ut_assert_nextline("00000008: 88 99 aa bb cc dd ee ff ........"); + ut_assert_nextline("00000010: 10 00 .."); ut_assert_console_end(); /* long line */ console_record_reset(); buf[0x41] = 0x41; print_buffer(0, buf, 1, 0x42, 0x40); - ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................"); - ut_assert_nextline("00000040: 00 41 .A"); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................"); + ut_assert_nextline("00000040: 00 41 .A"); ut_assert_console_end(); /* address */ console_record_reset(); print_buffer(0x12345678, buf, 1, 0x12, 0); - ut_assert_nextline("12345678: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); - ut_assert_nextline("12345688: 10 00 .."); + ut_assert_nextline("12345678: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("12345688: 10 00 .."); ut_assert_console_end(); /* 16-bit */ console_record_reset(); print_buffer(0, buf, 2, 9, 0); - ut_assert_nextline("00000000: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........"); - ut_assert_nextline("00000010: 0010 .."); + ut_assert_nextline("00000000: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........"); + ut_assert_nextline("00000010: 0010 .."); ut_assert_console_end(); /* 32-bit */ console_record_reset(); print_buffer(0, buf, 4, 5, 0); - ut_assert_nextline("00000000: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........"); - ut_assert_nextline("00000010: 00000010 ...."); + ut_assert_nextline("00000000: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........"); + ut_assert_nextline("00000010: 00000010 ...."); ut_assert_console_end(); /* 64-bit */ console_record_reset(); print_buffer(0, buf, 8, 3, 0); - ut_assert_nextline("00000000: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........"); - ut_assert_nextline("00000010: 0000000000000010 ........"); + ut_assert_nextline("00000000: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........"); + ut_assert_nextline("00000010: 0000000000000010 ........"); ut_assert_console_end(); /* ASCII */ @@ -218,7 +218,7 @@ static int print_display_buffer(struct unit_test_state *uts) buf[4 + i] = 126 + i; buf[8] = 255; print_buffer(0, buf, 1, 10, 0); - ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~....."); + ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~....."); ut_assert_console_end(); unmap_sysmem(buf); diff --git a/test/ut.c b/test/ut.c index 350509a292..1eec2a57df 100644 --- a/test/ut.c +++ b/test/ut.c @@ -151,7 +151,7 @@ int ut_check_console_dump(struct unit_test_state *uts, int total_bytes) if (str[8] != ':' || str[9] != ' ') return 1; - bytes = len - 8 - 2 - 3 * 16 - 4; + bytes = len - 8 - 2 - 3 * 16 - 2; upto += bytes; } From 2f410fe55766de190bcc2c3dd18245a00aad1d4f Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:01 -0600 Subject: [PATCH 06/13] hexdump: Move API to header file Move the comments to the header file so people can find the function info without digging in the implementation. Fix up the code style and add an enum for the first arg. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/hexdump.h | 75 ++++++++++++++++++++++++++++++++++++++++++++- lib/hexdump.c | 78 ++--------------------------------------------- 2 files changed, 77 insertions(+), 76 deletions(-) diff --git a/include/hexdump.h b/include/hexdump.h index f7b76ff712..62fce7ae7b 100644 --- a/include/hexdump.h +++ b/include/hexdump.h @@ -10,7 +10,7 @@ #include <linux/ctype.h> #include <linux/types.h> -enum { +enum dump_prefix_t { DUMP_PREFIX_NONE, DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET @@ -81,10 +81,83 @@ static inline char *bin2hex(char *dst, const void *src, size_t count) return dst; } +/** + * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory + * @buf: data blob to dump + * @len: number of bytes in the @buf + * @rowsize: number of bytes to print per line; must be 16 or 32 + * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @linebuf: where to put the converted data + * @linebuflen: total size of @linebuf, including space for terminating NUL + * @ascii: include ASCII after the hex output + * + * hex_dump_to_buffer() works on one "line" of output at a time, i.e., + * 16 or 32 bytes of input data converted to hex + ASCII output. + * + * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data + * to a hex + ASCII dump at the supplied memory location. + * The converted output is always NUL-terminated. + * + * E.g.: + * hex_dump_to_buffer(frame->data, frame->len, 16, 1, + * linebuf, sizeof(linebuf), true); + * + * example output buffer: + * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO + * + * Return: + * The amount of bytes placed in the buffer without terminating NUL. If the + * output was truncated, then the return value is the number of bytes + * (excluding the terminating NUL) which would have been written to the final + * string if enough space had been available. + */ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii); + +/** + * print_hex_dump - print a text hex dump to syslog for a binary blob of data + * @prefix_str: string to prefix each line with; + * caller supplies trailing spaces for alignment if desired + * @prefix_type: controls whether prefix of an offset, address, or none + * is printed (see enum dump_prefix_t) + * @rowsize: number of bytes to print per line; must be 16 or 32 + * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @buf: data blob to dump + * @len: number of bytes in the @buf + * @ascii: include ASCII after the hex output + * + * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump + * to the stdio, with an optional leading prefix. + * + * print_hex_dump() works on one "line" of output at a time, i.e., + * 16 or 32 bytes of input data converted to hex + ASCII output. + * print_hex_dump() iterates over the entire input @buf, breaking it into + * "line size" chunks to format and print. + * + * E.g.: + * print_hex_dump("raw data: ", DUMP_PREFIX_ADDRESS, 16, 1, frame->data, + * frame->len, true); + * + * Example output using %DUMP_PREFIX_OFFSET and 1-byte mode: + * 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO + * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode: + * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. + */ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, int groupsize, const void *buf, size_t len, bool ascii); + +/** + * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params + * @prefix_str: string to prefix each line with; + * caller supplies trailing spaces for alignment if desired + * @prefix_type: controls whether prefix of an offset, address, or none + * is printed (see enum dump_prefix_t) + * @buf: data blob to dump + * @len: number of bytes in the @buf + * + * Calls print_hex_dump(), rowsize of 16, groupsize of 1, + * and ASCII output included. + */ void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len); diff --git a/lib/hexdump.c b/lib/hexdump.c index a3f219a874..e31784cc11 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -19,36 +19,6 @@ const char hex_asc[] = "0123456789abcdef"; const char hex_asc_upper[] = "0123456789ABCDEF"; #if CONFIG_IS_ENABLED(HEXDUMP) -/** - * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory - * @buf: data blob to dump - * @len: number of bytes in the @buf - * @rowsize: number of bytes to print per line; must be 16 or 32 - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) - * @linebuf: where to put the converted data - * @linebuflen: total size of @linebuf, including space for terminating NUL - * @ascii: include ASCII after the hex output - * - * hex_dump_to_buffer() works on one "line" of output at a time, i.e., - * 16 or 32 bytes of input data converted to hex + ASCII output. - * - * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data - * to a hex + ASCII dump at the supplied memory location. - * The converted output is always NUL-terminated. - * - * E.g.: - * hex_dump_to_buffer(frame->data, frame->len, 16, 1, - * linebuf, sizeof(linebuf), true); - * - * example output buffer: - * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO - * - * Return: - * The amount of bytes placed in the buffer without terminating NUL. If the - * output was truncated, then the return value is the number of bytes - * (excluding the terminating NUL) which would have been written to the final - * string if enough space had been available. - */ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii) { @@ -150,35 +120,6 @@ overflow1: return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; } -/** - * print_hex_dump - print a text hex dump to syslog for a binary blob of data - * @prefix_str: string to prefix each line with; - * caller supplies trailing spaces for alignment if desired - * @prefix_type: controls whether prefix of an offset, address, or none - * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) - * @rowsize: number of bytes to print per line; must be 16 or 32 - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) - * @buf: data blob to dump - * @len: number of bytes in the @buf - * @ascii: include ASCII after the hex output - * - * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump - * to the stdio, with an optional leading prefix. - * - * print_hex_dump() works on one "line" of output at a time, i.e., - * 16 or 32 bytes of input data converted to hex + ASCII output. - * print_hex_dump() iterates over the entire input @buf, breaking it into - * "line size" chunks to format and print. - * - * E.g.: - * print_hex_dump("raw data: ", DUMP_PREFIX_ADDRESS, 16, 1, frame->data, - * frame->len, true); - * - * Example output using %DUMP_PREFIX_OFFSET and 1-byte mode: - * 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO - * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode: - * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. - */ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, int groupsize, const void *buf, size_t len, bool ascii) { @@ -210,18 +151,6 @@ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, } } -/** - * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params - * @prefix_str: string to prefix each line with; - * caller supplies trailing spaces for alignment if desired - * @prefix_type: controls whether prefix of an offset, address, or none - * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) - * @buf: data blob to dump - * @len: number of bytes in the @buf - * - * Calls print_hex_dump(), rowsize of 16, groupsize of 1, - * and ASCII output included. - */ void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { @@ -232,14 +161,13 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type, * Some code in U-Boot copy-pasted from Linux kernel uses both * functions below so to keep stuff compilable we keep these stubs here. */ -void print_hex_dump(const char *prefix_str, int prefix_type, - int rowsize, int groupsize, const void *buf, - size_t len, bool ascii) +void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) { } void print_hex_dump_bytes(const char *prefix_str, int prefix_type, - const void *buf, size_t len) + const void *buf, size_t len) { } #endif /* CONFIG_HEXDUMP */ From 19edf139e900ed61825b32bc7a261e5f6606b8b1 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:02 -0600 Subject: [PATCH 07/13] hexdump: Add support for sandbox The current implementation outputs an address as a pointer. Update the code to use an address instead, respecting the 32/64 nature of the CPU. Add some initial tests copied from print_test_display_buffer(), just the ones that can pass with the current implementation. Note that for this case print_hex_dump() and print_bufffer() produce the same result. For now the tests are duplicated sine we have separate functions. Signed-off-by: Simon Glass <sjg@chromium.org> --- lib/hexdump.c | 5 ++++- test/print_ut.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/hexdump.c b/lib/hexdump.c index e31784cc11..a76ea707b6 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -10,6 +10,7 @@ #include <common.h> #include <hexdump.h> +#include <mapmem.h> #include <linux/ctype.h> #include <linux/compat.h> #include <linux/log2.h> @@ -139,7 +140,9 @@ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, switch (prefix_type) { case DUMP_PREFIX_ADDRESS: - printf("%s%p: %s\n", prefix_str, ptr + i, linebuf); + printf("%s%0*lx: %s\n", prefix_str, + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, + (ulong)map_to_sysmem(ptr) + i, linebuf); break; case DUMP_PREFIX_OFFSET: printf("%s%.8x: %s\n", prefix_str, i, linebuf); diff --git a/test/print_ut.c b/test/print_ut.c index 9562db6ec3..b9c4b1142c 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -227,6 +227,64 @@ static int print_display_buffer(struct unit_test_state *uts) } PRINT_TEST(print_display_buffer, UT_TESTF_CONSOLE_REC); +static int print_do_hex_dump(struct unit_test_state *uts) +{ + u8 *buf; + int i; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\0', BUF_SIZE); + for (i = 0; i < 0x11; i++) + buf[i] = i * 0x11; + + /* bytes */ + console_record_reset(); + print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("00000010: 10 00 .."); + ut_assert_console_end(); + + /* 16-bit */ + console_record_reset(); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 2, buf, 0x12, true); + ut_assert_nextline("00000000: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........"); + ut_assert_nextline("00000010: 0010 .."); + ut_assert_console_end(); + unmap_sysmem(buf); + + /* 32-bit */ + console_record_reset(); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 4, buf, 0x14, true); + ut_assert_nextline("00000000: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........"); + ut_assert_nextline("00000010: 00000010 ...."); + ut_assert_console_end(); + unmap_sysmem(buf); + + /* 64-bit */ + console_record_reset(); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true); + ut_assert_nextline("00000000: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........"); + ut_assert_nextline("00000010: 0000000000000010 ........"); + ut_assert_console_end(); + unmap_sysmem(buf); + + /* ASCII */ + console_record_reset(); + buf[1] = 31; + buf[2] = 32; + buf[3] = 33; + for (i = 0; i < 4; i++) + buf[4 + i] = 126 + i; + buf[8] = 255; + print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 1, buf, 10, true); + ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~....."); + ut_assert_console_end(); + unmap_sysmem(buf); + + return 0; +} +PRINT_TEST(print_do_hex_dump, UT_TESTF_CONSOLE_REC); + int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = UNIT_TEST_SUITE_START(print_test); From 5d6d2b88389a99c9e20618593e64a9dd74862c8a Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:03 -0600 Subject: [PATCH 08/13] hexdump: Support any rowsize At present print_hex_dump() only supports either 16- or 32-byte lines. With U-Boot we want to support any line length up to a maximum of 64. Update the function to support this, with 0 defaulting to 16, as with print_buffer(). Signed-off-by: Simon Glass <sjg@chromium.org> --- include/hexdump.h | 4 ++-- lib/hexdump.c | 12 +++++++++--- test/print_ut.c | 23 ++++++++++++++++++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/include/hexdump.h b/include/hexdump.h index 62fce7ae7b..b75e26025a 100644 --- a/include/hexdump.h +++ b/include/hexdump.h @@ -85,7 +85,7 @@ static inline char *bin2hex(char *dst, const void *src, size_t count) * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory * @buf: data blob to dump * @len: number of bytes in the @buf - * @rowsize: number of bytes to print per line; must be 16 or 32 + * @rowsize: number of bytes to print per line; max 64 * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL @@ -120,7 +120,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, * caller supplies trailing spaces for alignment if desired * @prefix_type: controls whether prefix of an offset, address, or none * is printed (see enum dump_prefix_t) - * @rowsize: number of bytes to print per line; must be 16 or 32 + * @rowsize: number of bytes to print per line; max 64 * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) * @buf: data blob to dump * @len: number of bytes in the @buf diff --git a/lib/hexdump.c b/lib/hexdump.c index a76ea707b6..a56e108164 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -16,6 +16,8 @@ #include <linux/log2.h> #include <asm/unaligned.h> +#define MAX_LINE_LENGTH_BYTES 64 + const char hex_asc[] = "0123456789abcdef"; const char hex_asc_upper[] = "0123456789ABCDEF"; @@ -30,8 +32,10 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int ascii_column; int ret; - if (rowsize != 16 && rowsize != 32) + if (!rowsize) rowsize = 16; + else + rowsize = min(rowsize, MAX_LINE_LENGTH_BYTES); if (len > rowsize) /* limit to one line at a time */ len = rowsize; @@ -126,10 +130,12 @@ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, { const u8 *ptr = buf; int i, linelen, remaining = len; - char linebuf[32 * 3 + 2 + 32 + 1]; + char linebuf[MAX_LINE_LENGTH_BYTES * 3 + 2 + MAX_LINE_LENGTH_BYTES + 1]; - if (rowsize != 16 && rowsize != 32) + if (!rowsize) rowsize = 16; + else + rowsize = min(rowsize, MAX_LINE_LENGTH_BYTES); for (i = 0; i < len; i += rowsize) { linelen = min(remaining, rowsize); diff --git a/test/print_ut.c b/test/print_ut.c index b9c4b1142c..86b1a5477e 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -244,9 +244,26 @@ static int print_do_hex_dump(struct unit_test_state *uts) ut_assert_nextline("00000010: 10 00 .."); ut_assert_console_end(); + /* line length */ + console_record_reset(); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 ..\"3DUfw"); + ut_assert_nextline("00000008: 88 99 aa bb cc dd ee ff ........"); + ut_assert_nextline("00000010: 10 00 .."); + ut_assert_console_end(); + unmap_sysmem(buf); + + /* long line */ + console_record_reset(); + buf[0x41] = 0x41; + print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true); + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................"); + ut_assert_nextline("00000040: 00 41 .A"); + ut_assert_console_end(); + /* 16-bit */ console_record_reset(); - print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 2, buf, 0x12, true); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true); ut_assert_nextline("00000000: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........"); ut_assert_nextline("00000010: 0010 .."); ut_assert_console_end(); @@ -254,7 +271,7 @@ static int print_do_hex_dump(struct unit_test_state *uts) /* 32-bit */ console_record_reset(); - print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 4, buf, 0x14, true); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true); ut_assert_nextline("00000000: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........"); ut_assert_nextline("00000010: 00000010 ...."); ut_assert_console_end(); @@ -276,7 +293,7 @@ static int print_do_hex_dump(struct unit_test_state *uts) for (i = 0; i < 4; i++) buf[4 + i] = 126 + i; buf[8] = 255; - print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 1, buf, 10, true); + print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true); ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~....."); ut_assert_console_end(); unmap_sysmem(buf); From 735dd6ef89ebd3e05c1dfaef06aca61046503c63 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:04 -0600 Subject: [PATCH 09/13] hexdump: Allow ctrl-c to interrupt output If a long hexdump is initated the user may wish to interrupt it. Add support for this. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/hexdump.h | 6 ++++-- lib/hexdump.c | 13 +++++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/include/hexdump.h b/include/hexdump.h index b75e26025a..f2ca4793d6 100644 --- a/include/hexdump.h +++ b/include/hexdump.h @@ -125,6 +125,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, * @buf: data blob to dump * @len: number of bytes in the @buf * @ascii: include ASCII after the hex output + * Returns: 0 if finished normally, -EINTR if Ctrl-C was pressed, -ENOSYS if not + * supported * * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump * to the stdio, with an optional leading prefix. @@ -143,8 +145,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode: * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. */ -void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, - int groupsize, const void *buf, size_t len, bool ascii); +int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii); /** * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params diff --git a/lib/hexdump.c b/lib/hexdump.c index a56e108164..149c93ead8 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -125,8 +125,8 @@ overflow1: return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; } -void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, - int groupsize, const void *buf, size_t len, bool ascii) +int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) { const u8 *ptr = buf; int i, linelen, remaining = len; @@ -157,7 +157,11 @@ void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, printf("%s%s\n", prefix_str, linebuf); break; } + if (!IS_ENABLED(CONFIG_SPL_BUILD) && ctrlc()) + return -EINTR; } + + return 0; } void print_hex_dump_bytes(const char *prefix_str, int prefix_type, @@ -170,9 +174,10 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type, * Some code in U-Boot copy-pasted from Linux kernel uses both * functions below so to keep stuff compilable we keep these stubs here. */ -void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, - int groupsize, const void *buf, size_t len, bool ascii) +int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) { + return -ENOSYS; } void print_hex_dump_bytes(const char *prefix_str, int prefix_type, From 0cceb99ac59b1d383488ea3ce6511ffc01da5332 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:05 -0600 Subject: [PATCH 10/13] display_options: Split print_buffer() into two functions At present print_buffer() outputs a hex dump but it is not possible to place this dump in a string. Refactor it into a top-level function which does the printing and a utility function that dumps a line into a string. This makes the code more generally useful. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/display_options.h | 25 +++++++++ lib/display_options.c | 115 +++++++++++++++++++++++--------------- test/print_ut.c | 26 +++++++++ 3 files changed, 121 insertions(+), 45 deletions(-) diff --git a/include/display_options.h b/include/display_options.h index 049688e39e..43810cbe22 100644 --- a/include/display_options.h +++ b/include/display_options.h @@ -47,6 +47,31 @@ void print_freq(uint64_t freq, const char *suffix); int print_buffer(ulong addr, const void *data, uint width, uint count, uint linelen); +/* + * Maximum length of an output line is when width == 1 + * 9 for address, + * a space, two hex digits and an ASCII character for each byte + * 2 spaces between the hex and ASCII + * \0 terminator + */ +#define HEXDUMP_MAX_BUF_LENGTH(bytes) (9 + (bytes) * 4 + 3) + +/** + * hexdump_line() - Print out a single line of a hex dump + * + * @addr: Starting address to display at start of line + * @data: pointer to data buffer + * @width: data value width. May be 1, 2, or 4. + * @count: number of values to display + * @linelen: Number of values to print per line; specify 0 for default length + * @out: Output buffer to hold the dump + * @size: Size of output buffer in bytes + * @return number of bytes processed, if OK, -ENOSPC if buffer too small + * + */ +int hexdump_line(ulong addr, const void *data, uint width, uint count, + uint linelen, char *out, int size); + /** * display_options() - display the version string / build tag * diff --git a/lib/display_options.c b/lib/display_options.c index 7752baba2b..c08a87e316 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -131,10 +131,11 @@ void print_size(uint64_t size, const char *s) printf (" %ciB%s", c, s); } -#define MAX_LINE_LENGTH_BYTES (64) -#define DEFAULT_LINE_LENGTH_BYTES (16) -int print_buffer(ulong addr, const void *data, uint width, uint count, - uint linelen) +#define MAX_LINE_LENGTH_BYTES 64 +#define DEFAULT_LINE_LENGTH_BYTES 16 + +int hexdump_line(ulong addr, const void *data, uint width, uint count, + uint linelen, char *out, int size) { /* linebuf as a union causes proper alignment */ union linebuf { @@ -143,62 +144,86 @@ int print_buffer(ulong addr, const void *data, uint width, uint count, uint16_t us[MAX_LINE_LENGTH_BYTES/sizeof(uint16_t) + 1]; uint8_t uc[MAX_LINE_LENGTH_BYTES/sizeof(uint8_t) + 1]; } lb; + uint thislinelen; int i; ulong x; + if (linelen * width > MAX_LINE_LENGTH_BYTES) + linelen = MAX_LINE_LENGTH_BYTES / width; + if (linelen < 1) + linelen = DEFAULT_LINE_LENGTH_BYTES / width; + + /* + * Check the size here so that we don't need to use snprintf(). This + * helps to reduce code size + */ + if (size < HEXDUMP_MAX_BUF_LENGTH(linelen * width)) + return -ENOSPC; + + thislinelen = linelen; + out += sprintf(out, "%08lx:", addr); + + /* check for overflow condition */ + if (count < thislinelen) + thislinelen = count; + + /* Copy from memory into linebuf and print hex values */ + for (i = 0; i < thislinelen; i++) { + if (width == 4) + x = lb.ui[i] = *(volatile uint32_t *)data; + else if (MEM_SUPPORT_64BIT_DATA && width == 8) + x = lb.uq[i] = *(volatile ulong *)data; + else if (width == 2) + x = lb.us[i] = *(volatile uint16_t *)data; + else + x = lb.uc[i] = *(volatile uint8_t *)data; + if (CONFIG_IS_ENABLED(USE_TINY_PRINTF)) + out += sprintf(out, " %x", (uint)x); + else + out += sprintf(out, " %0*lx", width * 2, x); + data += width; + } + + /* fill line with whitespace for nice ASCII print */ + for (i = 0; i < (linelen - thislinelen) * (width * 2 + 1); i++) + *out++ = ' '; + + /* Print data in ASCII characters */ + for (i = 0; i < thislinelen * width; i++) { + if (!isprint(lb.uc[i]) || lb.uc[i] >= 0x80) + lb.uc[i] = '.'; + } + lb.uc[i] = '\0'; + out += sprintf(out, " %s", lb.uc); + + return thislinelen; +} + +int print_buffer(ulong addr, const void *data, uint width, uint count, + uint linelen) +{ if (linelen*width > MAX_LINE_LENGTH_BYTES) linelen = MAX_LINE_LENGTH_BYTES / width; if (linelen < 1) linelen = DEFAULT_LINE_LENGTH_BYTES / width; while (count) { - uint thislinelen = linelen; - printf("%08lx:", addr); + uint thislinelen; + char buf[HEXDUMP_MAX_BUF_LENGTH(width * linelen)]; - /* check for overflow condition */ - if (count < thislinelen) - thislinelen = count; - - /* Copy from memory into linebuf and print hex values */ - for (i = 0; i < thislinelen; i++) { - if (width == 4) - x = lb.ui[i] = *(volatile uint32_t *)data; - else if (MEM_SUPPORT_64BIT_DATA && width == 8) - x = lb.uq[i] = *(volatile ulong *)data; - else if (width == 2) - x = lb.us[i] = *(volatile uint16_t *)data; - else - x = lb.uc[i] = *(volatile uint8_t *)data; - if (CONFIG_IS_ENABLED(USE_TINY_PRINTF)) - printf(" %x", (uint)x); - else - printf(" %0*lx", width * 2, x); - data += width; - } - - while (thislinelen < linelen) { - /* fill line with whitespace for nice ASCII print */ - for (i=0; i<width*2+1; i++) - puts(" "); - linelen--; - } - - /* Print data in ASCII characters */ - for (i = 0; i < thislinelen * width; i++) { - if (!isprint(lb.uc[i]) || lb.uc[i] >= 0x80) - lb.uc[i] = '.'; - } - lb.uc[i] = '\0'; - printf(" %s\n", lb.uc); + thislinelen = hexdump_line(addr, data, width, count, linelen, + buf, sizeof(buf)); + assert(thislinelen >= 0); + puts(buf); + putc('\n'); /* update references */ + data += thislinelen * width; addr += thislinelen * width; count -= thislinelen; -#ifndef CONFIG_SPL_BUILD - if (ctrlc()) - return -1; -#endif + if (!IS_ENABLED(CONFIG_SPL_BUILD) && ctrlc()) + return -EINTR; } return 0; diff --git a/test/print_ut.c b/test/print_ut.c index 86b1a5477e..e2bcfbef00 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -227,6 +227,32 @@ static int print_display_buffer(struct unit_test_state *uts) } PRINT_TEST(print_display_buffer, UT_TESTF_CONSOLE_REC); +static int print_hexdump_line(struct unit_test_state *uts) +{ + char *linebuf; + u8 *buf; + int i; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\0', BUF_SIZE); + for (i = 0; i < 0x11; i++) + buf[i] = i * 0x11; + + /* Check buffer size calculations */ + linebuf = map_sysmem(0x400, BUF_SIZE); + memset(linebuf, '\xff', BUF_SIZE); + ut_asserteq(-ENOSPC, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 75)); + ut_asserteq(-1, linebuf[0]); + ut_asserteq(0x10, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 76)); + ut_asserteq(0, linebuf[75]); + ut_asserteq(-1, linebuf[76]); + + unmap_sysmem(buf); + + return 0; +} +PRINT_TEST(print_hexdump_line, UT_TESTF_CONSOLE_REC); + static int print_do_hex_dump(struct unit_test_state *uts) { u8 *buf; From 58b4b7133aba6fbb2409a975478157f9277c2e91 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 07:00:06 -0600 Subject: [PATCH 11/13] log: Add support for logging a buffer The print_buffer() function is very useful for debugging. Add a version of this in the log system also. Signed-off-by: Simon Glass <sjg@chromium.org> --- common/log.c | 30 ++++++++++++++++++++++++++++++ include/log.h | 35 +++++++++++++++++++++++++++++++++++ test/log/log_test.c | 27 +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/common/log.c b/common/log.c index ea407c6db9..1aaa6c1527 100644 --- a/common/log.c +++ b/common/log.c @@ -284,6 +284,36 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, return 0; } +#define MAX_LINE_LENGTH_BYTES 64 +#define DEFAULT_LINE_LENGTH_BYTES 16 + +int _log_buffer(enum log_category_t cat, enum log_level_t level, + const char *file, int line, const char *func, ulong addr, + const void *data, uint width, uint count, uint linelen) +{ + if (linelen * width > MAX_LINE_LENGTH_BYTES) + linelen = MAX_LINE_LENGTH_BYTES / width; + if (linelen < 1) + linelen = DEFAULT_LINE_LENGTH_BYTES / width; + + while (count) { + uint thislinelen; + char buf[HEXDUMP_MAX_BUF_LENGTH(width * linelen)]; + + thislinelen = hexdump_line(addr, data, width, count, linelen, + buf, sizeof(buf)); + assert(thislinelen >= 0); + _log(cat, level, file, line, func, "%s\n", buf); + + /* update references */ + data += thislinelen * width; + addr += thislinelen * width; + count -= thislinelen; + } + + return 0; +} + int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[], enum log_level_t level, const char *file_list, int flags) diff --git a/include/log.h b/include/log.h index add3a1e4a0..feb0204855 100644 --- a/include/log.h +++ b/include/log.h @@ -140,6 +140,24 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, return 0; } +/** + * _log_buffer - Internal function to print data buffer in hex and ascii form + * + * @cat: Category of log record (indicating which subsystem generated it) + * @level: Level of log record (indicating its severity) + * @file: File name of file where log record was generated + * @line: Line number in file where log record was generated + * @func: Function where log record was generated + * @addr: Starting address to display at start of line + * @data: pointer to data buffer + * @width: data value width. May be 1, 2, or 4. + * @count: number of values to display + * @linelen: Number of values to print per line; specify 0 for default length + */ +int _log_buffer(enum log_category_t cat, enum log_level_t level, + const char *file, int line, const char *func, ulong addr, + const void *data, uint width, uint count, uint linelen); + /* Define this at the top of a file to add a prefix to debug messages */ #ifndef pr_fmt #define pr_fmt(fmt) fmt @@ -200,8 +218,25 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, __LINE__, __func__, \ pr_fmt(_fmt), ##_args); \ }) + +/* Emit a dump if the level is less that the maximum */ +#define log_buffer(_cat, _level, _addr, _data, _width, _count, _linelen) ({ \ + int _l = _level; \ + if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \ + _log_buffer((enum log_category_t)(_cat), \ + (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \ + __LINE__, __func__, _addr, _data, \ + _width, _count, _linelen); \ + }) #else #define log(_cat, _level, _fmt, _args...) + +#define log_buffer(_cat, _level, _addr, _data, _width, _count, _linelen) ({ \ + int _l = _level; \ + if (_LOG_DEBUG != 0 || _l <= LOGL_INFO || \ + (_DEBUG && _l == LOGL_DEBUG)) \ + print_buffer(_addr, _data, _width, _count, _linelen); \ + }) #endif #define log_nop(_cat, _level, _fmt, _args...) ({ \ diff --git a/test/log/log_test.c b/test/log/log_test.c index 4a814ff413..f1e67509c1 100644 --- a/test/log/log_test.c +++ b/test/log/log_test.c @@ -429,3 +429,30 @@ int log_test_dropped(struct unit_test_state *uts) return 0; } LOG_TEST_FLAGS(log_test_dropped, UT_TESTF_CONSOLE_REC); + +/* Check log_buffer() */ +int log_test_buffer(struct unit_test_state *uts) +{ + u8 *buf; + int i; + + buf = malloc(0x20); + ut_assertnonnull(buf); + memset(buf, '\0', 0x20); + for (i = 0; i < 0x11; i++) + buf[i] = i * 0x11; + + ut_assertok(console_record_reset_enable()); + log_buffer(LOGC_BOOT, LOGL_INFO, 0, buf, 1, 0x12, 0); + + /* This one should product no output due to the debug level */ + log_buffer(LOGC_BOOT, LOGL_DEBUG, 0, buf, 1, 0x12, 0); + + ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........"); + ut_assert_nextline("00000010: 10 00 .."); + ut_assert_console_end(); + free(buf); + + return 0; +} +LOG_TEST_FLAGS(log_test_buffer, UT_TESTF_CONSOLE_REC); From 8d9bb98f8649222777193c6c161361e1ebd5e3fa Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 13:46:53 -0600 Subject: [PATCH 12/13] sandbox: log: Avoid build error with !CONFIG_LOG The pr_cont_test.c test requires CONFIG_LOG since it directly accesses fields in global_data that require it. Move the test into the CONFIG_LOG condition to avoid build errors. Enable CONFIG_LOG on sandbox (not sandbox_spl, etc.) so that we still run this test. This requires resyncing of the configs. Signed-off-by: Simon Glass <sjg@chromium.org> --- configs/sandbox_defconfig | 1 + test/log/Makefile | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index bdbf714e2b..60cdad1084 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -21,6 +21,7 @@ CONFIG_BOOTSTAGE_STASH_SIZE=0x4096 CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_PRE_CONSOLE_BUFFER=y +CONFIG_LOG=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_F=y CONFIG_STACKPROTECTOR=y diff --git a/test/log/Makefile b/test/log/Makefile index a3dedace04..09f8689d07 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -17,6 +17,7 @@ endif ifdef CONFIG_LOG obj-y += pr_cont_test.o obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o +obj-y += pr_cont_test.o else obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o endif From e1cbd916c86cbfdb87a7b2219624057428c285d4 Mon Sep 17 00:00:00 2001 From: Simon Glass <sjg@chromium.org> Date: Sat, 8 May 2021 13:46:54 -0600 Subject: [PATCH 13/13] log: Convert log values to printf() if not enabled At present if logging not enabled, log_info() becomes a nop. But we want log output at the 'info' level to be akin to printf(). Update the macro to pass the output straight to printf() in this case. This mimics the behaviour for the log_...() macros like log_debug() and log_info(), so we can drop the special case for these. Add new tests to cover this case. Signed-off-by: Simon Glass <sjg@chromium.org> --- doc/develop/logging.rst | 6 ++++-- include/log.h | 34 +++++++++++++++------------------- test/log/Makefile | 1 + test/log/nolog_ndebug.c | 39 +++++++++++++++++++++++++++++++++++++++ test/log/nolog_test.c | 3 +++ 5 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 test/log/nolog_ndebug.c diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst index f4e925048e..51095b05ba 100644 --- a/doc/develop/logging.rst +++ b/doc/develop/logging.rst @@ -52,6 +52,10 @@ If CONFIG_LOG is not set, then no logging will be available. The above have SPL and TPL versions also, e.g. CONFIG_SPL_LOG_MAX_LEVEL and CONFIG_TPL_LOG_MAX_LEVEL. +If logging is disabled, the default behaviour is to output any message at +level LOGL_INFO and below. If logging is disabled and DEBUG is defined (at +the very top of a C file) then any message at LOGL_DEBUG will be written. + Temporary logging within a single file -------------------------------------- @@ -291,8 +295,6 @@ More logging destinations: Convert debug() statements in the code to log() statements -Support making printf() emit log statements at L_INFO level - Convert error() statements in the code to log() statements Figure out what to do with BUG(), BUG_ON() and warn_non_spl() diff --git a/include/log.h b/include/log.h index feb0204855..e0e12ce194 100644 --- a/include/log.h +++ b/include/log.h @@ -174,6 +174,10 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level, */ #if CONFIG_IS_ENABLED(LOG) #define _LOG_MAX_LEVEL CONFIG_VAL(LOG_MAX_LEVEL) +#else +#define _LOG_MAX_LEVEL LOGL_INFO +#endif + #define log_emer(_fmt...) log(LOG_CATEGORY, LOGL_EMERG, ##_fmt) #define log_alert(_fmt...) log(LOG_CATEGORY, LOGL_ALERT, ##_fmt) #define log_crit(_fmt...) log(LOG_CATEGORY, LOGL_CRIT, ##_fmt) @@ -185,34 +189,19 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level, #define log_content(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_CONTENT, ##_fmt) #define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) #define log_cont(_fmt...) log(LOGC_CONT, LOGL_CONT, ##_fmt) -#else -#define _LOG_MAX_LEVEL LOGL_INFO -#define log_emerg(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_alert(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_crit(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_info(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_cont(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__) -#define log_content(_fmt...) log_nop(LOG_CATEGORY, \ - LOGL_DEBUG_CONTENT, ##_fmt) -#define log_io(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) -#endif -#if CONFIG_IS_ENABLED(LOG) #ifdef LOG_DEBUG #define _LOG_DEBUG LOGL_FORCE_DEBUG #else #define _LOG_DEBUG 0 #endif +#if CONFIG_IS_ENABLED(LOG) + /* Emit a log record if the level is less that the maximum */ #define log(_cat, _level, _fmt, _args...) ({ \ int _l = _level; \ - if (CONFIG_IS_ENABLED(LOG) && \ - (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL)) \ + if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \ _log((enum log_category_t)(_cat), \ (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \ __LINE__, __func__, \ @@ -229,7 +218,14 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level, _width, _count, _linelen); \ }) #else -#define log(_cat, _level, _fmt, _args...) + +/* Note: _LOG_DEBUG != 0 avoids a warning with clang */ +#define log(_cat, _level, _fmt, _args...) ({ \ + int _l = _level; \ + if (_LOG_DEBUG != 0 || _l <= LOGL_INFO || \ + (_DEBUG && _l == LOGL_DEBUG)) \ + printf(_fmt, ##_args); \ + }) #define log_buffer(_cat, _level, _addr, _data, _width, _count, _linelen) ({ \ int _l = _level; \ diff --git a/test/log/Makefile b/test/log/Makefile index 09f8689d07..08eea70e34 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -20,6 +20,7 @@ obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o obj-y += pr_cont_test.o else obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o +obj-$(CONFIG_CONSOLE_RECORD) += nolog_ndebug.o endif endif # CONFIG_UT_LOG diff --git a/test/log/nolog_ndebug.c b/test/log/nolog_ndebug.c new file mode 100644 index 0000000000..bd9a4f408e --- /dev/null +++ b/test/log/nolog_ndebug.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2021 Google LLC + * + * Logging function tests for CONFIG_LOG=n without #define DEBUG + */ + +#include <common.h> +#include <console.h> +#include <log.h> +#include <asm/global_data.h> +#include <test/log.h> +#include <test/ut.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define BUFFSIZE 32 + +static int log_test_log_disabled_ndebug(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + int i; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + + /* Output a log record at every level */ + for (i = LOGL_EMERG; i < LOGL_COUNT; i++) + log(LOGC_NONE, i, "testing level %i\n", i); + gd->flags &= ~GD_FLG_RECORD; + + /* Since DEBUG is not defined, we expect to not get debug output */ + for (i = LOGL_EMERG; i < LOGL_DEBUG; i++) + ut_assertok(ut_check_console_line(uts, "testing level %d", i)); + ut_assertok(ut_check_console_end(uts)); + + return 0; +} +LOG_TEST(log_test_log_disabled_ndebug); diff --git a/test/log/nolog_test.c b/test/log/nolog_test.c index cb4fb3db9a..4e52e5bed8 100644 --- a/test/log/nolog_test.c +++ b/test/log/nolog_test.c @@ -10,6 +10,7 @@ #include <common.h> #include <console.h> +#include <log.h> #include <asm/global_data.h> #include <test/log.h> #include <test/test.h> @@ -128,8 +129,10 @@ static int log_test_nolog_debug(struct unit_test_state *uts) memset(buf, 0, BUFFSIZE); console_record_reset_enable(); log_debug("testing %s\n", "log_debug"); + log(LOGC_NONE, LOGL_DEBUG, "more %s\n", "log_debug"); gd->flags &= ~GD_FLG_RECORD; ut_assertok(ut_check_console_line(uts, "testing log_debug")); + ut_assertok(ut_check_console_line(uts, "more log_debug")); ut_assertok(ut_check_console_end(uts)); return 0; }