[PATCH v3 0/3] hexdump: Allow skipping identical lines

[PATCH v3 0/3] hexdump: Allow skipping identical lines

[Miquel Raynal]

While working on NAND issues, I used print_hex_dump() a lot to compare
data. But I am mostly working on embedded systems where the kernel
messages go through a serial console. Sometimes network support is an
option, sometimes not. Anyway, I often print buffers both in kernel
space and user space to compare them, and they may be full of 0's or
1's, which means lines are repeated a lot in the output and this is slow
*and* hard to compare.

I initially hacked into lib/hexdump.c for my own purpose and just
discarded all the other users, but it felt like this might be a useful
feature for others and decided to make it a public patch.

* First patch creates a helper named print_hex() with a prototype
  containing a flags parameter, instead of an entry for the prefix type
  and another entry for the ascii boolean. print_hex_dump() now
  falls back to print_hex().
* Second patch adds a new flag to possibly skip the identical lines.
* Third patch is optional, and adds the offset at which the dump
  finishes in case the last line to be printed would be skipped.

The Cc-list, as provided by get_maintainers.pl, was returning 330
e-mail addresses which felt to much, so I ran the script only on the
second patch (the printk/includes/debug/Doc changes). It gave this
Cc-list which sounds more reasonable. Hopefully this is a smart move,
otherwise let me know what you think would be best.

---
Changes in v3:
- Drop the tree-wide changes.
- Create a print_hex() helper with the new prototype discussed with
  David and Petr. Create a fallback from print_hex_dump() to
  print_hex().
- Include a new patch to print the final offset if the previous lines
  where skipped.
- Fix the typo reported by Randy.
- Link to v2: https://lore.kernel.org/r/20250110-perso-hexdump-v2-0-7f9a6a799170@bootlin.com

Changes in v2:
- Rebased on v6.13-rc1.
- Manually fixed the diff in many places to fit Andy's requests.
- Added a real life example (code diff and output diff) with the
  modification of the API as well as the use of the new flag introduced
  by this series in the cover letter (at the bottom) as requested by
  Andy.
- Link to v1: https://lore.kernel.org/r/20240826162416.74501-1-miquel.raynal@bootlin.com
---

Here is a typical diff showing the code change with a perfectly equivalent
output:

 print_hex_dump_debug("", DUMP_PREFIX_OFFSET, 32, 1, spinand->databuf, mtd->writesize,
-                     false);
+                     0);

Here is a typical output of a NAND buffer without the new 'skip' flag,
ie. with the above code snippet:

00000000: 55 42 49 23 01 00 00 00 00 00 00 00 00 00 00 01 00 00 08 00 00 00 10 00 2b 10 f1 92 00 00 00 00
00000020: 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 db 93 e9 fc
00000040: 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
00000060: 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
00000080: 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
000000a0: 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
000000c0: 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
000000e0: 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
00000100: 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
00000120: 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
00000140: 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
00000160: 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
00000180: 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
000001a0: 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
000001c0: 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
000001e0: 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
00000200: 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
00000220: 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
00000240: 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
00000260: 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
00000280: 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
000002a0: 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
000002c0: 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
000002e0: 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
00000300: 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
00000320: 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
00000340: 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
00000360: 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
00000380: 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
000003a0: 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
000003c0: 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
000003e0: 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
00000400: 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
00000420: 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
00000440: 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
00000460: 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
00000480: 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
000004a0: 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
000004c0: 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
000004e0: 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
00000500: 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
00000520: 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
00000540: 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
00000560: 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
00000580: 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
000005a0: 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
000005c0: 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
000005e0: 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
00000600: 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
00000620: 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
00000640: 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
00000660: 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
00000680: 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
000006a0: 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
000006c0: 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
000006e0: 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
00000700: 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
00000720: 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
00000740: 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
00000760: 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
00000780: 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
000007a0: 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
000007c0: 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
000007e0: 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

And with the new flag added the code looks like this:

-print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 32, 1, spinand->databuf, mtd->writesize, 0);
+print_hex(KERN_ERR, "", 32, 1, spinand->databuf, mtd->writesize,
+          DUMP_PREFIX_OFFSET | DUMP_FLAG_SKIP_IDENTICAL_LINES);

