[PATCH v1 1/2] hexdump: Introduce debug APIs

[Heinrich Schuchardt]

On 3/30/22 11:30, Andy Shevchenko wrote:
> On Sat, Nov 13, 2021 at 11:14:28AM -0700, Simon Glass wrote:
>> On Tue, 9 Nov 2021 at 05:03, Andy Shevchenko
>> <andriy.shevchenko at linux.intel.com> wrote:
>>>
>>> debug_hex_dump() and debug_hex_dump_bytes() conditionally print
>>> the dump based on DEBUG definition.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
>>> ---
>>>   include/hexdump.h |  4 ++++
>>>   lib/hexdump.c     | 60 ++++++++++++++++++++++++++++++++++++++++-------
>>>   2 files changed, 56 insertions(+), 8 deletions(-)
>>
>> +Heinrich Schuchardt
>
> Heinrich, it's several months passed. What should we do?

The logging system was designed by Simon.

>
>> Is it possible to resolve this in the header file? It is a bit odd to
>> have to add DEBUG to hexdump.c in order to get debug output.
>>
>> Note also the logging system so you can do
>>
>> log(LOG_CATEGORY, dbg ? LOGL_DEBUG : LOGL_INFO, "fmt ...", ...)
>


I would do something like below.

Then you can use the log command to switch debug output on and off.

Best regards

Heinrich


--- a/include/hexdump.h
+++ b/include/hexdump.h
@@ -148,6 +148,19 @@ int hex_dump_to_buffer(const void *buf, size_t len,
int rowsize, int groupsize,
  int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize,
  		   int groupsize, const void *buf, size_t len, bool ascii);

+
+int log_hex_dump(enum log_category_t cat, enum log_level_t level,
+		 const char *prefix_str, int prefix_type, int rowsize,
+		 int groupsize, const void *buf, size_t len, bool ascii);
+
+#define print_hex_dump(args...) ({ \
+	log_hex_dump(LOG_CATEGORY, LOGL_INFO, args); \
+	})
+
+#define debug_hex_dump(args...) ({ \
+	log_hex_dump(LOG_CATEGORY, LOGL_DEBUG, args); \
+	})
+
  /**
   * print_hex_dump_bytes - shorthand form of print_hex_dump() with
default params
   * @prefix_str: string to prefix each line with;
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 149c93ead8..eb77a9cf94 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -10,6 +10,7 @@

  #include <common.h>
  #include <hexdump.h>
+#include <log.h>
  #include <mapmem.h>
  #include <linux/ctype.h>
  #include <linux/compat.h>
@@ -125,8 +126,9 @@ overflow1:
  	return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
  }

-int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize,
-		   int groupsize, const void *buf, size_t len, bool ascii)
+int log_hex_dump(enum log_category_t cat, enum log_level_t level,
+		 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;
@@ -146,15 +148,17 @@ int print_hex_dump(const char *prefix_str, int
prefix_type, int rowsize,

  		switch (prefix_type) {
  		case DUMP_PREFIX_ADDRESS:
-			printf("%s%0*lx: %s\n", prefix_str,
-			       IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
-			       (ulong)map_to_sysmem(ptr) + i, linebuf);
+			log(cat, level,
+			    "%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);
+			log(cat, level,
+			    "%s%.8x: %s\n", prefix_str, i, linebuf);
  			break;
  		default:
-			printf("%s%s\n", prefix_str, linebuf);
+			log(cat, level, "%s%s\n", prefix_str, linebuf);
  			break;
  		}
  		if (!IS_ENABLED(CONFIG_SPL_BUILD) && ctrlc())