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

Since v1:
- Use snprintf() in patches 1-5

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é (13):
  ui/console-vc: Replace sprintf() by snprintf()
  hw/vfio/pci: Replace sprintf() by snprintf()
  hw/ppc/spapr: Replace sprintf() by snprintf()
  hw/mips/malta: Add re-usable rng_seed_hex_new() method
  hw/mips/malta: Replace sprintf() by snprintf()
  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         | 23 ++++++++++++++---------
 hw/ppc/spapr.c          |  2 +-
 hw/scsi/scsi-disk.c     |  8 ++------
 hw/vfio/pci.c           |  2 +-
 hw/virtio/vhost-vdpa.c  | 11 ++++++-----
 system/qtest.c          |  8 +++-----
 ui/console-vc.c         |  2 +-
 util/hexdump.c          | 33 ++++++++++++++++++---------------
 12 files changed, 73 insertions(+), 77 deletions(-)

-- 
2.41.0



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

* [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 10:21   ` Marc-André Lureau
  2024-04-11 19:27   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 02/13] hw/vfio/pci: " Philippe Mathieu-Daudé
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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 snprintf() 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 899fa11c94..847d5fb174 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -821,7 +821,7 @@ static void vc_putchar(VCChardev *vc, int ch)
                     break;
                 case 6:
                     /* report cursor position */
-                    sprintf(response, "\033[%d;%dR",
+                    snprintf(response, sizeof(response), "\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] 27+ messages in thread

* [PATCH v2 02/13] hw/vfio/pci: Replace sprintf() by snprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 19:42   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 03/13] hw/ppc/spapr: " Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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 snprintf() instead.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/vfio/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 64780d1b79..cc545bcfbe 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2444,7 +2444,7 @@ bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
 {
     char tmp[13];
 
-    sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
+    snprintf(tmp, sizeof(tmp), "%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] 27+ messages in thread

* [PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 02/13] hw/vfio/pci: " Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 19:47   ` Richard Henderson
  2024-04-15  6:44   ` Harsh Prateek Bora
  2024-04-11 10:15 ` [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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 snprintf() 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e9bc97fee0..9e97992c79 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -382,7 +382,7 @@ static int spapr_dt_memory_node(SpaprMachineState *spapr, void *fdt, int nodeid,
     mem_reg_property[0] = cpu_to_be64(start);
     mem_reg_property[1] = cpu_to_be64(size);
 
-    sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
+    snprintf(mem_name, sizeof(mem_name), "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] 27+ messages in thread

* [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 03/13] hw/ppc/spapr: " Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:07   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 05/13] hw/mips/malta: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Philippe Mathieu-Daudé,
	Aurelien Jarno, Jiaxun Yang

Extract common code from reinitialize_rng_seed() and
load_kernel() to rng_seed_hex_new().

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

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index af74008c82..9fc6a7d313 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 *rng_seed_hex_new(void)
 {
-    char *rng_seed_hex = opaque;
     uint8_t rng_seed[32];
+    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
 
     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]);
     }
+
+    return g_strdup(rng_seed_hex);
+}
+
+static void reinitialize_rng_seed(void *opaque)
+{
+    g_autofree char *rng_seed_hex = rng_seed_hex_new();
+
+    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 = rng_seed_hex_new();
     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] 27+ messages in thread

* [PATCH v2 05/13] hw/mips/malta: Replace sprintf() by snprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Philippe Mathieu-Daudé,
	Aurelien Jarno, Jiaxun Yang

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

Replace sprintf() by snprintf() 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]

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

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9fc6a7d313..5d33aa5123 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -857,7 +857,8 @@ static char *rng_seed_hex_new(void)
 
     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]);
+        snprintf(rng_seed_hex + i * 2, sizeof(rng_seed_hex) - i * 2,
+                 "%02x", rng_seed[i]);
     }
 
     return g_strdup(rng_seed_hex);
-- 
2.41.0



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

* [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 05/13] hw/mips/malta: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:13   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 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] 27+ messages in thread