And the output is easier to parse and also faster to show on a serial
console:

00000000: 55 42 49 23 01 00 00 00 00 00 00 00 00 00 00 01 00 00 08 00 00 00 10 00 2b 10 f1 92 00 00 00 00
00000020: 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 db 93 e9 fc
00000040: 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
*

00000800

---
Miquel Raynal (3):
      hexdump: Simplify print_hex_dump()
      hexdump: Allow skipping identical lines
      hexdump: Print the prefix after the last line to show the dump is over

 Documentation/core-api/printk-formats.rst |  4 +-
 include/linux/printk.h                    | 43 +++++++++++++----
 lib/hexdump.c                             | 76 +++++++++++++++++++------------
 3 files changed, 85 insertions(+), 38 deletions(-)
---
base-commit: d5d7d26716afd36b8c2a49a3265fff0b16e3694b
change-id: 20241224-perso-hexdump-7a008b5053a2

Best regards,
-- 
Miquel Raynal <miquel.raynal@bootlin.com>

[PATCH v3 1/3] hexdump: Simplify print_hex_dump()

[Miquel Raynal]

print_hex_dump() already has numerous parameters, and could be extended
with a new one. Adding new parameters is super painful due to the number
of users, and it makes the function calls even longer.

Create a print_hex() to replace print_hex_dump(), with 'prefix_type' and
'ascii' being merged into a 'dump_flags' parameter. This way extending
the list of dump flags will be much easier.

For convenience, a print_hex_dump macro is created to fallback on the
print_hex() implementation. A tree-wide change to remove its use could
be done in the future.

No functional change intended.

Suggested-by: Petr Mladek <pmladek@suse.com>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/core-api/printk-formats.rst |  2 +-
 include/linux/printk.h                    | 42 +++++++++++++++++++++++-------
 lib/hexdump.c                             | 43 ++++++++++++++-----------------
 3 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473da9c10f45f2464566f690472c61401e..f80b5e262e9933992d1291f1d78fba97589d5631 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -310,7 +310,7 @@ Raw buffer as a hex string
 
 For printing small buffers (up to 64 bytes long) as a hex string with a
 certain separator. For larger buffers consider using
-:c:func:`print_hex_dump`.
+:c:func:`print_hex`.
 
 MAC/FDDI addresses
 ------------------
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 4217a9f412b265f1dc027be88eff7f5a0e0e4aab..7dca2270c82c0ed788cd706274f1c1b14ed9a7fe 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -747,22 +747,31 @@ do {									\
 
 extern const struct file_operations kmsg_fops;
 
+/*
+ * Dump flags for print_hex().
+ * DUMP_PREFIX_{NONE,ADDRESS,OFFSET} are mutually exclusive.
+ */
 enum {
-	DUMP_PREFIX_NONE,
-	DUMP_PREFIX_ADDRESS,
-	DUMP_PREFIX_OFFSET
+	DUMP_HEX_DATA = 0,
+	DUMP_ASCII = BIT(0),
+	DUMP_PREFIX_NONE = 0, /* Legacy definition for print_hex_dump() */
+	DUMP_PREFIX_ADDRESS = BIT(1),
+	DUMP_PREFIX_OFFSET = BIT(2),
 };
+
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			      int groupsize, char *linebuf, size_t linebuflen,
 			      bool ascii);
 #ifdef CONFIG_PRINTK
-extern void print_hex_dump(const char *level, const char *prefix_str,
-			   int prefix_type, int rowsize, int groupsize,
-			   const void *buf, size_t len, bool ascii);
+extern void print_hex(const char *level, const char *prefix_str,
+		      int rowsize, int groupsize,
+		      const void *buf, size_t len,
+		      unsigned int dump_flags);
 #else
-static inline void print_hex_dump(const char *level, const char *prefix_str,
-				  int prefix_type, int rowsize, int groupsize,
-				  const void *buf, size_t len, bool ascii)
+static inline void print_hex(const char *level, const char *prefix_str,
+			     int rowsize, int groupsize,
+			     const void *buf, size_t len,
+			     unsigned int dump_flags)
 {
 }
 static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
