From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Test that the fix in commit 20aa05edc2c ("util/hexdump: fix
QEMU_HEXDUMP_LINE_WIDTH logic") make sense.

To not break compilation when we build without 'block', move
hexdump.c out of "if have_block" in meson.build.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v6:
- mention commit 20aa05edc2c in desc
- wrap long lines for checkpatch.pl
- use g_file_open_tmp()
---
 tests/unit/test-cutils.c | 67 ++++++++++++++++++++++++++++++++++++++++
 util/meson.build         |  2 +-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 75fae29003a..d70218ad1c1 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -3626,6 +3626,68 @@ static void test_si_prefix(void)
     g_assert_cmpstr(si_prefix(18), ==, "E");
 }
 
+static void test_qemu_hexdump_alignment(void)
+{
+    /*
+     * Test that ASCII part is properly aligned for incomplete lines.
+     * This test catches the bug that was fixed in previous commit
+     * "util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic".
+     *
+     * We use data that is not aligned to 16 bytes, so last line
+     * is incomplete.
+     */
+    static const uint8_t data[] = {
+        /* First line: 16 bytes */
+        0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f,  /* "Hello Wo" */
+        0x72, 0x6c, 0x64, 0x21, 0x20, 0x54, 0x68, 0x69,  /* "rld! Thi" */
+        /* Second line: 5 bytes (incomplete) */
+        0x73, 0x20, 0x69, 0x73, 0x20                     /* "s is " */
+    };
+    char *fname = NULL;
+    int fd;
+    g_autofree char *output = NULL;
+    size_t size, bytes_read;
+    FILE *f;
+
+    fd = g_file_open_tmp("test-qemu-hexdump-alignment-XXXXXX", &fname, NULL);
+    g_assert(fd >= 0);
+    g_assert_nonnull(fname);
+    f = fdopen(fd, "w+");
+
+    g_assert_nonnull(f);
+
+    qemu_hexdump(f, "test", data, sizeof(data));
+
+    size = ftell(f);
+    fseek(f, 0, SEEK_SET);
+
+    output = g_malloc(size + 1);
+    bytes_read = 0;
+    while (bytes_read < size) {
+        size_t chunk = fread(output + bytes_read, 1, size - bytes_read, f);
+        if (chunk == 0) {
+            break;
+        }
+        bytes_read += chunk;
+    }
+    g_assert_cmpuint(bytes_read, ==, size);
+    output[size] = '\0';
+
+    fclose(f);
+    close(fd);
+    unlink(fname);
+    g_free(fname);
+
+    /* We expect proper alignment of "s is" part on the second line */
+    static const char *expected =
+        "test: 0000: 48 65 6c 6c  6f 20 57 6f  72 6c 64 21  20 54 68 69   "
+            "Hello World! Thi\n"
+        "test: 0010: 73 20 69 73  20                                      "
+            "s is \n";
+
+    g_assert_cmpstr(output, ==, expected);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -3995,5 +4057,10 @@ int main(int argc, char **argv)
                     test_iec_binary_prefix);
     g_test_add_func("/cutils/si_prefix",
                     test_si_prefix);
+
+    /* qemu_hexdump() test */
+    g_test_add_func("/cutils/qemu_hexdump/alignment",
+                    test_qemu_hexdump_alignment);
+
     return g_test_run();
 }
diff --git a/util/meson.build b/util/meson.build
index a0cfdc30ba6..7c9445615d7 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -33,6 +33,7 @@ endif
 util_ss.add(files('defer-call.c'))
 util_ss.add(files('envlist.c', 'path.c', 'module.c'))
 util_ss.add(files('event.c'))
+util_ss.add(files('hexdump.c'))
 util_ss.add(files('host-utils.c'))
 util_ss.add(files('bitmap.c', 'bitops.c'))
 util_ss.add(files('fifo8.c'))
@@ -90,7 +91,6 @@ if have_block
   util_ss.add(files('buffer.c'))
   util_ss.add(files('bufferiszero.c'))
   util_ss.add(files('hbitmap.c'))
-  util_ss.add(files('hexdump.c'))
   util_ss.add(files('iova-tree.c'))
   util_ss.add(files('iov.c'))
   util_ss.add(files('nvdimm-utils.c'))
-- 
2.52.0

