diff mbox series

[v2,08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer

Message ID 20240411101550.99392-9-philmd@linaro.org
State New
Headers show
Series misc: Remove sprintf() due to macOS deprecation | expand

Commit Message

Philippe Mathieu-Daudé April 11, 2024, 10:15 a.m. UTC 
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/cutils.h  | 10 +++++++---
 hw/virtio/vhost-vdpa.c |  5 +++--
 util/hexdump.c         | 12 ++++++++----
 3 files changed, 18 insertions(+), 9 deletions(-)

Comments

Richard Henderson April 11, 2024, 8:47 p.m. UTC | #1 
On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qemu/cutils.h  | 10 +++++++---
>   hw/virtio/vhost-vdpa.c |  5 +++--
>   util/hexdump.c         | 12 ++++++++----
>   3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 70ca4b876b..e8d6b86098 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -254,18 +254,22 @@ int parse_debug_env(const char *name, int max, int initial);
>   
>   /**
>    * qemu_hexdump_line:
> - * @line: Buffer to be filled by the hexadecimal/ASCII dump
>    * @bufptr: Buffer to dump
>    * @offset: Offset within @bufptr to start the dump
>    * @len: Length of the bytes do dump
>    * @ascii: Replace non-ASCII characters by the dot symbol
>    *
>    * Hexdump a line of a byte buffer into a hexadecimal/ASCII buffer
> + *
> + * The caller must use g_free() to free the returned data when it is
> + * no longer required.
> + *
> + * Returns: Hexadecimal/ASCII dump
>    */
>   #define QEMU_HEXDUMP_LINE_BYTES 16 /* Number of bytes to dump */
>   #define QEMU_HEXDUMP_LINE_LEN 75   /* Number of characters in line */
> -void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
> -                       unsigned int len, bool ascii);
> +char *qemu_hexdump_line(const void *bufptr, unsigned offset,
> +                        unsigned int len, bool ascii);
>   
>   /*
>    * Hexdump a buffer to a file. An optional string prefix is added to every line
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index cf7cfa3f16..e61af86d9d 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -942,12 +942,13 @@ static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
>                                      uint32_t config_len)
>   {
>       int ofs, len;
> -    char line[QEMU_HEXDUMP_LINE_LEN];
> +    char *line;
>   
>       for (ofs = 0; ofs < config_len; ofs += 16) {
>           len = config_len - ofs;
> -        qemu_hexdump_line(line, config, ofs, len, false);
> +        line = qemu_hexdump_line(config, ofs, len, false);
>           trace_vhost_vdpa_dump_config(dev, line);
> +        g_free(line);
>       }
>   }
>   
> diff --git a/util/hexdump.c b/util/hexdump.c
> index 469083d8c0..b6f70e93bb 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -16,9 +16,10 @@
>   #include "qemu/osdep.h"
>   #include "qemu/cutils.h"
>   
> -void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
> -                       unsigned int len, bool ascii)
> +char *qemu_hexdump_line(const void *bufptr, unsigned offset,
> +                        unsigned int len, bool ascii)
>   {
> +    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
>       const char *buf = bufptr;
>       int i, c;
>   
> @@ -48,18 +49,21 @@ void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
>           }
>       }
>       *line = '\0';
> +
> +    return g_strdup(linebuf);
>   }
>   
>   void qemu_hexdump(FILE *fp, const char *prefix,
>                     const void *bufptr, size_t size)
>   {
>       unsigned int ofs, len;
> -    char line[QEMU_HEXDUMP_LINE_LEN];
> +    char *line;
>   
>       for (ofs = 0; ofs < size; ofs += QEMU_HEXDUMP_LINE_BYTES) {
>           len = size - ofs;
> -        qemu_hexdump_line(line, bufptr, ofs, len, true);
> +        line = qemu_hexdump_line(bufptr, ofs, len, true);
>           fprintf(fp, "%s: %s\n", prefix, line);
> +        g_free(line);
>       }
>   
>   }

Not especially efficient, re-allocating for each line.

How about

GString *qemu_hexdump_line(GString *str, buf, offset, len, ascii)
{
     if (str) {
         g_string_truncate(str, 0);
     } else {
         str = g_string_sized_new(QEMU_HEXDUMP_LINE_LEN);
     }
     ...
     return str;
}

void qemu_hexdump(FILE *fp, ...)
{
     g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_LEN);

     for (...) {
         qemu_hexdump_line(str, ...);
         fprintf(fp, "%s: %s\n", prefix, str->str);
     }
}

So that we reuse the one allocation across the whole loop.

r~
diff mbox series

Patch

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 70ca4b876b..e8d6b86098 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -254,18 +254,22 @@  int parse_debug_env(const char *name, int max, int initial);
 
 /**
  * qemu_hexdump_line:
- * @line: Buffer to be filled by the hexadecimal/ASCII dump
  * @bufptr: Buffer to dump
  * @offset: Offset within @bufptr to start the dump
  * @len: Length of the bytes do dump
  * @ascii: Replace non-ASCII characters by the dot symbol
  *
  * Hexdump a line of a byte buffer into a hexadecimal/ASCII buffer
+ *
+ * The caller must use g_free() to free the returned data when it is
+ * no longer required.
+ *
+ * Returns: Hexadecimal/ASCII dump
  */
 #define QEMU_HEXDUMP_LINE_BYTES 16 /* Number of bytes to dump */
 #define QEMU_HEXDUMP_LINE_LEN 75   /* Number of characters in line */
-void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
-                       unsigned int len, bool ascii);
+char *qemu_hexdump_line(const void *bufptr, unsigned offset,
+                        unsigned int len, bool ascii);
 
 /*
  * Hexdump a buffer to a file. An optional string prefix is added to every line
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index cf7cfa3f16..e61af86d9d 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -942,12 +942,13 @@  static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
                                    uint32_t config_len)
 {
     int ofs, len;
-    char line[QEMU_HEXDUMP_LINE_LEN];
+    char *line;
 
     for (ofs = 0; ofs < config_len; ofs += 16) {
         len = config_len - ofs;
-        qemu_hexdump_line(line, config, ofs, len, false);
+        line = qemu_hexdump_line(config, ofs, len, false);
         trace_vhost_vdpa_dump_config(dev, line);
+        g_free(line);
     }
 }
 
diff --git a/util/hexdump.c b/util/hexdump.c
index 469083d8c0..b6f70e93bb 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,9 +16,10 @@ 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
 
-void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
-                       unsigned int len, bool ascii)
+char *qemu_hexdump_line(const void *bufptr, unsigned offset,
+                        unsigned int len, bool ascii)
 {
+    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
     const char *buf = bufptr;
     int i, c;
 
@@ -48,18 +49,21 @@  void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
         }
     }
     *line = '\0';
+
+    return g_strdup(linebuf);
 }
 
 void qemu_hexdump(FILE *fp, const char *prefix,
                   const void *bufptr, size_t size)
 {
     unsigned int ofs, len;
-    char line[QEMU_HEXDUMP_LINE_LEN];
+    char *line;
 
     for (ofs = 0; ofs < size; ofs += QEMU_HEXDUMP_LINE_BYTES) {
         len = size - ofs;
-        qemu_hexdump_line(line, bufptr, ofs, len, true);
+        line = qemu_hexdump_line(bufptr, ofs, len, true);
         fprintf(fp, "%s: %s\n", prefix, line);
+        g_free(line);
     }
 
 }