@@ -791,6 +800,21 @@ static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
 }
 #endif
 
+/*
+ * print_hex_dump - legacy version of print_hex() with a longer parameter list
+ *
+ * Refer to print_hex() for the parameters definition which are identical except:
+ * - prefix_type: controls whether prefix of an offset, address, or none
+ * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE).
+ * This parameter has been removed in favor of a common 'flags' parameter.
+ * - ascii: include ASCII after the hex output.
+ * This parameter has been removed in favor of a common 'flags' parameter.
+ */
+
+#define print_hex_dump(level, prefix_str, prefix_type, rowsize, groupsize, buf, len, ascii) \
+	print_hex(level, prefix_str, rowsize, groupsize, buf, len, \
+		  (prefix_type) | ((ascii) ? DUMP_ASCII : DUMP_HEX_DATA))
+
 /**
  * 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 c3db7c3a764365b01e78f8ed0b7f782f33a5ce34..74fdcb4566d27f257a0e1288c261d81d231b06bf 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -228,39 +228,39 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
 
 #ifdef CONFIG_PRINTK
 /**
- * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * print_hex - print a text hex dump to syslog for a binary blob of data
  * @level: kernel log level (e.g. KERN_DEBUG)
  * @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
+ * @dump_flags: controls the output format, typically:
+ *   - %DUMP_PREFIX_OFFSET shows the offset in front of each line
+ *   - %DUMP_PREFIX_ADDRESS shows the address in front of each line
+ *   - %DUMP_ASCII prints the ascii equivalent after the hex output
  *
- * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
+ * Given a buffer of u8 data, print_hex() prints a hex + ASCII dump
  * to the kernel log at the specified kernel log level, with an optional
  * leading prefix.
  *
- * print_hex_dump() works on one "line" of output at a time, i.e.,
+ * print_hex() 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
+ * print_hex() iterates over the entire input @buf, breaking it into
  * "line size" chunks to format and print.
  *
  * E.g.:
- *   print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
- *		    16, 1, frame->data, frame->len, true);
+ *   print_hex(KERN_DEBUG, "raw data: ", 16, 1, frame->data, frame->len,
+ *             DUMP_PREFIX_ADDRESS | DUMP_ASCII);
  *
  * 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 *level, const char *prefix_str, int prefix_type,
-		    int rowsize, int groupsize,
-		    const void *buf, size_t len, bool ascii)
+void print_hex(const char *level, const char *prefix_str, int rowsize, int groupsize,
+	       const void *buf, size_t len, unsigned int dump_flags)
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
@@ -274,22 +274,17 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		remaining -= rowsize;
 
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-				   linebuf, sizeof(linebuf), ascii);
+				   linebuf, sizeof(linebuf),
+				   dump_flags & DUMP_ASCII);
 
-		switch (prefix_type) {
-		case DUMP_PREFIX_ADDRESS:
-			printk("%s%s%p: %s\n",
-			       level, prefix_str, ptr + i, linebuf);
-			break;
-		case DUMP_PREFIX_OFFSET:
+		if (dump_flags & DUMP_PREFIX_ADDRESS)
+			printk("%s%s%p: %s\n", level, prefix_str, ptr + i, linebuf);
+		else if (dump_flags & DUMP_PREFIX_OFFSET)
 			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
-			break;
-		default:
+		else
 			printk("%s%s%s\n", level, prefix_str, linebuf);
-			break;
-		}
 	}
 }
-EXPORT_SYMBOL(print_hex_dump);
+EXPORT_SYMBOL(print_hex);
 
 #endif /* defined(CONFIG_PRINTK) */

-- 
2.48.1

Re: [PATCH v3 1/3] hexdump: Simplify print_hex_dump()

[Andy Shevchenko]

On Wed, Mar 19, 2025 at 05:08:10PM +0100, Miquel Raynal wrote:
> print_hex_dump() already has numerous parameters, and could be extended
> with a new one. Adding new parameters is super painful due to the number
> of users, and it makes the function calls even longer.
> 
> Create a print_hex() to replace print_hex_dump(), with 'prefix_type' and
> 'ascii' being merged into a 'dump_flags' parameter. This way extending
> the list of dump flags will be much easier.
> 
> For convenience, a print_hex_dump macro is created to fallback on the

