qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation
@ 2024-04-10 16:06 Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block,
	Philippe =?unknown-8bit?q?Mathieu-Daud=C3=A9?=

Hi,

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Suggestion to avoid the super-noisy warning on macOS forum are [*]:

* use -Wno-deprecated-declarations on the whole build
* surgically add #pragma clang diagnostic around each use.

None of these options seem reasonable, so we are somehow forced to
spend time converting each sprintf() call, even if they are safe
enough.

I'm so tired of seeing them than I started the conversion. This is
the first part. Help for the rest is welcomed.

Regards,

Phil.

[*] https://forums.developer.apple.com/forums/thread/714675

Philippe Mathieu-Daudé (12):
  ui/console-vc: Replace sprintf() by g_strdup_printf()
  hw/vfio/pci: Replace sprintf() by g_strdup_printf()
  hw/ppc/spapr: Replace sprintf() by g_strdup_printf()
  hw/mips/malta: Replace sprintf() by g_string_append_printf()
  system/qtest: Replace sprintf() by g_string_append_printf()
  util/hexdump: Rename @offset argument in qemu_hexdump_line()
  util/hexdump: Have qemu_hexdump_line() return heap allocated buffer
  util/hexdump: Replace sprintf() by g_string_append_printf()
  hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()
  hw/ide/atapi: Use qemu_hexdump_line() to avoid sprintf()
  hw/dma/pl330: Use qemu_hexdump_line() to avoid sprintf()
  backends/tpm: Use qemu_hexdump_line() to avoid sprintf()

 include/qemu/cutils.h   | 17 ++++++++++++++---
 backends/tpm/tpm_util.c | 24 ++++++++----------------
 hw/dma/pl330.c          | 12 +++---------
 hw/ide/atapi.c          |  8 ++------
 hw/mips/malta.c         | 22 +++++++++++++---------
 hw/ppc/spapr.c          |  4 ++--
 hw/scsi/scsi-disk.c     |  8 ++------
 hw/vfio/pci.c           |  7 +++----
 hw/virtio/vhost-vdpa.c  | 11 ++++++-----
 system/qtest.c          |  8 +++-----
 ui/console-vc.c         |  4 ++--
 util/hexdump.c          | 33 ++++++++++++++++++---------------
 12 files changed, 76 insertions(+), 82 deletions(-)

