Re: [PATCH v2 1/2] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 31/10/25 21:54, Philippe Mathieu-Daudé wrote:
> On 31/10/25 20:02, Vladimir Sementsov-Ogievskiy wrote:
>> QEMU_HEXDUMP_LINE_WIDTH calculation doesn't correspond to
>> qemu_hexdump_line(). This leads to last line of the dump (when
>> length is not multiply of 16) has badly aligned ASCII part.
>>
>> Let's calculate length the same way.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   util/hexdump.c | 38 ++++++++++++++++++++++++--------------
>>   1 file changed, 24 insertions(+), 14 deletions(-)
>>
>> diff --git a/util/hexdump.c b/util/hexdump.c
>> index f29ffceb74..7cfc547261 100644
>> --- a/util/hexdump.c
>> +++ b/util/hexdump.c
>> @@ -22,6 +22,19 @@ static inline char hexdump_nibble(unsigned x)
>>       return (x < 10 ? '0' : 'a' - 10) + x;
>>   }
>> +static size_t hexdump_line_length(size_t buf_len, size_t unit_len,
>> +                                  size_t block_len)
>> +{
>> +    size_t est = buf_len * 2;
>> +    if (unit_len) {
>> +        est += buf_len / unit_len;
>> +    }
>> +    if (block_len) {
>> +        est += buf_len / block_len;
>> +    }
>> +    return est;
>> +}
> 
> 
>>   void qemu_hexdump(FILE *fp, const char *prefix,
>>                     const void *bufptr, size_t size)
>>   {
>> -    g_autoptr(GString) str = 
>> g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
>> +    int width = hexdump_line_length(QEMU_HEXDUMP_LINE_BYTES,
> 
> size_t

Hmm I now see why you use 'int':

../../util/hexdump.c:107:36: error: field width should have type 'int', 
but argument has type 'size_t' (aka 'unsigned long') [-Werror,-Wformat]
   107 |         fprintf(fp, "%s: %04zx: %-*s %s\n", prefix, b, width, 
str->str, ascii);
       |                                 ~~~^                   ~~~~~

> 
>> +                                    QEMU_HEXDUMP_UNIT,
>> +                                    QEMU_HEXDUMP_BLOCK);
>> +    g_autoptr(GString) str = g_string_sized_new(width + 1);
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>