print_hex_dump()

> print_hex() implementation. A tree-wide change to remove its use could
> be done in the future.
> 
> No functional change intended.

...

>  For printing small buffers (up to 64 bytes long) as a hex string with a
>  certain separator. For larger buffers consider using
> -:c:func:`print_hex_dump`.
> +:c:func:`print_hex`.

Why replacement? I would rather expect

:c:func:`print_hex_dump` or :c:func:`print_hex` depending on your needs.

...

> +/*
> + * Dump flags for print_hex().
> + * DUMP_PREFIX_{NONE,ADDRESS,OFFSET} are mutually exclusive.

This is confusing, taking into account two definitions to 0.
> + */
>  enum {
> +	DUMP_HEX_DATA = 0,
> +	DUMP_ASCII = BIT(0),
> +	DUMP_PREFIX_NONE = 0, /* Legacy definition for print_hex_dump() */
> +	DUMP_PREFIX_ADDRESS = BIT(1),
> +	DUMP_PREFIX_OFFSET = BIT(2),
>  };

Can we rather add a new enum and leave this untouched?

Also you can use bit mask and two bits for the value:

	DUMP_PREFIX_MASK = GENMASK(1, 0)

and no need to have the above comment about exclusiveness and no need to change
the values.

...

> +extern void print_hex(const char *level, const char *prefix_str,
> +		      int rowsize, int groupsize,
> +		      const void *buf, size_t len,
> +		      unsigned int dump_flags);

> +static inline void print_hex(const char *level, const char *prefix_str,
> +			     int rowsize, int groupsize,
> +			     const void *buf, size_t len,
> +			     unsigned int dump_flags)

Hmm... Wouldn't you want to have a enum as a last parameter?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/3] hexdump: Simplify print_hex_dump()

[Miquel Raynal]

On 19/03/2025 at 18:37:26 +02, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Mar 19, 2025 at 05:08:10PM +0100, Miquel Raynal wrote:
>> print_hex_dump() already has numerous parameters, and could be extended
>> with a new one. Adding new parameters is super painful due to the number
>> of users, and it makes the function calls even longer.
>> 
>> Create a print_hex() to replace print_hex_dump(), with 'prefix_type' and
>> 'ascii' being merged into a 'dump_flags' parameter. This way extending
>> the list of dump flags will be much easier.
>> 
>> For convenience, a print_hex_dump macro is created to fallback on the
>
> print_hex_dump()

It is a macro, not a function, so I don't feel bothered by the absence
of parenthesis. Anyway, that's really a nitpick, so if you want, I'll
add them.

>> print_hex() implementation. A tree-wide change to remove its use could
>> be done in the future.
>> 
>> No functional change intended.
>
> ...
>
>>  For printing small buffers (up to 64 bytes long) as a hex string with a
>>  certain separator. For larger buffers consider using
>> -:c:func:`print_hex_dump`.
>> +:c:func:`print_hex`.
>
> Why replacement? I would rather expect

Because it is a replacement. I initially wanted a tree-wide change but
it is too heavy and painful to carry. So I am replacing print_hex_dump()
by print_hex() as it was discussed in v2, but keeping print_hex_dump()
possible. In practice it is a handy fallback on print_hex(), nothing
else.

> :c:func:`print_hex_dump` or :c:func:`print_hex` depending on your
> needs.

There is no need print_hex_dump() fills and print_hex() does not. It
is actually the opposite. We no longer need print_hex_dump().

>
> ...
>
>> +/*
>> + * Dump flags for print_hex().
>> + * DUMP_PREFIX_{NONE,ADDRESS,OFFSET} are mutually exclusive.
>
> This is confusing, taking into account two definitions to 0.
>> + */
>>  enum {
>> +	DUMP_HEX_DATA = 0,
>> +	DUMP_ASCII = BIT(0),
>> +	DUMP_PREFIX_NONE = 0, /* Legacy definition for print_hex_dump() */
>> +	DUMP_PREFIX_ADDRESS = BIT(1),
>> +	DUMP_PREFIX_OFFSET = BIT(2),
>>  };
>
> Can we rather add a new enum and leave this untouched?

