Re: [PATCH 1/3] hexdump: Implement macro for converting large buffers
From: Paolo Abeni <pabeni@redhat.com>
Date: 2025-02-18 12:31:44
Also in:
lkml
On 2/17/25 4:09 PM, Nick Child wrote:
Thank you David and Simon for testing and review! On Sun, Feb 16, 2025 at 11:24:30AM +0000, David Laight wrote:quoted
I just changed the prototypes (include/linux/printk.h) to make both rowsize and groupsize 'unsigned int'. The same change in lib/hexdump.c + changing the local 'i, linelen, remaining' to unsigned int and it all compiled. FWIW that hexdump code is pretty horrid (especially if groupsize != 1).Given this discussion and my own head-scratching, I think I will take a closer look at hex_dump_to_buffer. I was trying to avoid editing this function due the number of callers it has across the kernel. But I think we can get away with keeping the API (but change args to uint's) and editing the body of the function to always iterate byte-by-byte, addding space chars where necessary. At the cost of a few more cycles, this will allow for more dynamic values for rowsize and groupsize and shorten the code up a bit. This would also address the "Side question" in my cover letter. Will send a v3 regardless if I can figure that out or not. The return value of hex_dump_to_buffer on error still irks me a bit but I don't think that can easily be changed.
For the new versions, please: - respect the 24h grace period: https://elixir.bootlin.com/linux/v6.11.8/source/Documentation/process/maintainer-netdev.rst#L15 - add the target tree in the subj prefix (in this case 'net-next') - ensure all the patches in the series have consistent subj prefix, otherwise patchwork will be fooled. I think it would be better to avoid modifying hex_dump_to_buffer(), if not necessary, and I think you could do so either changing the type used in patch 2 or even dropping such patch. /P