Re: [PATCH 1/3] hexdump: Implement macro for converting large buffers

On Sat, Feb 15, 2025 at 05:46:35PM +0000, David Laight wrote:

On Sat, 15 Feb 2025 17:40:39 +0000
David Laight [off-list ref] wrote:

quoted

On Sat, 15 Feb 2025 16:36:12 +0000
Simon Horman [off-list ref] wrote:

quoted

+ David Laight

On Fri, Feb 14, 2025 at 10:24:34AM -0600, Nick Child wrote:  

quoted

Define for_each_line_in_hex_dump which loops over a buffer and calls
hex_dump_to_buffer for each segment in the buffer. This allows the
caller to decide what to do with the resulting string and is not
limited by a specific printing format like print_hex_dump.

Signed-off-by: Nick Child <nnac123@linux.ibm.com>
---
 include/linux/printk.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 4217a9f412b2..559d4bfe0645 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h

@@ -755,6 +755,27 @@ enum {
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			      int groupsize, char *linebuf, size_t linebuflen,
 			      bool ascii);
+/**
+ * for_each_line_in_hex_dump - iterate over buffer, converting into hex ASCII
+ * @i: offset in @buff
+ * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @linebuf: where to put the converted data
+ * @linebuflen: total size of @linebuf, including space for terminating NUL
+ *		IOW >= (@rowsize * 2) + ((@rowsize - 1 / @groupsize)) + 1
+ * @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
+ */
+ #define for_each_line_in_hex_dump(i, rowsize, linebuf, linebuflen, groupsize, \
+				   buf, len) \
+	for ((i) = 0;							\
+	     (i) < (len) &&						\
+	     hex_dump_to_buffer((unsigned char *)(buf) + (i),		\
+				min((len) - (i), rowsize),		\
+				(rowsize), (groupsize), (linebuf),	\
+				(linebuflen), false);			\
+	     (i) += (rowsize) == 16 || (rowsize) == 32 ? (rowsize) : 16	\  
+	    )
 #ifdef CONFIG_PRINTK
 extern void print_hex_dump(const char *level, const char *prefix_str,
 			   int prefix_type, int rowsize, int groupsize,    

Hi Nick,

When compiling with gcc 7.5.0 (old, but still supported AFAIK) on x86_64
with patch 2/3 (and 1/3) applied I see this:

  CC      lib/hexdump.o
In file included from <command-line>:0:0:
lib/hexdump.c: In function 'print_hex_dump':
././include/linux/compiler_types.h:542:38: error: call to '__compiletime_assert_11' declared with attribute error: min((len) - (i), rowsize) signedness error  

...

quoted

Highlighting the min line in the macro for context, it looks like this:

	min((len) - (i), rowsize)

And in this case the types involved are:

	size_t len
	int i
	int rowsize  

Yep, that should fail for all versions of gcc.
Both 'i' and 'rowsize' should be unsigned types.
In fact all three can be 'unsigned int'.

To give a bit more context, a complication changing the types is that the
type of len and rowsise (but not i) is in the signature of the calling
function, print_hex_dump(). And I believe that function is widely used
throughout the tree.

Thinking a bit more.
If the compiler can determine that 'rowsize >= 0' then the test will pass.
More modern compilers do better value tracking so that might be enough
to stop the compiler 'bleating'.

FTR, I did not see this with GCC 14.2.0 (or clang 19.1.7).