No, because the DUMP_PREFIX_ADDRESS/OFFSET are needed in
both. DUMP_PREFIX_NONE is no longer really needed, that's why I mark it
legacy with a comment, it's presence or absence no longer matters with
print_hex().

> Also you can use bit mask and two bits for the value:
>
> 	DUMP_PREFIX_MASK = GENMASK(1, 0)

Why? What is the use case?

> and no need to have the above comment about exclusiveness and no need to change
> the values.

Exclusiveness has always been there, just look at the code, I'm not
adding anything new. Refusing to change values for an enumeration is
totally pointless, it has no impact, no cost, no consequence. I don't
see your point.

>
> ...
>
>> +extern void print_hex(const char *level, const char *prefix_str,
>> +		      int rowsize, int groupsize,
>> +		      const void *buf, size_t len,
>> +		      unsigned int dump_flags);
>
>> +static inline void print_hex(const char *level, const char *prefix_str,
>> +			     int rowsize, int groupsize,
>> +			     const void *buf, size_t len,
>> +			     unsigned int dump_flags)
>
> Hmm... Wouldn't you want to have a enum as a last parameter?

And this has already been discussed in v2, we need to pass multiple
flags and decided to go for an unsigned int|long, I do not think the
compiler will like it otherwise.

Regards,
Miquèl

[PATCH v3 2/3] hexdump: Allow skipping identical lines

[Miquel Raynal]

When dumping long buffers (especially for debug purposes) it may be very
convenient to sometimes avoid spitting all the lines of the buffer if
the lines are identical. Typically on embedded devices, the console
would be wired to a UART running at 115200 bauds, which makes the dumps
very (very) slow. In this case, having a flag to avoid printing
duplicated lines is handy.

Example of a made up repetitive output:
0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb

Same but with the flag enabled:
0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
*
ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/core-api/printk-formats.rst |  4 +++-
 include/linux/printk.h                    |  1 +
 lib/hexdump.c                             | 20 +++++++++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index f80b5e262e9933992d1291f1d78fba97589d5631..820f92c65dc64e7d24af5c0031ee8c8d6bb0f931 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -310,7 +310,9 @@ Raw buffer as a hex string
 
 For printing small buffers (up to 64 bytes long) as a hex string with a
 certain separator. For larger buffers consider using
-:c:func:`print_hex`.
+:c:func:`print_hex`, especially since duplicated lines can be
+skipped automatically to reduce the overhead with the
+``DUMP_SKIP_IDENTICAL_LINES`` flag.
 
 MAC/FDDI addresses
 ------------------
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 7dca2270c82c0ed788cd706274f1c1b14ed9a7fe..d9e3e4b0bab8d3ff6a49600abbdbc9b1e6320a60 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -757,6 +757,7 @@ enum {
 	DUMP_PREFIX_NONE = 0, /* Legacy definition for print_hex_dump() */
 	DUMP_PREFIX_ADDRESS = BIT(1),
 	DUMP_PREFIX_OFFSET = BIT(2),
+	DUMP_SKIP_IDENTICAL_LINES = BIT(3),
 };
 
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 74fdcb4566d27f257a0e1288c261d81d231b06bf..f0d1a7f1ce817fd53a7ffd259fbe9b9c8348db16 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -8,6 +8,7 @@
 #include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/minmax.h>
+#include <linux/string.h>
 #include <linux/export.h>
 #include <linux/unaligned.h>
 
@@ -240,6 +241,8 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  *   - %DUMP_PREFIX_OFFSET shows the offset in front of each line
  *   - %DUMP_PREFIX_ADDRESS shows the address in front of each line
  *   - %DUMP_ASCII prints the ascii equivalent after the hex output
+ *   - %DUMP_SKIP_IDENTICAL_LINES will display a single '*' instead of
+ *     duplicated lines.
  *
  * Given a buffer of u8 data, print_hex() prints a hex + ASCII dump
  * to the kernel log at the specified kernel log level, with an optional