-- 
2.41.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-11  6:58   ` Marc-André Lureau
  2024-04-11  7:47   ` Gerd Hoffmann
  2024-04-10 16:06 ` [PATCH 02/12] hw/vfio/pci: " Philippe Mathieu-Daudé
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Marc-André Lureau

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Replace sprintf() by g_strdup_printf() in order to avoid:

  [702/1310] Compiling C object libcommon.fa.p/ui_console-vc.c.o
  ui/console-vc.c:824:21: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
                    sprintf(response, "\033[%d;%dR",
                    ^
  1 warning generated.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 ui/console-vc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 899fa11c94..b455db436d 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -648,7 +648,7 @@ static void vc_putchar(VCChardev *vc, int ch)
     QemuTextConsole *s = vc->console;
     int i;
     int x, y;
-    char response[40];
+    g_autofree char *response = NULL;
 
     switch(vc->state) {
     case TTY_STATE_NORM:
@@ -821,7 +821,7 @@ static void vc_putchar(VCChardev *vc, int ch)
                     break;
                 case 6:
                     /* report cursor position */
-                    sprintf(response, "\033[%d;%dR",
+                    response = g_strdup_printf("\033[%d;%dR",
                            (s->y_base + s->y) % s->total_height + 1,
                             s->x + 1);
                     vc_respond_str(vc, response);
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/12] hw/vfio/pci: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-12 15:25   ` Alex Williamson
  2024-04-10 16:06 ` [PATCH 03/12] hw/ppc/spapr: " Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Alex Williamson, Cédric Le Goater

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience. Use g_strdup_printf()
instead.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/vfio/pci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 64780d1b79..cc3cc89122 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2442,10 +2442,9 @@ void vfio_pci_post_reset(VFIOPCIDevice *vdev)
 
 bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
 {
-    char tmp[13];
-
-    sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
-            addr->bus, addr->slot, addr->function);
+    g_autofree char *tmp = g_strdup_printf("%04x:%02x:%02x.%1x",
+                                           addr->domain, addr->bus,
+                                           addr->slot, addr->function);
 
     return (strcmp(tmp, name) == 0);
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/12] hw/ppc/spapr: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 02/12] hw/vfio/pci: " Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 04/12] hw/mips/malta: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Nicholas Piggin, Daniel Henrique Barboza, David Gibson,
	Harsh Prateek Bora

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Replace sprintf() by g_strdup_printf() in order to avoid:

  hw/ppc/spapr.c:385:5: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
      sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
      ^
  1 warning generated.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e9bc97fee0..9807f47690 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -375,14 +375,14 @@ static void add_str(GString *s, const gchar *s1)
 static int spapr_dt_memory_node(SpaprMachineState *spapr, void *fdt, int nodeid,
                                 hwaddr start, hwaddr size)
 {
-    char mem_name[32];
+    g_autofree char *mem_name = NULL;
     uint64_t mem_reg_property[2];
     int off;
 
     mem_reg_property[0] = cpu_to_be64(start);
     mem_reg_property[1] = cpu_to_be64(size);
 
-    sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
+    mem_name = g_strdup_printf("memory@%" HWADDR_PRIx, start);
     off = fdt_add_subnode(fdt, 0, mem_name);
     _FDT(off);
     _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/12] hw/mips/malta: Replace sprintf() by g_string_append_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 03/12] hw/ppc/spapr: " Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 05/12] system/qtest: " Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Aurelien Jarno, Jiaxun Yang

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Extract common code to get_rng_seed_hex(), replacing the
sprintf() calls by GString API ones in order to avoid:

  [120/169] Compiling C object libcommon.fa.p/system_qtest.c.o
  hw/mips/malta.c:860:9: warning: 'sprintf' is deprecated:
          sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
          ^
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
  hw/mips/malta.c:951:9: warning: 'sprintf' is deprecated:
          sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
          ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/malta.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index af74008c82..095abbf0ec 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -850,15 +850,24 @@ static void G_GNUC_PRINTF(3, 4) prom_set(uint32_t *prom_buf, int index,
     va_end(ap);
 }
 
-static void reinitialize_rng_seed(void *opaque)
+static char *get_rng_seed_hex(void)
 {
-    char *rng_seed_hex = opaque;
+    g_autoptr(GString) gs = g_string_new("");
     uint8_t rng_seed[32];
 
     qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
     for (size_t i = 0; i < sizeof(rng_seed); ++i) {
-        sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
+        g_string_append_printf(gs, "%02x", rng_seed[i]);
     }
+
+    return g_strdup(gs->str);
+}
+
+static void reinitialize_rng_seed(void *opaque)
+{
+    g_autofree char *rng_seed_hex = get_rng_seed_hex();
+
+    strcpy(opaque, rng_seed_hex);
 }
 
 /* Kernel */
@@ -870,8 +879,7 @@ static uint64_t load_kernel(void)
     uint32_t *prom_buf;
     long prom_size;
     int prom_index = 0;
-    uint8_t rng_seed[32];
-    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
+    g_autofree char *rng_seed_hex = get_rng_seed_hex();
     size_t rng_seed_prom_offset;
 
     kernel_size = load_elf(loaderparams.kernel_filename, NULL,
@@ -946,10 +954,6 @@ static uint64_t load_kernel(void)
     prom_set(prom_buf, prom_index++, "modetty0");
     prom_set(prom_buf, prom_index++, "38400n8r");
 
-    qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-    for (size_t i = 0; i < sizeof(rng_seed); ++i) {
-        sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
-    }
     prom_set(prom_buf, prom_index++, "rngseed");
     rng_seed_prom_offset = prom_index * ENVP_ENTRY_SIZE +
                            sizeof(uint32_t) * ENVP_NB_ENTRIES;
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/12] system/qtest: Replace sprintf() by g_string_append_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 04/12] hw/mips/malta: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-11  6:30   ` Thomas Huth
  2024-04-10 16:06 ` [PATCH 06/12] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Thomas Huth, Laurent Vivier, Paolo Bonzini

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Replace sprintf() by GString API uses in order to avoid:

  [120/169] Compiling C object libcommon.fa.p/system_qtest.c.o
  system/qtest.c:623:13: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
            sprintf(&enc[i * 2], "%02x", data[i]);
            ^
  1 warning generated.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/qtest.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/system/qtest.c b/system/qtest.c
index 6da58b3874..22bf1a33dc 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -601,9 +601,9 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0x%016" PRIx64 "\n", value);
     } else if (strcmp(words[0], "read") == 0) {
+        g_autoptr(GString) enc = g_string_new("");
         uint64_t addr, len, i;
         uint8_t *data;
-        char *enc;
         int ret;
 
         g_assert(words[1] && words[2]);
@@ -618,16 +618,14 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
                            len);
 
-        enc = g_malloc(2 * len + 1);
         for (i = 0; i < len; i++) {
-            sprintf(&enc[i * 2], "%02x", data[i]);
+            g_string_append_printf(enc, "%02x", data[i]);
         }
 
         qtest_send_prefix(chr);
-        qtest_sendf(chr, "OK 0x%s\n", enc);
+        qtest_sendf(chr, "OK 0x%s\n", enc->str);
 
         g_free(data);
-        g_free(enc);
     } else if (strcmp(words[0], "b64read") == 0) {
         uint64_t addr, len;
         uint8_t *data;
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/12] util/hexdump: Rename @offset argument in qemu_hexdump_line()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 05/12] system/qtest: " Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 07/12] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Michael S. Tsirkin

@offset argument is more descriptive than @b.

Inverse @bufptr <-> @offset arguments order.

Document qemu_hexdump_line().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/cutils.h  | 11 +++++++++--
 hw/virtio/vhost-vdpa.c |  8 ++++----
 util/hexdump.c         | 16 ++++++++--------
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 92c927a6a3..70ca4b876b 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -252,12 +252,19 @@ static inline const char *yes_no(bool b)
  */
 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
  */
 #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, unsigned int b, const void *bufptr,
+void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
                        unsigned int len, bool ascii);
 
 /*
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index e827b9175f..cf7cfa3f16 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -941,12 +941,12 @@ static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
 static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
                                    uint32_t config_len)
 {
-    int b, len;
+    int ofs, len;
     char line[QEMU_HEXDUMP_LINE_LEN];
 
-    for (b = 0; b < config_len; b += 16) {
-        len = config_len - b;
-        qemu_hexdump_line(line, b, config, len, false);
+    for (ofs = 0; ofs < config_len; ofs += 16) {
+        len = config_len - ofs;
+        qemu_hexdump_line(line, config, ofs, len, false);
         trace_vhost_vdpa_dump_config(dev, line);
     }
 }
diff --git a/util/hexdump.c b/util/hexdump.c
index 9921114b3c..469083d8c0 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,7 +16,7 @@
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
 
-void qemu_hexdump_line(char *line, unsigned int b, const void *bufptr,
+void qemu_hexdump_line(char *line, const void *bufptr, unsigned offset,
                        unsigned int len, bool ascii)
 {
     const char *buf = bufptr;
@@ -26,13 +26,13 @@ void qemu_hexdump_line(char *line, unsigned int b, const void *bufptr,
         len = QEMU_HEXDUMP_LINE_BYTES;
     }
 
-    line += snprintf(line, 6, "%04x:", b);
+    line += snprintf(line, 6, "%04x:", offset);
     for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
         if ((i % 4) == 0) {
             *line++ = ' ';
         }
         if (i < len) {
-            line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
+            line += sprintf(line, " %02x", (unsigned char)buf[offset + i]);
         } else {
             line += sprintf(line, "   ");
         }
@@ -40,7 +40,7 @@ void qemu_hexdump_line(char *line, unsigned int b, const void *bufptr,
     if (ascii) {
         *line++ = ' ';
         for (i = 0; i < len; i++) {
-            c = buf[b + i];
+            c = buf[offset + i];
             if (c < ' ' || c > '~') {
                 c = '.';
             }
@@ -53,12 +53,12 @@ void qemu_hexdump_line(char *line, unsigned int b, const void *bufptr,
 void qemu_hexdump(FILE *fp, const char *prefix,
                   const void *bufptr, size_t size)
 {
-    unsigned int b, len;
+    unsigned int ofs, len;
     char line[QEMU_HEXDUMP_LINE_LEN];
 
-    for (b = 0; b < size; b += QEMU_HEXDUMP_LINE_BYTES) {
-        len = size - b;
-        qemu_hexdump_line(line, b, bufptr, len, true);
+    for (ofs = 0; ofs < size; ofs += QEMU_HEXDUMP_LINE_BYTES) {
+        len = size - ofs;
+        qemu_hexdump_line(line, bufptr, ofs, len, true);
         fprintf(fp, "%s: %s\n", prefix, line);
     }
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/12] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 06/12] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 08/12] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Michael S. Tsirkin

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);
     }
 
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/12] util/hexdump: Replace sprintf() by g_string_append_printf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 07/12] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 09/12] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Replace sprintf() by GString API in order to avoid:

  [426/1310] Compiling C object libqemuutil.a.p/util_hexdump.c.o
  util/hexdump.c:35:21: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
            line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
                    ^
  util/hexdump.c:37:21: warning: 'sprintf' is deprecated:
            line += sprintf(line, "   ");
                    ^
  2 warnings generated.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 util/hexdump.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/util/hexdump.c b/util/hexdump.c
index b6f70e93bb..2ec1171de3 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -19,7 +19,7 @@
 char *qemu_hexdump_line(const void *bufptr, unsigned offset,
                         unsigned int len, bool ascii)
 {
-    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
+    g_autoptr(GString) gs = g_string_sized_new(QEMU_HEXDUMP_LINE_BYTES);
     const char *buf = bufptr;
     int i, c;
 
@@ -27,30 +27,29 @@ char *qemu_hexdump_line(const void *bufptr, unsigned offset,
         len = QEMU_HEXDUMP_LINE_BYTES;
     }
 
-    line += snprintf(line, 6, "%04x:", offset);
+    g_string_append_printf(gs, "%04x:", offset);
     for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
         if ((i % 4) == 0) {
-            *line++ = ' ';
+            g_string_append_c(gs, ' ');
         }
         if (i < len) {
-            line += sprintf(line, " %02x", (unsigned char)buf[offset + i]);
+            g_string_append_printf(gs, " %02x", (unsigned char)buf[offset + i]);
         } else {
-            line += sprintf(line, "   ");
+            g_string_append(gs, "   ");
         }
     }
     if (ascii) {
-        *line++ = ' ';
+        g_string_append_c(gs, ' ');
         for (i = 0; i < len; i++) {
             c = buf[offset + i];
             if (c < ' ' || c > '~') {
                 c = '.';
             }
-            *line++ = c;
+            g_string_append_c(gs, c);
         }
     }
-    *line = '\0';
 
-    return g_strdup(linebuf);
+    return g_strdup(gs->str);
 }
 
 void qemu_hexdump(FILE *fp, const char *prefix,
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/12] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 08/12] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 10/12] hw/ide/atapi: " Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Paolo Bonzini, Fam Zheng

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Use qemu_hexdump_line() to avoid sprintf() calls, silencing:

  [105/169] Compiling C object libcommon.fa.p/hw_scsi_scsi-disk.c.o
  hw/scsi/scsi-disk.c:2659:14: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
        p += sprintf(p, " 0x%02x", buf[i]);
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/scsi-disk.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 4bd7af9d0c..4f914df5c2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2648,16 +2648,12 @@ static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
 
 static void scsi_disk_new_request_dump(uint32_t lun, uint32_t tag, uint8_t *buf)
 {
-    int i;
     int len = scsi_cdb_length(buf);
-    char *line_buffer, *p;
+    char *line_buffer;
 
     assert(len > 0 && len <= 16);
-    line_buffer = g_malloc(len * 5 + 1);
 
-    for (i = 0, p = line_buffer; i < len; i++) {
-        p += sprintf(p, " 0x%02x", buf[i]);
-    }
+    line_buffer = qemu_hexdump_line(buf, 0, len, false);
     trace_scsi_disk_new_request(lun, tag, line_buffer);
 
     g_free(line_buffer);
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/12] hw/ide/atapi: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 09/12] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 11/12] hw/dma/pl330: " Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	John Snow

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Use qemu_hexdump_line() to avoid sprintf() calls, silencing:

  [1367/1604] Compiling C object libcommon.fa.p/backends_tpm_tpm_util.c.o
  backends/tpm/tpm_util.c:355:18: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
            p += sprintf(p, "\n");
                 ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/atapi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 73ec373184..27b6aa59fd 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
 #include "scsi/constants.h"
@@ -1309,12 +1310,7 @@ void ide_atapi_cmd(IDEState *s)
     trace_ide_atapi_cmd(s, s->io_buffer[0]);
 
     if (trace_event_get_state_backends(TRACE_IDE_ATAPI_CMD_PACKET)) {
-        /* Each pretty-printed byte needs two bytes and a space; */
-        char *ppacket = g_malloc(ATAPI_PACKET_SIZE * 3 + 1);
-        int i;
-        for (i = 0; i < ATAPI_PACKET_SIZE; i++) {
-            sprintf(ppacket + (i * 3), "%02x ", buf[i]);
-        }
+        char *ppacket = qemu_hexdump_line(buf, 0, ATAPI_PACKET_SIZE, false);
         trace_ide_atapi_cmd_packet(s, s->lcyl | (s->hcyl << 8), ppacket);
         g_free(ppacket);
     }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 11/12] hw/dma/pl330: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 10/12] hw/ide/atapi: " Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 16:06 ` [PATCH 12/12] backends/tpm: " Philippe Mathieu-Daudé
  2024-04-10 19:12 ` [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Richard Henderson
  12 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Peter Maydell

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Use qemu_hexdump_line() to avoid sprintf() calls, silencing:

  [5/8] Compiling C object libcommon.fa.p/hw_dma_pl330.c.o
  hw/dma/pl330.c:333:13: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
            sprintf(tmpbuf + strlen(tmpbuf), " %02x", buf[b + i]);
            ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/dma/pl330.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index 70a502d245..0435378b7e 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -15,6 +15,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
@@ -317,21 +318,14 @@ typedef struct PL330InsnDesc {
 
 static void pl330_hexdump(uint8_t *buf, size_t size)
 {
-    unsigned int b, i, len;
-    char tmpbuf[80];
+    unsigned int b, len;
 
     for (b = 0; b < size; b += 16) {
         len = size - b;
         if (len > 16) {
             len = 16;
         }
-        tmpbuf[0] = '\0';
-        for (i = 0; i < len; i++) {
-            if ((i % 4) == 0) {
-                strcat(tmpbuf, " ");
-            }
-            sprintf(tmpbuf + strlen(tmpbuf), " %02x", buf[b + i]);
-        }
+        g_autofree char *tmpbuf = qemu_hexdump_line(buf, b, len, false);
         trace_pl330_hexdump(b, tmpbuf);
     }
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 12/12] backends/tpm: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 11/12] hw/dma/pl330: " Philippe Mathieu-Daudé
@ 2024-04-10 16:06 ` Philippe Mathieu-Daudé
  2024-04-10 19:51   ` Stefan Berger
  2024-04-10 19:12 ` [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Richard Henderson
  12 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-10 16:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Philippe Mathieu-Daudé,
	Stefan Berger

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Use qemu_hexdump_line() to avoid sprintf() calls, silencing:

  backends/tpm/tpm_util.c:357:14: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
        p += sprintf(p, "%.2X ", buffer[i]);
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 backends/tpm/tpm_util.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 1856589c3b..0747af2d1c 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -21,6 +21,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "tpm_int.h"
@@ -336,27 +337,18 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
 void tpm_util_show_buffer(const unsigned char *buffer,
                           size_t buffer_size, const char *string)
 {
-    size_t len, i;
-    char *line_buffer, *p;
+    size_t len;
+    char *line, *lineup;
 
     if (!trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
         return;
     }
     len = MIN(tpm_cmd_get_size(buffer), buffer_size);
 
-    /*
-     * allocate enough room for 3 chars per buffer entry plus a
-     * newline after every 16 chars and a final null terminator.
-     */
-    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
+    line = qemu_hexdump_line(buffer, 0, len, false);
+    lineup = g_ascii_strup(line, -1);
+    trace_tpm_util_show_buffer(string, len, lineup);
 
-    for (i = 0, p = line_buffer; i < len; i++) {
-        if (i && !(i % 16)) {
-            p += sprintf(p, "\n");
-        }
-        p += sprintf(p, "%.2X ", buffer[i]);
-    }
-    trace_tpm_util_show_buffer(string, len, line_buffer);
-
-    g_free(line_buffer);
+    g_free(line);
+    g_free(lineup);
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation
  2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (11 preceding siblings ...)
  2024-04-10 16:06 ` [PATCH 12/12] backends/tpm: " Philippe Mathieu-Daudé
@ 2024-04-10 19:12 ` Richard Henderson
  2024-04-10 22:27   ` BALATON Zoltan
  12 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2024-04-10 19:12 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: qemu-ppc, qemu-arm, qemu-block

On 4/10/24 06:06, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.

Is snprintf also deprecated?
It might be easier to convert some of these fixed buffer cases that way, if allowed.


r~


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 12/12] backends/tpm: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-10 16:06 ` [PATCH 12/12] backends/tpm: " Philippe Mathieu-Daudé
@ 2024-04-10 19:51   ` Stefan Berger
  0 siblings, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2024-04-10 19:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Stefan Berger



On 4/10/24 12:06, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
> 
> Use qemu_hexdump_line() to avoid sprintf() calls, silencing:
> 
>    backends/tpm/tpm_util.c:357:14: warning: 'sprintf' is deprecated:
>      This function is provided for compatibility reasons only.
>      Due to security concerns inherent in the design of sprintf(3),
>      it is highly recommended that you use snprintf(3) instead.
>      [-Wdeprecated-declarations]
>          p += sprintf(p, "%.2X ", buffer[i]);
>               ^ >
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   backends/tpm/tpm_util.c | 24 ++++++++----------------
>   1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
> index 1856589c3b..0747af2d1c 100644
> --- a/backends/tpm/tpm_util.c
> +++ b/backends/tpm/tpm_util.c
> @@ -21,6 +21,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>   #include "qapi/error.h"
>   #include "qapi/visitor.h"
>   #include "tpm_int.h"
> @@ -336,27 +337,18 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
>   void tpm_util_show_buffer(const unsigned char *buffer,
>                             size_t buffer_size, const char *string)
>   {
> -    size_t len, i;
> -    char *line_buffer, *p;
> +    size_t len;
> +    char *line, *lineup;
>   
>       if (!trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
>           return;
>       }
>       len = MIN(tpm_cmd_get_size(buffer), buffer_size);
>   
> -    /*
> -     * allocate enough room for 3 chars per buffer entry plus a
> -     * newline after every 16 chars and a final null terminator.
> -     */
> -    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
> +    line = qemu_hexdump_line(buffer, 0, len, false);
> +    lineup = g_ascii_strup(line, -1);
> +    trace_tpm_util_show_buffer(string, len, lineup);
>   
> -    for (i = 0, p = line_buffer; i < len; i++) {
> -        if (i && !(i % 16)) {
> -            p += sprintf(p, "\n");
> -        }
> -        p += sprintf(p, "%.2X ", buffer[i]);
> -    }
> -    trace_tpm_util_show_buffer(string, len, line_buffer);
> -
> -    g_free(line_buffer);
> +    g_free(line);
> +    g_free(lineup);
>   }


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation
  2024-04-10 19:12 ` [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Richard Henderson
@ 2024-04-10 22:27   ` BALATON Zoltan
  2024-04-11  9:38     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 25+ messages in thread
From: BALATON Zoltan @ 2024-04-10 22:27 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-ppc, qemu-arm,
	qemu-block

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Wed, 10 Apr 2024, Richard Henderson wrote:
> On 4/10/24 06:06, Philippe Mathieu-Daudé wrote:
>> Hi,
>> 
>> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
>> resulting in painful developper experience.
>
> Is snprintf also deprecated?
> It might be easier to convert some of these fixed buffer cases that way, if 
> allowed.

I had the same thought as some of these might also have performance 
implications (although most of them are in rarely called places).

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 05/12] system/qtest: Replace sprintf() by g_string_append_printf()
  2024-04-10 16:06 ` [PATCH 05/12] system/qtest: " Philippe Mathieu-Daudé
@ 2024-04-11  6:30   ` Thomas Huth
  0 siblings, 0 replies; 25+ messages in thread
From: Thomas Huth @ 2024-04-11  6:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-ppc, qemu-arm, qemu-block, Laurent Vivier, Paolo Bonzini

On 10/04/2024 18.06, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
> 
> Replace sprintf() by GString API uses in order to avoid:
> 
>    [120/169] Compiling C object libcommon.fa.p/system_qtest.c.o
>    system/qtest.c:623:13: warning: 'sprintf' is deprecated:
>      This function is provided for compatibility reasons only.
>      Due to security concerns inherent in the design of sprintf(3),
>      it is highly recommended that you use snprintf(3) instead.
>      [-Wdeprecated-declarations]
>              sprintf(&enc[i * 2], "%02x", data[i]);
>              ^
>    1 warning generated.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/qtest.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
@ 2024-04-11  6:58   ` Marc-André Lureau
  2024-04-11  7:47   ` Gerd Hoffmann
  1 sibling, 0 replies; 25+ messages in thread
From: Marc-André Lureau @ 2024-04-11  6:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block, Gerd Hoffmann

On Wed, Apr 10, 2024 at 8:06 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
>
> Replace sprintf() by g_strdup_printf() in order to avoid:
>
>   [702/1310] Compiling C object libcommon.fa.p/ui_console-vc.c.o
>   ui/console-vc.c:824:21: warning: 'sprintf' is deprecated:
>     This function is provided for compatibility reasons only.
>     Due to security concerns inherent in the design of sprintf(3),
>     it is highly recommended that you use snprintf(3) instead.
>     [-Wdeprecated-declarations]
>                     sprintf(response, "\033[%d;%dR",
>                     ^
>   1 warning generated.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  ui/console-vc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/console-vc.c b/ui/console-vc.c
> index 899fa11c94..b455db436d 100644
> --- a/ui/console-vc.c
> +++ b/ui/console-vc.c
> @@ -648,7 +648,7 @@ static void vc_putchar(VCChardev *vc, int ch)
>      QemuTextConsole *s = vc->console;
>      int i;
>      int x, y;
> -    char response[40];
> +    g_autofree char *response = NULL;
>
>      switch(vc->state) {
>      case TTY_STATE_NORM:
> @@ -821,7 +821,7 @@ static void vc_putchar(VCChardev *vc, int ch)
>                      break;
>                  case 6:
>                      /* report cursor position */
> -                    sprintf(response, "\033[%d;%dR",
> +                    response = g_strdup_printf("\033[%d;%dR",
>                             (s->y_base + s->y) % s->total_height + 1,
>                              s->x + 1);
>                      vc_respond_str(vc, response);
> --
> 2.41.0
>



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
  2024-04-11  6:58   ` Marc-André Lureau
@ 2024-04-11  7:47   ` Gerd Hoffmann
  2024-04-11  9:36     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 25+ messages in thread
From: Gerd Hoffmann @ 2024-04-11  7:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block,
	Marc-André Lureau

  Hi,

>     Due to security concerns inherent in the design of sprintf(3),
>     it is highly recommended that you use snprintf(3) instead.

> -    char response[40];
> +    g_autofree char *response = NULL;

> -                    sprintf(response, "\033[%d;%dR",
> +                    response = g_strdup_printf("\033[%d;%dR",

Any specific reason why you don't go with the recommendation above?

While using g_strdup_printf() isn't wrong it allocates memory which
is not needed here because you can continue to use the stack buffer
this way:

	snprintf(response, sizeof(response), ...);

take care,
  Gerd



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()
  2024-04-11  7:47   ` Gerd Hoffmann
@ 2024-04-11  9:36     ` Philippe Mathieu-Daudé
  2024-04-11 10:02       ` Gerd Hoffmann
  0 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11  9:36 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block,
	Marc-André Lureau

On 11/4/24 09:47, Gerd Hoffmann wrote:
>    Hi,
> 
>>      Due to security concerns inherent in the design of sprintf(3),
>>      it is highly recommended that you use snprintf(3) instead.
> 
>> -    char response[40];
>> +    g_autofree char *response = NULL;
> 
>> -                    sprintf(response, "\033[%d;%dR",
>> +                    response = g_strdup_printf("\033[%d;%dR",
> 
> Any specific reason why you don't go with the recommendation above?
> 
> While using g_strdup_printf() isn't wrong it allocates memory which
> is not needed here because you can continue to use the stack buffer
> this way:
> 
> 	snprintf(response, sizeof(response), ...);

I thought GLib/GString was recommended for formatting, so choose
this thinking mostly about style. Indeed in this case snprintf()
is sufficient. I'll respin, thanks.

> 
> take care,
>    Gerd
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation
  2024-04-10 22:27   ` BALATON Zoltan
@ 2024-04-11  9:38     ` Philippe Mathieu-Daudé
  2024-04-19 10:48       ` Daniel P. Berrangé
  0 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11  9:38 UTC (permalink / raw)
  To: BALATON Zoltan, Richard Henderson, Alex Bennée,
	Daniel P. Berrangé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block

On 11/4/24 00:27, BALATON Zoltan wrote:
> On Wed, 10 Apr 2024, Richard Henderson wrote:
>> On 4/10/24 06:06, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
>>> resulting in painful developper experience.
>>
>> Is snprintf also deprecated?
>> It might be easier to convert some of these fixed buffer cases that 
>> way, if allowed.
> 
> I had the same thought as some of these might also have performance 
> implications (although most of them are in rarely called places).

I thought GLib/GString was recommended for formatting (IIRC some
previous discussion with Alex / Daniel), so I switched to this
API for style, rather than thinking of performance. Anyway, I'll
respin using sprintf() when the buffer size maths are already done.

> 
> Regards,
> BALATON Zoltan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()
  2024-04-11  9:36     ` Philippe Mathieu-Daudé
@ 2024-04-11 10:02       ` Gerd Hoffmann
  0 siblings, 0 replies; 25+ messages in thread
From: Gerd Hoffmann @ 2024-04-11 10:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block,
	Marc-André Lureau

On Thu, Apr 11, 2024 at 11:36:10AM +0200, Philippe Mathieu-Daudé wrote:
> On 11/4/24 09:47, Gerd Hoffmann wrote:
> >    Hi,
> > 
> > >      Due to security concerns inherent in the design of sprintf(3),
> > >      it is highly recommended that you use snprintf(3) instead.
> > 
> > > -    char response[40];
> > > +    g_autofree char *response = NULL;
> > 
> > > -                    sprintf(response, "\033[%d;%dR",
> > > +                    response = g_strdup_printf("\033[%d;%dR",
> > 
> > Any specific reason why you don't go with the recommendation above?
> > 
> > While using g_strdup_printf() isn't wrong it allocates memory which
> > is not needed here because you can continue to use the stack buffer
> > this way:
> > 
> > 	snprintf(response, sizeof(response), ...);
> 
> I thought GLib/GString was recommended for formatting,

If you allocate the output buffer anyway (and there are patches in this
series where this is the case) it's clearly better to use
g_strdup_printf instead of malloc + snprintf.

In case a fixed-size buffer can be used I wouldn't switch to dynamic
allocation ...

take care,
  Gerd



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 02/12] hw/vfio/pci: Replace sprintf() by g_strdup_printf()
  2024-04-10 16:06 ` [PATCH 02/12] hw/vfio/pci: " Philippe Mathieu-Daudé
@ 2024-04-12 15:25   ` Alex Williamson
  2024-04-15 14:01     ` Cédric Le Goater
  0 siblings, 1 reply; 25+ messages in thread
From: Alex Williamson @ 2024-04-12 15:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block, Cédric Le Goater

On Wed, 10 Apr 2024 18:06:03 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience. Use g_strdup_printf()
> instead.

Isn't this code only compiled for Linux hosts?  Maybe still a valid
change, but the rationale seems irrelevant.  Thanks,

Alex

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/vfio/pci.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 64780d1b79..cc3cc89122 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2442,10 +2442,9 @@ void vfio_pci_post_reset(VFIOPCIDevice *vdev)
>  
>  bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
>  {
> -    char tmp[13];
> -
> -    sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
> -            addr->bus, addr->slot, addr->function);
> +    g_autofree char *tmp = g_strdup_printf("%04x:%02x:%02x.%1x",
> +                                           addr->domain, addr->bus,
> +                                           addr->slot, addr->function);
>  
>      return (strcmp(tmp, name) == 0);
>  }



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 02/12] hw/vfio/pci: Replace sprintf() by g_strdup_printf()
  2024-04-12 15:25   ` Alex Williamson
@ 2024-04-15 14:01     ` Cédric Le Goater
  0 siblings, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2024-04-15 14:01 UTC (permalink / raw)
  To: Alex Williamson, Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-ppc, qemu-arm, qemu-block

On 4/12/24 17:25, Alex Williamson wrote:
> On Wed, 10 Apr 2024 18:06:03 +0200
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
>> resulting in painful developper experience. Use g_strdup_printf()
>> instead.
> 
> Isn't this code only compiled for Linux hosts?  

It is not.

> Maybe still a valid change, but the rationale seems irrelevant.

I agree the commit log should be rephrased.

There is also a v2 doing a different change :

   https://lore.kernel.org/qemu-devel/20240411101550.99392-1-philmd@linaro.org/

This is a bit confusing.

Thanks,

C.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation
  2024-04-11  9:38     ` Philippe Mathieu-Daudé
@ 2024-04-19 10:48       ` Daniel P. Berrangé
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2024-04-19 10:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: BALATON Zoltan, Richard Henderson, Alex Bennée, qemu-devel,
	qemu-ppc, qemu-arm, qemu-block

On Thu, Apr 11, 2024 at 11:38:41AM +0200, Philippe Mathieu-Daudé wrote:
> On 11/4/24 00:27, BALATON Zoltan wrote:
> > On Wed, 10 Apr 2024, Richard Henderson wrote:
> > > On 4/10/24 06:06, Philippe Mathieu-Daudé wrote:
> > > > Hi,
> > > > 
> > > > sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> > > > resulting in painful developper experience.
> > > 
> > > Is snprintf also deprecated?
> > > It might be easier to convert some of these fixed buffer cases that
> > > way, if allowed.
> > 
> > I had the same thought as some of these might also have performance
> > implications (although most of them are in rarely called places).
> 
> I thought GLib/GString was recommended for formatting (IIRC some
> previous discussion with Alex / Daniel), so I switched to this
> API for style, rather than thinking of performance. Anyway, I'll
> respin using sprintf() when the buffer size maths are already done.

There are places in QEMU where the strings end up living in a fixed
size struct fields, and those would be candidates for sticking with
snprint().

For stack allocated string buffers, it is preferrable to switch to
g_autofree + g_strdup_printf(), unless there's a compelling performance
reason to avoid allocation in a hot path IMHO.

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] 25+ messages in thread

end of thread, other threads:[~2024-04-19 10:49 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 16:06 [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf() Philippe Mathieu-Daudé
2024-04-11  6:58   ` Marc-André Lureau
2024-04-11  7:47   ` Gerd Hoffmann
2024-04-11  9:36     ` Philippe Mathieu-Daudé
2024-04-11 10:02       ` Gerd Hoffmann
2024-04-10 16:06 ` [PATCH 02/12] hw/vfio/pci: " Philippe Mathieu-Daudé
2024-04-12 15:25   ` Alex Williamson
2024-04-15 14:01     ` Cédric Le Goater
2024-04-10 16:06 ` [PATCH 03/12] hw/ppc/spapr: " Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 04/12] hw/mips/malta: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 05/12] system/qtest: " Philippe Mathieu-Daudé
2024-04-11  6:30   ` Thomas Huth
2024-04-10 16:06 ` [PATCH 06/12] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 07/12] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 08/12] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 09/12] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 10/12] hw/ide/atapi: " Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 11/12] hw/dma/pl330: " Philippe Mathieu-Daudé
2024-04-10 16:06 ` [PATCH 12/12] backends/tpm: " Philippe Mathieu-Daudé
2024-04-10 19:51   ` Stefan Berger
2024-04-10 19:12 ` [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation Richard Henderson
2024-04-10 22:27   ` BALATON Zoltan
2024-04-11  9:38     ` Philippe Mathieu-Daudé
2024-04-19 10:48       ` Daniel P. Berrangé

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).