On 02.02.26 01:13, Philippe Mathieu-Daudé wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> Test that the fix in commit 20aa05edc2c ("util/hexdump: fix
> QEMU_HEXDUMP_LINE_WIDTH logic") make sense.
> 
> To not break compilation when we build without 'block', move
> hexdump.c out of "if have_block" in meson.build.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v6:
> - mention commit 20aa05edc2c in desc
> - wrap long lines for checkpatch.pl
> - use g_file_open_tmp()
> ---
>   tests/unit/test-cutils.c | 67 ++++++++++++++++++++++++++++++++++++++++
>   util/meson.build         |  2 +-
>   2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> index 75fae29003a..d70218ad1c1 100644
> --- a/tests/unit/test-cutils.c
> +++ b/tests/unit/test-cutils.c
> @@ -3626,6 +3626,68 @@ static void test_si_prefix(void)
>       g_assert_cmpstr(si_prefix(18), ==, "E");
>   }
>   
> +static void test_qemu_hexdump_alignment(void)
> +{
> +    /*
> +     * Test that ASCII part is properly aligned for incomplete lines.
> +     * This test catches the bug that was fixed in previous commit
> +     * "util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic".
> +     *
> +     * We use data that is not aligned to 16 bytes, so last line
> +     * is incomplete.
> +     */
> +    static const uint8_t data[] = {
> +        /* First line: 16 bytes */
> +        0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f,  /* "Hello Wo" */
> +        0x72, 0x6c, 0x64, 0x21, 0x20, 0x54, 0x68, 0x69,  /* "rld! Thi" */
> +        /* Second line: 5 bytes (incomplete) */
> +        0x73, 0x20, 0x69, 0x73, 0x20                     /* "s is " */
> +    };
> +    char *fname = NULL;
> +    int fd;
> +    g_autofree char *output = NULL;
> +    size_t size, bytes_read;
> +    FILE *f;
> +
> +    fd = g_file_open_tmp("test-qemu-hexdump-alignment-XXXXXX", &fname, NULL);
> +    g_assert(fd >= 0);
> +    g_assert_nonnull(fname);
> +    f = fdopen(fd, "w+");
> +
> +    g_assert_nonnull(f);
> +
> +    qemu_hexdump(f, "test", data, sizeof(data));
> +
> +    size = ftell(f);
> +    fseek(f, 0, SEEK_SET);
> +
> +    output = g_malloc(size + 1);
> +    bytes_read = 0;
> +    while (bytes_read < size) {
> +        size_t chunk = fread(output + bytes_read, 1, size - bytes_read, f);
> +        if (chunk == 0) {
> +            break;
> +        }
> +        bytes_read += chunk;
> +    }
> +    g_assert_cmpuint(bytes_read, ==, size);
> +    output[size] = '\0';
> +
> +    fclose(f);
> +    close(fd);

As I understand, fclose() is enough, documentation for fdopen() says that original
fd is closed on fclose().

Otherwise, no objections, thanks for resend.

> +    unlink(fname);
> +    g_free(fname);
> +
> +    /* We expect proper alignment of "s is" part on the second line */
> +    static const char *expected =
> +        "test: 0000: 48 65 6c 6c  6f 20 57 6f  72 6c 64 21  20 54 68 69   "
> +            "Hello World! Thi\n"
> +        "test: 0010: 73 20 69 73  20                                      "
> +            "s is \n";
> +
> +    g_assert_cmpstr(output, ==, expected);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       g_test_init(&argc, &argv, NULL);
> @@ -3995,5 +4057,10 @@ int main(int argc, char **argv)
>                       test_iec_binary_prefix);
>       g_test_add_func("/cutils/si_prefix",
>                       test_si_prefix);
> +
> +    /* qemu_hexdump() test */
> +    g_test_add_func("/cutils/qemu_hexdump/alignment",
> +                    test_qemu_hexdump_alignment);
> +
>       return g_test_run();
>   }
> diff --git a/util/meson.build b/util/meson.build
> index a0cfdc30ba6..7c9445615d7 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -33,6 +33,7 @@ endif
>   util_ss.add(files('defer-call.c'))
>   util_ss.add(files('envlist.c', 'path.c', 'module.c'))
>   util_ss.add(files('event.c'))
> +util_ss.add(files('hexdump.c'))
>   util_ss.add(files('host-utils.c'))
>   util_ss.add(files('bitmap.c', 'bitops.c'))
>   util_ss.add(files('fifo8.c'))
> @@ -90,7 +91,6 @@ if have_block
>     util_ss.add(files('buffer.c'))
>     util_ss.add(files('bufferiszero.c'))
>     util_ss.add(files('hbitmap.c'))
> -  util_ss.add(files('hexdump.c'))
>     util_ss.add(files('iova-tree.c'))
>     util_ss.add(files('iov.c'))
>     util_ss.add(files('nvdimm-utils.c'))


-- 
Best regards,
Vladimir

On 1/2/26 23:13, Philippe Mathieu-Daudé wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> Test that the fix in commit 20aa05edc2c ("util/hexdump: fix
> QEMU_HEXDUMP_LINE_WIDTH logic") make sense.
> 
> To not break compilation when we build without 'block', move
> hexdump.c out of "if have_block" in meson.build.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v6:
> - mention commit 20aa05edc2c in desc
> - wrap long lines for checkpatch.pl
> - use g_file_open_tmp()
> ---
>   tests/unit/test-cutils.c | 67 ++++++++++++++++++++++++++++++++++++++++
>   util/meson.build         |  2 +-
>   2 files changed, 68 insertions(+), 1 deletion(-)


> +    /* We expect proper alignment of "s is" part on the second line */
> +    static const char *expected =
> +        "test: 0000: 48 65 6c 6c  6f 20 57 6f  72 6c 64 21  20 54 68 69   "
> +            "Hello World! Thi\n"
> +        "test: 0010: 73 20 69 73  20                                      "
> +            "s is \n";

BTW I chose to ignore this checkpatch.pl error:

   ERROR: unnecessary whitespace before a quoted newline
   #90: FILE: tests/unit/test-cutils.c:3686:
   +        "s is \n";