@@ -263,8 +266,9 @@ void print_hex(const char *level, const char *prefix_str, int rowsize, int group
 	       const void *buf, size_t len, unsigned int dump_flags)
 {
 	const u8 *ptr = buf;
-	int i, linelen, remaining = len;
+	int i, prev_i, linelen, remaining = len;
 	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+	bool same_line = false;
 
 	if (rowsize != 16 && rowsize != 32)
 		rowsize = 16;
@@ -273,6 +277,20 @@ void print_hex(const char *level, const char *prefix_str, int rowsize, int group
 		linelen = min(remaining, rowsize);
 		remaining -= rowsize;
 
+		if (dump_flags & DUMP_SKIP_IDENTICAL_LINES) {
+			if (i && !memcmp(ptr + i, ptr + prev_i, linelen)) {
+				prev_i = i;
+				if (same_line)
+					continue;
+				same_line = true;
+				printk("%s*\n", level);
+				continue;
+			} else {
+				prev_i = i;
+				same_line = false;
+			}
+		}
+
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
 				   linebuf, sizeof(linebuf),
 				   dump_flags & DUMP_ASCII);

-- 
2.48.1

Re: [PATCH v3 2/3] hexdump: Allow skipping identical lines

[Andy Shevchenko]

On Wed, Mar 19, 2025 at 05:08:11PM +0100, Miquel Raynal wrote:
> When dumping long buffers (especially for debug purposes) it may be very
> convenient to sometimes avoid spitting all the lines of the buffer if
> the lines are identical. Typically on embedded devices, the console
> would be wired to a UART running at 115200 bauds, which makes the dumps
> very (very) slow. In this case, having a flag to avoid printing
> duplicated lines is handy.
> 
> Example of a made up repetitive output:
> 0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb
> 
> Same but with the flag enabled:
> 0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> *
> ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb

...

>  For printing small buffers (up to 64 bytes long) as a hex string with a
>  certain separator. For larger buffers consider using
> -:c:func:`print_hex`.

Instead of fixing this (see also comment in previous patch), just add the text
like

:c:func:`print_hex` is  especially useful since duplicated lines can be skipped
automatically to reduce the overhead with the ``DUMP_SKIP_IDENTICAL_LINES`` flag.

> +:c:func:`print_hex`, especially since duplicated lines can be
> +skipped automatically to reduce the overhead with the
> +``DUMP_SKIP_IDENTICAL_LINES`` flag.

Also, can  we also put a sub name spaces to the flags, like for HEX/ASCII

DUMP_DATA_HEX
DUMP_DATA_ASCII

This SKIP  will start a new sub name space.

...

>  #include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/minmax.h>

> +#include <linux/string.h>
>  #include <linux/export.h>

It's more natural to put it here, with given context it makes more order
(speaking of alphabetical one).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] hexdump: Allow skipping identical lines

[Miquel Raynal]

>>  For printing small buffers (up to 64 bytes long) as a hex string with a
>>  certain separator. For larger buffers consider using
>> -:c:func:`print_hex`.
>
> Instead of fixing this (see also comment in previous patch), just add the text
> like
>
> :c:func:`print_hex` is  especially useful since duplicated lines can be skipped
> automatically to reduce the overhead with the
> ``DUMP_SKIP_IDENTICAL_LINES`` flag.

Why are you bothered here? I am sorry but this part of the review is
absolutely pointless. I have no words to emphasize how high the
nitpicking level is. Please refrain yourself.

>> +:c:func:`print_hex`, especially since duplicated lines can be
>> +skipped automatically to reduce the overhead with the
>> +``DUMP_SKIP_IDENTICAL_LINES`` flag.
>
> Also, can  we also put a sub name spaces to the flags, like for HEX/ASCII
>
> DUMP_DATA_HEX
> DUMP_DATA_ASCII

They just refer to the way the data is dumped, so "data" is in the name,
that's true. The fact that they share the same namespace is fortunate
but not super relevant either. I am following the hints discussed in v2
regarding the naming, I am not too attached to these, but they felt
correct, so I use them.

