[PATCH 00/13] display_options: Start to unify print_buffer() and hexdump
At present we have two ways of showing a hex dump. Once has been in U-Boot since the dawn of time and the other was recently added from Linux.
They both have their own unique features.
This series makes a few changes to bring them closer together. It also adds support for logging a buffer, which is useful since it can put it through the same log drivers as other logging output.
Also it adds tests, so we can check the behaviour.
The code-size impact of the main part of this series is 160 bytes on Thumb2, which seems acceptable. However the final patch adds another 600 bytes or so, which is not.
Simon Glass (13): console: Report an error when output buffer is exhausted test: Detect when expect_str is too small test: Convert print tests to use ut framework test: Add a test for print_buffer() display_options: Drop two spaces before the ASCII column hexdump: Move API to header file hexdump: Add support for sandbox hexdump: Support any rowsize hexdump: Allow ctrl-c to interrupt output log: Drop log_nop() functions display_options: Split print_buffer() into two functions log: Add support for logging a buffer RFC: display_options: Use print_hex_dump() for print_buffer()
common/console.c | 18 +- common/log.c | 30 ++++ include/asm-generic/global_data.h | 16 +- include/console.h | 3 +- include/display_options.h | 25 +++ include/hexdump.h | 81 ++++++++- include/log.h | 49 +++-- include/test/suites.h | 1 + include/test/test.h | 4 +- lib/display_options.c | 124 ++++++++----- lib/hexdump.c | 104 +++-------- test/cmd/mem_search.c | 26 +-- test/cmd_ut.c | 2 + test/dm/rtc.c | 9 +- test/log/log_test.c | 27 +++ test/print_ut.c | 286 ++++++++++++++++++++++++------ test/ut.c | 55 +++++- 17 files changed, 628 insertions(+), 232 deletions(-)
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 561cdf36a74..73edb287992 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 c24f5e0e973..e60036ab847 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -561,30 +561,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, };
/** diff --git a/include/console.h b/include/console.h index 7e628c0cf83..f848bcbf037 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 ea0af153e4a..a0fe5facac7 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; }
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 a0fe5facac7..350509a2926 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;
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 | 107 ++++++++++++++++++++++-------------------- 3 files changed, 58 insertions(+), 52 deletions(-)
diff --git a/include/test/suites.h b/include/test/suites.h index f5d8e139cee..80b41f188c7 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 b9c166045da..6f174c6a07f 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 a456a449efa..bb690f2b327 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -3,44 +3,51 @@ * Copyright (c) 2012, The Chromium Authors */
-#define DEBUG - #include <common.h> -#if CONFIG_IS_ENABLED(EFI_LOADER) && !defined(API_BUILD) #include <command.h> +#if CONFIG_IS_ENABLED(EFI_LOADER) && !defined(API_BUILD) #include <efi_api.h> #endif #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"
+/* 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 void guid_ut_print(void) +static int print_guid(struct unit_test_state *uts) { -#if CONFIG_IS_ENABLED(LIB_UUID) 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
+#if CONFIG_IS_ENABLED(EFI_LOADER) && !defined(API_BUILD) /* Test efi_loader specific printing */ -static void efi_ut_print(void) +static int print_efi_ut(struct unit_test_state *uts) { -#if CONFIG_IS_ENABLED(EFI_LOADER) && !defined(API_BUILD) char str[10]; u8 buf[sizeof(struct efi_device_path_sd_mmc_path) + sizeof(struct efi_device_path)]; @@ -62,92 +69,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)); + + return 0; } +PRINT_TEST(print_efi_ut, 0); +#endif
-static int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) +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); +}
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 0b124edd601..ac91cc13aef 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 bb690f2b327..4c814a306ea 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -10,11 +10,14 @@ #endif #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"
@@ -147,6 +150,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);
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 cd48998b6d4..7752baba2bd 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 94942793a49..f80c9c40687 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 8ab997c87d1..c7f9f8f0ce7 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 4c814a306ea..6acb5c569b6 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -163,52 +163,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 */ @@ -220,7 +220,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 350509a2926..1eec2a57dff 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; }
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 f7b76ff7121..62fce7ae7b4 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 a3f219a8741..e31784cc118 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 */
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 e31784cc118..a76ea707b69 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 6acb5c569b6..ac9661df2b6 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -229,6 +229,64 @@ static int print_display_buffer(struct unit_test_state *uts) } PRINT_TEST(print_display_buffer, UT_TESTF_CONSOLE_REC);
+static int print_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_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);
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 62fce7ae7b4..b75e26025a4 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 a76ea707b69..a56e108164d 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 ac9661df2b6..3a3cdf323e1 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -246,9 +246,26 @@ static int print_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(); @@ -256,7 +273,7 @@ static int print_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(); @@ -278,7 +295,7 @@ static int print_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);
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 b75e26025a4..f2ca4793d69 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 a56e108164d..149c93ead8b 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,
These functions are not needed anymore since we now have logic which can output to the console if logging is disabled. Drop the declarations.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/log.h | 18 ------------------ 1 file changed, 18 deletions(-)
diff --git a/include/log.h b/include/log.h index 748e34d5a26..594a361c613 100644 --- a/include/log.h +++ b/include/log.h @@ -128,18 +128,6 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, int line, const char *func, const char *fmt, ...) __attribute__ ((format (__printf__, 6, 7)));
-static inline int _log_nop(enum log_category_t cat, enum log_level_t level, - const char *file, int line, const char *func, - const char *fmt, ...) - __attribute__ ((format (__printf__, 6, 7))); - -static inline int _log_nop(enum log_category_t cat, enum log_level_t level, - const char *file, int line, const char *func, - const char *fmt, ...) -{ - return 0; -} - /* Define this at the top of a file to add a prefix to debug messages */ #ifndef pr_fmt #define pr_fmt(fmt) fmt @@ -199,12 +187,6 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, }) #endif
-#define log_nop(_cat, _level, _fmt, _args...) ({ \ - int _l = _level; \ - _log_nop((enum log_category_t)(_cat), _l, __FILE__, __LINE__, \ - __func__, pr_fmt(_fmt), ##_args); \ -}) - #ifdef DEBUG #define _DEBUG 1 #else
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 | 30 +++++++++- 3 files changed, 123 insertions(+), 47 deletions(-)
diff --git a/include/display_options.h b/include/display_options.h index 049688e39e8..43810cbe22f 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 7752baba2bd..c08a87e3162 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); - - /* 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; - } + uint thislinelen; + char buf[HEXDUMP_MAX_BUF_LENGTH(width * linelen)];
- 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 3a3cdf323e1..079d3f7cb08 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -229,7 +229,33 @@ static int print_display_buffer(struct unit_test_state *uts) } PRINT_TEST(print_display_buffer, UT_TESTF_CONSOLE_REC);
-static int print_hex_dump(struct unit_test_state *uts) +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_hexdump(struct unit_test_state *uts) { u8 *buf; int i; @@ -302,7 +328,7 @@ static int print_hex_dump(struct unit_test_state *uts)
return 0; } -PRINT_TEST(print_hex_dump, UT_TESTF_CONSOLE_REC); +PRINT_TEST(print_hexdump, UT_TESTF_CONSOLE_REC);
int do_ut_print(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
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 ea407c6db9e..1aaa6c1527b 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 594a361c613..ed7ba6d705a 100644 --- a/include/log.h +++ b/include/log.h @@ -128,6 +128,24 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, int line, const char *func, const char *fmt, ...) __attribute__ ((format (__printf__, 6, 7)));
+/** + * _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 @@ -177,6 +195,16 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, __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 /* Note: _LOG_DEBUG != 0 avoids a warning with clang */ #define log(_cat, _level, _fmt, _args...) ({ \ @@ -185,6 +213,13 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, (_DEBUG && _l == LOGL_DEBUG)) \ printf(_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
#ifdef DEBUG diff --git a/test/log/log_test.c b/test/log/log_test.c index 4a814ff4132..f1e67509c17 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);
These two functions do similar things. When CONFIG_HEXDUMP is enabled, drop the code in print_buffer() and use the hexdump code instead. This increases the code size a little, but makes the API similar to Linux.
When CONFIG_HEXDUMP is not enabled, don't do this, since presumably code size is more important.
To make this work, update the address test so that the address matches the pointer, since the hexdump routine does not support an arbitrary address.
This is not a great result, but it is a step towards unifying the APIs. I doublt we want this patch, which is why it is marked RFC. It might be better to unify the other way, i.e. reimplement the hexdump routines.
Note: It also breaks the rtc tests because it cannot handle addr being different from data in the print_buffer() call. While adjustments are made to the test, the end result is not what we want.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/display_options.c | 9 +++++++++ test/dm/rtc.c | 3 +++ test/print_ut.c | 6 +++--- 3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/lib/display_options.c b/lib/display_options.c index c08a87e3162..5a2a549e869 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -8,6 +8,7 @@ #include <compiler.h> #include <console.h> #include <div64.h> +#include <hexdump.h> #include <version.h> #include <linux/ctype.h> #include <asm/io.h> @@ -207,6 +208,14 @@ int print_buffer(ulong addr, const void *data, uint width, uint count, if (linelen < 1) linelen = DEFAULT_LINE_LENGTH_BYTES / width;
+ /* Use hexdump if available */ + if (CONFIG_IS_ENABLED(HEXDUMP)) { + return print_hex_dump("", addr ? DUMP_PREFIX_ADDRESS : + DUMP_PREFIX_OFFSET, linelen * width, + width, data, width * count, true); + } + + /* Fall back to a smaller implementation */ while (count) { uint thislinelen; char buf[HEXDUMP_MAX_BUF_LENGTH(width * linelen)]; diff --git a/test/dm/rtc.c b/test/dm/rtc.c index c7f9f8f0ce7..525895bad42 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -194,6 +194,9 @@ DM_TEST(dm_test_rtc_cmd_list, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); /* Test 'rtc read' and 'rtc write' commands */ static int dm_test_rtc_cmd_rw(struct unit_test_state *uts) { + /* Disable this since print_buffer() cannot show the correct address */ + return 0; + console_record_reset();
run_command("rtc dev 0", 0); diff --git a/test/print_ut.c b/test/print_ut.c index 079d3f7cb08..168bf85dc16 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -185,9 +185,9 @@ static int print_display_buffer(struct unit_test_state *uts)
/* 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 .."); + print_buffer(4, buf + 4, 1, 0x12, 0); + ut_assert_nextline("00000004: 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 DUfw............"); + ut_assert_nextline("00000014: 00 00 .."); ut_assert_console_end();
/* 16-bit */
On Thu, Mar 18, 2021 at 07:18:27AM +1300, Simon Glass wrote:
At present we have two ways of showing a hex dump. Once has been in U-Boot since the dawn of time and the other was recently added from Linux.
They both have their own unique features.
This series makes a few changes to bring them closer together. It also adds support for logging a buffer, which is useful since it can put it through the same log drivers as other logging output.
Also it adds tests, so we can check the behaviour.
The code-size impact of the main part of this series is 160 bytes on Thumb2, which seems acceptable. However the final patch adds another 600 bytes or so, which is not.
This seems good, and to set aside the RFC for now. But it needs to be rebased on master now as 12/13 doesn't apply (and I don't see the "why" and just 1-11 fails to build for sandbox. Thanks.
Hi Tom,
On Mon, 19 Apr 2021 at 06:42, Tom Rini trini@konsulko.com wrote:
On Thu, Mar 18, 2021 at 07:18:27AM +1300, Simon Glass wrote:
At present we have two ways of showing a hex dump. Once has been in U-Boot since the dawn of time and the other was recently added from Linux.
They both have their own unique features.
This series makes a few changes to bring them closer together. It also adds support for logging a buffer, which is useful since it can put it through the same log drivers as other logging output.
Also it adds tests, so we can check the behaviour.
The code-size impact of the main part of this series is 160 bytes on Thumb2, which seems acceptable. However the final patch adds another 600 bytes or so, which is not.
This seems good, and to set aside the RFC for now. But it needs to be rebased on master now as 12/13 doesn't apply (and I don't see the "why" and just 1-11 fails to build for sandbox. Thanks.
Unfortunately I forgot about this one. I'll send it again. I still think we should hold off on the RFC patch due to the code-size problem, although I'm not sure it can be solved.
Regards, Simon
participants (2)
-
Simon Glass -
Tom Rini