* [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic
@ 2025-09-05 14:20 Vladimir Sementsov-Ogievskiy
2025-09-08 14:51 ` Daniel P. Berrangé
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-05 14:20 UTC (permalink / raw)
To: berrange; +Cc: qemu-devel, vsementsov
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;
+}
+
GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
size_t unit_len, size_t block_len)
{
@@ -30,14 +43,8 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
if (str == NULL) {
/* Estimate the length of the output to avoid reallocs. */
- size_t est = len * 2;
- if (unit_len) {
- est += len / unit_len;
- }
- if (block_len) {
- est += len / block_len;
- }
- str = g_string_sized_new(est + 1);
+ str = g_string_sized_new(hexdump_line_length(len, unit_len, block_len)
+ + 1);
}
for (u = 0, b = 0; len; u++, b++, len--, buf++) {
@@ -76,13 +83,16 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
}
#define QEMU_HEXDUMP_LINE_BYTES 16
-#define QEMU_HEXDUMP_LINE_WIDTH \
- (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
+#define QEMU_HEXDUMP_UNIT 1
+#define QEMU_HEXDUMP_BLOCK 4
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,
+ QEMU_HEXDUMP_UNIT,
+ QEMU_HEXDUMP_BLOCK);
+ g_autoptr(GString) str = g_string_sized_new(width + 1);
char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
size_t b, len;
@@ -90,11 +100,11 @@ void qemu_hexdump(FILE *fp, const char *prefix,
len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
g_string_truncate(str, 0);
- qemu_hexdump_line(str, bufptr + b, len, 1, 4);
+ qemu_hexdump_line(str, bufptr + b, len,
+ QEMU_HEXDUMP_UNIT, QEMU_HEXDUMP_BLOCK);
asciidump_line(ascii, bufptr + b, len);
- fprintf(fp, "%s: %04zx: %-*s %s\n",
- prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
+ fprintf(fp, "%s: %04zx: %-*s %s\n", prefix, b, width, str->str, ascii);
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic
2025-09-05 14:20 [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic Vladimir Sementsov-Ogievskiy
@ 2025-09-08 14:51 ` Daniel P. Berrangé
2025-09-08 16:03 ` Vladimir Sementsov-Ogievskiy
2025-09-09 14:20 ` Ján Tomko
0 siblings, 2 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2025-09-08 14:51 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy; +Cc: qemu-devel
On Fri, Sep 05, 2025 at 05:20:06PM +0300, 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(-)
Would you mind also adding a test to tests/unit/test-cutils.c
to show the correctness, as this is a nice standalone func
we ought to be unit testing.
>
> 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;
> +}
> +
> GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
> size_t unit_len, size_t block_len)
> {
> @@ -30,14 +43,8 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
>
> if (str == NULL) {
> /* Estimate the length of the output to avoid reallocs. */
> - size_t est = len * 2;
> - if (unit_len) {
> - est += len / unit_len;
> - }
> - if (block_len) {
> - est += len / block_len;
> - }
> - str = g_string_sized_new(est + 1);
> + str = g_string_sized_new(hexdump_line_length(len, unit_len, block_len)
> + + 1);
> }
>
> for (u = 0, b = 0; len; u++, b++, len--, buf++) {
> @@ -76,13 +83,16 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
> }
>
> #define QEMU_HEXDUMP_LINE_BYTES 16
> -#define QEMU_HEXDUMP_LINE_WIDTH \
> - (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
> +#define QEMU_HEXDUMP_UNIT 1
> +#define QEMU_HEXDUMP_BLOCK 4
>
> 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,
> + QEMU_HEXDUMP_UNIT,
> + QEMU_HEXDUMP_BLOCK);
> + g_autoptr(GString) str = g_string_sized_new(width + 1);
> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
> size_t b, len;
>
> @@ -90,11 +100,11 @@ void qemu_hexdump(FILE *fp, const char *prefix,
> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
>
> g_string_truncate(str, 0);
> - qemu_hexdump_line(str, bufptr + b, len, 1, 4);
> + qemu_hexdump_line(str, bufptr + b, len,
> + QEMU_HEXDUMP_UNIT, QEMU_HEXDUMP_BLOCK);
> asciidump_line(ascii, bufptr + b, len);
>
> - fprintf(fp, "%s: %04zx: %-*s %s\n",
> - prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
> + fprintf(fp, "%s: %04zx: %-*s %s\n", prefix, b, width, str->str, ascii);
> }
>
> }
> --
> 2.48.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic
2025-09-08 14:51 ` Daniel P. Berrangé
@ 2025-09-08 16:03 ` Vladimir Sementsov-Ogievskiy
2025-09-09 14:20 ` Ján Tomko
1 sibling, 0 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-08 16:03 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel
On 08.09.25 17:51, Daniel P. Berrangé wrote:
> On Fri, Sep 05, 2025 at 05:20:06PM +0300, 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(-)
>
> Would you mind also adding a test to tests/unit/test-cutils.c
> to show the correctness, as this is a nice standalone func
> we ought to be unit testing.
Sure, will add
>
>>
>> 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;
>> +}
>> +
>> GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
>> size_t unit_len, size_t block_len)
>> {
>> @@ -30,14 +43,8 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
>>
>> if (str == NULL) {
>> /* Estimate the length of the output to avoid reallocs. */
>> - size_t est = len * 2;
>> - if (unit_len) {
>> - est += len / unit_len;
>> - }
>> - if (block_len) {
>> - est += len / block_len;
>> - }
>> - str = g_string_sized_new(est + 1);
>> + str = g_string_sized_new(hexdump_line_length(len, unit_len, block_len)
>> + + 1);
>> }
>>
>> for (u = 0, b = 0; len; u++, b++, len--, buf++) {
>> @@ -76,13 +83,16 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
>> }
>>
>> #define QEMU_HEXDUMP_LINE_BYTES 16
>> -#define QEMU_HEXDUMP_LINE_WIDTH \
>> - (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
>> +#define QEMU_HEXDUMP_UNIT 1
>> +#define QEMU_HEXDUMP_BLOCK 4
>>
>> 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,
>> + QEMU_HEXDUMP_UNIT,
>> + QEMU_HEXDUMP_BLOCK);
>> + g_autoptr(GString) str = g_string_sized_new(width + 1);
>> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
>> size_t b, len;
>>
>> @@ -90,11 +100,11 @@ void qemu_hexdump(FILE *fp, const char *prefix,
>> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
>>
>> g_string_truncate(str, 0);
>> - qemu_hexdump_line(str, bufptr + b, len, 1, 4);
>> + qemu_hexdump_line(str, bufptr + b, len,
>> + QEMU_HEXDUMP_UNIT, QEMU_HEXDUMP_BLOCK);
>> asciidump_line(ascii, bufptr + b, len);
>>
>> - fprintf(fp, "%s: %04zx: %-*s %s\n",
>> - prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
>> + fprintf(fp, "%s: %04zx: %-*s %s\n", prefix, b, width, str->str, ascii);
>> }
>>
>> }
>> --
>> 2.48.1
>>
>
> With regards,
> Daniel
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic
2025-09-08 14:51 ` Daniel P. Berrangé
2025-09-08 16:03 ` Vladimir Sementsov-Ogievskiy
@ 2025-09-09 14:20 ` Ján Tomko
1 sibling, 0 replies; 4+ messages in thread
From: Ján Tomko @ 2025-09-09 14:20 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Vladimir Sementsov-Ogievskiy, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]
On a Monday in 2025, Daniel P. Berrangé wrote:
>On Fri, Sep 05, 2025 at 05:20:06PM +0300, 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(-)
>
>Would you mind also adding a test to tests/unit/test-cutils.c
>to show the correctness, as this is a nice standalone func
>we ought to be unit testing.
>
>>
>> diff --git a/util/hexdump.c b/util/hexdump.c
>> index f29ffceb74..7cfc547261 100644
>> --- a/util/hexdump.c
>> +++ b/util/hexdump.c
>> @@ -30,14 +43,8 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
>>
>> if (str == NULL) {
>> /* Estimate the length of the output to avoid reallocs. */
>> - size_t est = len * 2;
>> - if (unit_len) {
>> - est += len / unit_len;
>> - }
>> - if (block_len) {
>> - est += len / block_len;
>> - }
>> - str = g_string_sized_new(est + 1);
>> + str = g_string_sized_new(hexdump_line_length(len, unit_len, block_len)
>> + + 1);
Fun fact: with new enough glib, this will allocate a buffer of 128 bytes (nearest
power of 64+1) anyway:
https://gitlab.gnome.org/GNOME/glib/-/commit/7fc7c57b6fd470bbee267471a61c714f3a0d281c
Jano
>> }
>>
>> for (u = 0, b = 0; len; u++, b++, len--, buf++) {
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-09 14:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05 14:20 [PATCH] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic Vladimir Sementsov-Ogievskiy
2025-09-08 14:51 ` Daniel P. Berrangé
2025-09-08 16:03 ` Vladimir Sementsov-Ogievskiy
2025-09-09 14:20 ` Ján Tomko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).