> This SKIP  will start a new sub name space.

Yes. And?

I would have probably chosen a common name space from day 1 if I was
creating these now, but they are already two enums named
"DUMP_PREFIX". I guess we all agree it would be stupid to prefix all
enums "DUMP_PREFIX" anyway? And because you disgusted me from attempting
brave tree-wide changes, I will not rename them.

>
> ...
>
>>  #include <linux/errno.h>
>>  #include <linux/kernel.h>
>>  #include <linux/minmax.h>
>
>> +#include <linux/string.h>
>>  #include <linux/export.h>
>
> It's more natural to put it here, with given context it makes more order
> (speaking of alphabetical one).

If I learned something on these mailing lists, it is that what is
natural for me might not be for others. The alphabetical order is not
respected. You already pointed that in your first review, so I chose
another place which felt more relevant... to me.
e, k, m, s, u. This is the alphabetical order. export.h is not at the
correct place, I am very sorry.

[PATCH v3 3/3] hexdump: Print the prefix after the last line to show the dump is over

[Miquel Raynal]

When skipping duplicated lines, the end of the log can just be a
star ("*") which may be confusing (?), so in order to clarify the end of
the log, tell the user how many lines where skipped and mimic the
userspace hexdump output, let's add in this case a new line with the
next offset.

With the following data:

00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000050: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000060: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000070: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................

When skipping identical lines we were seeing:

00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
*

And now we will have:

00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
*
00000080

However if the output was (first bit of first byte on last line changed):

00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000050: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000060: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
00000070: fe ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
           ^

When skipping identical lines we would see:

00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
*
00000070: fe ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................

And in this case the output would not change with this patch.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
This change is not related to the two first patches and is just an
addition that was requested by Andy who felt like it was not clear where
the dump was ending with the 'SKIP_DUPLICATE' flag when the last line
was skipped. Honestly this never bothered me, so depending on the
maintainers wishes, this can either be applied or skipped.
---
 lib/hexdump.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index f0d1a7f1ce817fd53a7ffd259fbe9b9c8348db16..c0c9a13838513bc7dec2ebdeadf622cd319ea4f8 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -302,6 +302,13 @@ void print_hex(const char *level, const char *prefix_str, int rowsize, int group
 		else
 			printk("%s%s%s\n", level, prefix_str, linebuf);
 	}
+
+	if (same_line) {
+		if (dump_flags & DUMP_PREFIX_ADDRESS)
+			printk("%s%s%p\n", level, prefix_str, ptr + i);
+		else if (dump_flags & DUMP_PREFIX_OFFSET)
+			printk("%s%s%.8x\n", level, prefix_str, i);
+	}
 }
 EXPORT_SYMBOL(print_hex);
 

-- 
2.48.1

Re: [PATCH v3 3/3] hexdump: Print the prefix after the last line to show the dump is over

[Andy Shevchenko]

On Wed, Mar 19, 2025 at 05:08:12PM +0100, Miquel Raynal wrote:
> When skipping duplicated lines, the end of the log can just be a
> star ("*") which may be confusing (?), so in order to clarify the end of
> the log, tell the user how many lines where skipped and mimic the
> userspace hexdump output, let's add in this case a new line with the
> next offset.
> 
> With the following data:
> 
> 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000050: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000060: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000070: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 
> When skipping identical lines we were seeing:
> 
> 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> *
> 
> And now we will have:
> 
> 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> *
> 00000080
> 
> However if the output was (first bit of first byte on last line changed):
> 
> 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000050: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000060: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 00000070: fe ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
>            ^
> 
> When skipping identical lines we would see:
> 
> 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> *
> 00000070: fe ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> 
> And in this case the output would not change with this patch.

Thanks!

LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> This change is not related to the two first patches and is just an
> addition that was requested by Andy who felt like it was not clear where
> the dump was ending with the 'SKIP_DUPLICATE' flag when the last line
> was skipped. Honestly this never bothered me, so depending on the
> maintainers wishes, this can either be applied or skipped.

I was also referring to userpsace `hexdump` behaviour. And I find it logical.

-- 
With Best Regards,
Andy Shevchenko