* [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:34   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:47   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:43   ` Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel; +Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 20:53   ` Richard Henderson
  2024-04-11 10:15 ` [PATCH v2 11/13] hw/ide/atapi: " Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 11/13] hw/ide/atapi: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 12/13] hw/dma/pl330: " Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 13/13] backends/tpm: " Philippe Mathieu-Daudé
  12 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 12/13] hw/dma/pl330: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 11/13] hw/ide/atapi: " Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  2024-04-11 10:15 ` [PATCH v2 13/13] backends/tpm: " Philippe Mathieu-Daudé
  12 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, 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] 27+ messages in thread

* [PATCH v2 13/13] backends/tpm: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
                   ` (11 preceding siblings ...)
  2024-04-11 10:15 ` [PATCH v2 12/13] hw/dma/pl330: " Philippe Mathieu-Daudé
@ 2024-04-11 10:15 ` Philippe Mathieu-Daudé
  12 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 10:15 UTC (permalink / raw
  To: qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Philippe Mathieu-Daudé,
	Stefan Berger, 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>
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);
 }
-- 
2.41.0



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

* Re: [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()
  2024-04-11 10:15 ` [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
@ 2024-04-11 10:21   ` Marc-André Lureau
  2024-04-11 19:27   ` Richard Henderson
  1 sibling, 0 replies; 27+ messages in thread
From: Marc-André Lureau @ 2024-04-11 10:21 UTC (permalink / raw
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-arm, qemu-block, qemu-ppc, Gerd Hoffmann

On Thu, Apr 11, 2024 at 2:16 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 snprintf() 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/console-vc.c b/ui/console-vc.c
> index 899fa11c94..847d5fb174 100644
> --- a/ui/console-vc.c
> +++ b/ui/console-vc.c
> @@ -821,7 +821,7 @@ static void vc_putchar(VCChardev *vc, int ch)
>                      break;
>                  case 6:
>                      /* report cursor position */
> -                    sprintf(response, "\033[%d;%dR",
> +                    snprintf(response, sizeof(response), "\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] 27+ messages in thread

* Re: [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()
  2024-04-11 10:15 ` [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
  2024-04-11 10:21   ` Marc-André Lureau
@ 2024-04-11 19:27   ` Richard Henderson
  1 sibling, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 19:27 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Gerd Hoffmann,
	Marc-André Lureau

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
> 
> Replace sprintf() by snprintf() 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 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH v2 02/13] hw/vfio/pci: Replace sprintf() by snprintf()
  2024-04-11 10:15 ` [PATCH v2 02/13] hw/vfio/pci: " Philippe Mathieu-Daudé
@ 2024-04-11 19:42   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 19:42 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Alex Williamson,
	Cédric Le Goater

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience. Use snprintf() instead.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/vfio/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()
  2024-04-11 10:15 ` [PATCH v2 03/13] hw/ppc/spapr: " Philippe Mathieu-Daudé
@ 2024-04-11 19:47   ` Richard Henderson
  2024-04-15  6:44   ` Harsh Prateek Bora
  1 sibling, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 19:47 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Nicholas Piggin,
	Daniel Henrique Barboza, David Gibson, Harsh Prateek Bora

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
> 
> Replace sprintf() by snprintf() 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 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method
  2024-04-11 10:15 ` [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method Philippe Mathieu-Daudé
@ 2024-04-11 20:07   ` Richard Henderson
  2024-04-11 20:31     ` Richard Henderson
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:07 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Aurelien Jarno, Jiaxun Yang

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> Extract common code from reinitialize_rng_seed() and
> load_kernel() to rng_seed_hex_new().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/mips/malta.c | 20 ++++++++++++--------
>   1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index af74008c82..9fc6a7d313 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 *rng_seed_hex_new(void)
>   {
> -    char *rng_seed_hex = opaque;
>       uint8_t rng_seed[32];
> +    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
>   
>       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]);
>       }
> +
> +    return g_strdup(rng_seed_hex);
> +}
> +
> +static void reinitialize_rng_seed(void *opaque)
> +{
> +    g_autofree char *rng_seed_hex = rng_seed_hex_new();
> +
> +    strcpy(opaque, rng_seed_hex);
>   }

Though it isn't deprecated, strcpy isn't really any safer than sprintf.
We don't need to be copying text around quite as much as this.

How about:

#define RNG_SEED_SIZE 32

static void rng_seed_hex_new(char buf[2 * RNG_SEED_SIZE + 1])
{
     static const char hex = "0123456789abcdef";
     uint8_t rng_seed[RNG_SEED_SIZE];

     qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
     for (int i = 0; i < RNG_SEED_SIZE; ++i) {
         buf[i * 2 + 0] = hex[rng_seed[i] / 16];
         buf[i * 2 + 1] = hex[rng_seed[i] % 16];
     }
     buf[RNG_SEED_SIZE * 2] = '\0';
}

static void reinitialize_rng_seed(void *opaque)
{
     rng_seed_hex_new(opaque);
}

with little change in load_kernel.


r~


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

* Re: [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf()
  2024-04-11 10:15 ` [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-11 20:13   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:13 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Thomas Huth, Laurent Vivier,
	Paolo Bonzini

On 4/11/24 03:15, 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>
> Reviewed-by: Thomas Huth<thuth@redhat.com>
> ---
>   system/qtest.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method
  2024-04-11 20:07   ` Richard Henderson
@ 2024-04-11 20:31     ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:31 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Aurelien Jarno, Jiaxun Yang

On 4/11/24 13:07, Richard Henderson wrote:
> On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
>> Extract common code from reinitialize_rng_seed() and
>> load_kernel() to rng_seed_hex_new().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/mips/malta.c | 20 ++++++++++++--------
>>   1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>> index af74008c82..9fc6a7d313 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 *rng_seed_hex_new(void)
>>   {
>> -    char *rng_seed_hex = opaque;
>>       uint8_t rng_seed[32];
>> +    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
>>       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]);
>>       }
>> +
>> +    return g_strdup(rng_seed_hex);
>> +}
>> +
>> +static void reinitialize_rng_seed(void *opaque)
>> +{
>> +    g_autofree char *rng_seed_hex = rng_seed_hex_new();
>> +
>> +    strcpy(opaque, rng_seed_hex);
>>   }
> 
> Though it isn't deprecated, strcpy isn't really any safer than sprintf.
> We don't need to be copying text around quite as much as this.
> 
> How about:
> 
> #define RNG_SEED_SIZE 32
> 
> static void rng_seed_hex_new(char buf[2 * RNG_SEED_SIZE + 1])
> {
>      static const char hex = "0123456789abcdef";
>      uint8_t rng_seed[RNG_SEED_SIZE];
> 
>      qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
>      for (int i = 0; i < RNG_SEED_SIZE; ++i) {
>          buf[i * 2 + 0] = hex[rng_seed[i] / 16];
>          buf[i * 2 + 1] = hex[rng_seed[i] % 16];

Hmm.  Maybe a

static inline char hexdump_nibble(unsigned val)
{
     return (val < 10 ? '0' : 'a') + val;
}

static inline void hexdump_byte(char *out, uint8_t byte)
{
     out[0] = hexdump_nibble(byte >> 4);
     out[1] = hexdump_nibble(byte & 15);
}

in "qemu/cutils.h", for use elsewhere including util/hexdump.c.


r~


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

* Re: [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line()
  2024-04-11 10:15 ` [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
@ 2024-04-11 20:34   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:34 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Michael S. Tsirkin

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
> @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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()
  2024-04-11 10:15 ` [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
@ 2024-04-11 20:43   ` Philippe Mathieu-Daudé
  2024-04-11 20:50     ` Richard Henderson
  0 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-11 20:43 UTC (permalink / raw
  To: qemu-devel; +Cc: qemu-arm, qemu-block, qemu-ppc

On 11/4/24 12:15, 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 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]);

I find using g_string_append_printf() simpler than checking snprintf()
return value, and don't expect this function to be in hot path, but if
preferred I can try to not use the GString API.

>           } else {
> -            line += sprintf(line, "   ");
> +            g_string_append(gs, "   ");
>           }
>       }



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

* Re: [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer
  2024-04-11 10:15 ` [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
@ 2024-04-11 20:47   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:47 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Michael S. Tsirkin

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~


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

* Re: [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()
  2024-04-11 20:43   ` Philippe Mathieu-Daudé
@ 2024-04-11 20:50     ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:50 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: qemu-arm, qemu-block, qemu-ppc

On 4/11/24 13:43, Philippe Mathieu-Daudé wrote:
> On 11/4/24 12:15, 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 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]);
> 
> I find using g_string_append_printf() simpler than checking snprintf()
> return value, and don't expect this function to be in hot path, but if
> preferred I can try to not use the GString API.

GString api is pretty good.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


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

* Re: [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()
  2024-04-11 10:15 ` [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
@ 2024-04-11 20:53   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2024-04-11 20:53 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Paolo Bonzini, Fam Zheng

On 4/11/24 03:15, 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:
> 
>    [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);

This is adding "0000: " as an unnecessary prefix, because it's added by qemu_hexdump_line.
I think having qemu_hexdump_line as a primitive is good, but probably the offset argument 
should be dropped and printed by the two callers that need it (mostly qemu_hexdump).


r~


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

* Re: [PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()
  2024-04-11 10:15 ` [PATCH v2 03/13] hw/ppc/spapr: " Philippe Mathieu-Daudé
  2024-04-11 19:47   ` Richard Henderson
@ 2024-04-15  6:44   ` Harsh Prateek Bora
  1 sibling, 0 replies; 27+ messages in thread
From: Harsh Prateek Bora @ 2024-04-15  6:44 UTC (permalink / raw
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-arm, qemu-block, qemu-ppc, Nicholas Piggin,
	Daniel Henrique Barboza, David Gibson



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

s/developper/developer ?

> 
> Replace sprintf() by snprintf() 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>

With the typo fixed,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> ---
>   hw/ppc/spapr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e9bc97fee0..9e97992c79 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -382,7 +382,7 @@ static int spapr_dt_memory_node(SpaprMachineState *spapr, void *fdt, int nodeid,
>       mem_reg_property[0] = cpu_to_be64(start);
>       mem_reg_property[1] = cpu_to_be64(size);
>   
> -    sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
> +    snprintf(mem_name, sizeof(mem_name), "memory@%" HWADDR_PRIx, start);
>       off = fdt_add_subnode(fdt, 0, mem_name);
>       _FDT(off);
>       _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));


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

end of thread, other threads:[~2024-04-15  6:46 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-11 10:15 [PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation Philippe Mathieu-Daudé
2024-04-11 10:15 ` [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
2024-04-11 10:21   ` Marc-André Lureau
2024-04-11 19:27   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 02/13] hw/vfio/pci: " Philippe Mathieu-Daudé
2024-04-11 19:42   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 03/13] hw/ppc/spapr: " Philippe Mathieu-Daudé
2024-04-11 19:47   ` Richard Henderson
2024-04-15  6:44   ` Harsh Prateek Bora
2024-04-11 10:15 ` [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method Philippe Mathieu-Daudé
2024-04-11 20:07   ` Richard Henderson
2024-04-11 20:31     ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 05/13] hw/mips/malta: Replace sprintf() by snprintf() Philippe Mathieu-Daudé
2024-04-11 10:15 ` [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
2024-04-11 20:13   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line() Philippe Mathieu-Daudé
2024-04-11 20:34   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 08/13] util/hexdump: Have qemu_hexdump_line() return heap allocated buffer Philippe Mathieu-Daudé
2024-04-11 20:47   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf() Philippe Mathieu-Daudé
2024-04-11 20:43   ` Philippe Mathieu-Daudé
2024-04-11 20:50     ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf() Philippe Mathieu-Daudé
2024-04-11 20:53   ` Richard Henderson
2024-04-11 10:15 ` [PATCH v2 11/13] hw/ide/atapi: " Philippe Mathieu-Daudé
2024-04-11 10:15 ` [PATCH v2 12/13] hw/dma/pl330: " Philippe Mathieu-Daudé
2024-04-11 10:15 ` [PATCH v2 13/13] backends/tpm: " Philippe Mathieu-Daudé

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 read-only IMAP folder(s) and NNTP newsgroup(s).