Re: [PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()

2024-04-11 Thread Richard Henderson 

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é 
---
  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 ": " 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~

Re: [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread Richard Henderson 

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é 
---
  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 


r~

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

2024-04-11 Thread Richard Henderson 

On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  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~

Re: [PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread Philippe Mathieu-Daudé 

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é 
---
  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, "   ");
  }
  }

Re: [PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line()

2024-04-11 Thread Richard Henderson 

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é
---
  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 

r~

Re: [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method

2024-04-11 Thread Richard Henderson 

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é 
---
  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~

Re: [PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread Richard Henderson 

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([i * 2], "%02x", data[i]);
 ^
   1 warning generated.

Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
---
  system/qtest.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method

2024-04-11 Thread Richard Henderson 

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é 
---
  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~

Re: [PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()

2024-04-11 Thread Richard Henderson 

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é
---
  hw/ppc/spapr.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 02/13] hw/vfio/pci: Replace sprintf() by snprintf()

2024-04-11 Thread Richard Henderson 

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é
---
  hw/vfio/pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()

2024-04-11 Thread Richard Henderson 

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é
---
  ui/console-vc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-04-11 Thread Yu Zhang 
> 1) Either a CI test covering at least the major RDMA paths, or at least
> periodically tests for each QEMU release will be needed.
We use a batch of regression test cases for the stack, which covers the
test for QEMU. I did such test for most of the QEMU releases planned as
candidates for rollout.

The migration test needs a pair of (either physical or virtual) servers with
InfiniBand network, which makes it difficult to do on a single server. The
nested VM could be a possible approach, for which we may need virtual
InfiniBand network. Is SoftRoCE [1] a choice? I will try it and let you know.

[1]  https://enterprise-support.nvidia.com/s/article/howto-configure-soft-roce

Thanks and best regards!

On Thu, Apr 11, 2024 at 4:20 PM Peter Xu  wrote:
>
> On Wed, Apr 10, 2024 at 09:49:15AM -0400, Peter Xu wrote:
> > On Wed, Apr 10, 2024 at 02:28:59AM +, Zhijian Li (Fujitsu) via wrote:
> > >
> > >
> > > on 4/10/2024 3:46 AM, Peter Xu wrote:
> > >
> > > >> Is there document/link about the unittest/CI for migration tests, Why
> > > >> are those tests missing?
> > > >> Is it hard or very special to set up an environment for that? maybe we
> > > >> can help in this regards.
> > > > See tests/qtest/migration-test.c.  We put most of our migration tests
> > > > there and that's covered in CI.
> > > >
> > > > I think one major issue is CI systems don't normally have rdma devices.
> > > > Can rdma migration test be carried out without a real hardware?
> > >
> > > Yeah,  RXE aka. SOFT-RoCE is able to emulate the RDMA, for example
> > > $ sudo rdma link add rxe_eth0 type rxe netdev eth0  # on host
> > > then we can get a new RDMA interface "rxe_eth0".
> > > This new RDMA interface is able to do the QEMU RDMA migration.
> > >
> > > Also, the loopback(lo) device is able to emulate the RDMA interface
> > > "rxe_lo", however when
> > > I tried(years ago) to do RDMA migration over this
> > > interface(rdma:127.0.0.1:) , it got something wrong.
> > > So i gave up enabling the RDMA migration qtest at that time.
> >
> > Thanks, Zhijian.
> >
> > I'm not sure adding an emu-link for rdma is doable for CI systems, though.
> > Maybe someone more familiar with how CI works can chim in.
>
> Some people got dropped on the cc list for unknown reason, I'm adding them
> back (Fabiano, Peter Maydell, Phil).  Let's make sure nobody is dropped by
> accident.
>
> I'll try to summarize what is still missing, and I think these will be
> greatly helpful if we don't want to deprecate rdma migration:
>
>   1) Either a CI test covering at least the major RDMA paths, or at least
>  periodically tests for each QEMU release will be needed.
>
>   2) Some performance tests between modern RDMA and NIC devices are
>  welcomed.  The current knowledge is modern NIC can work similarly to
>  RDMA in performance, then it's debatable why we still maintain so much
>  rdma specific code.
>
>   3) No need to be soild patchsets for this one, but some plan to improve
>  RDMA migration code so that it is not almost isolated from the rest
>  protocols.
>
>   4) Someone to look after this code for real.
>
> For 2) and 3) more info is here:
>
> https://lore.kernel.org/r/ZhWa0YeAb9ySVKD1@x1n
>
> Here 4) can be the most important as Markus pointed out.  We just didn't
> get there yet on the discussions, but maybe Markus is right that we should
> talk that first.
>
> Thanks,
>
> --
> Peter Xu
>

Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-04-11 Thread Jinpu Wang 
Hi Peter,

On Tue, Apr 9, 2024 at 9:47 PM Peter Xu  wrote:
>
> On Tue, Apr 09, 2024 at 09:32:46AM +0200, Jinpu Wang wrote:
> > Hi Peter,
> >
> > On Mon, Apr 8, 2024 at 6:18 PM Peter Xu  wrote:
> > >
> > > On Mon, Apr 08, 2024 at 04:07:20PM +0200, Jinpu Wang wrote:
> > > > Hi Peter,
> > >
> > > Jinpu,
> > >
> > > Thanks for joining the discussion.
> > >
> > > >
> > > > On Tue, Apr 2, 2024 at 11:24 PM Peter Xu  wrote:
> > > > >
> > > > > On Mon, Apr 01, 2024 at 11:26:25PM +0200, Yu Zhang wrote:
> > > > > > Hello Peter und Zhjian,
> > > > > >
> > > > > > Thank you so much for letting me know about this. I'm also a bit 
> > > > > > surprised at
> > > > > > the plan for deprecating the RDMA migration subsystem.
> > > > >
> > > > > It's not too late, since it looks like we do have users not yet 
> > > > > notified
> > > > > from this, we'll redo the deprecation procedure even if it'll be the 
> > > > > final
> > > > > plan, and it'll be 2 releases after this.
> > > > >
> > > > > >
> > > > > > > IMHO it's more important to know whether there are still users 
> > > > > > > and whether
> > > > > > > they would still like to see it around.
> > > > > >
> > > > > > > I admit RDMA migration was lack of testing(unit/CI test), which 
> > > > > > > led to the a few
> > > > > > > obvious bugs being noticed too late.
> > > > > >
> > > > > > Yes, we are a user of this subsystem. I was unaware of the lack of 
> > > > > > test coverage
> > > > > > for this part. As soon as 8.2 was released, I saw that many of the
> > > > > > migration test
> > > > > > cases failed and came to realize that there might be a bug between 
> > > > > > 8.1
> > > > > > and 8.2, but
> > > > > > was unable to confirm and report it quickly to you.
> > > > > >
> > > > > > The maintenance of this part could be too costly or difficult from
> > > > > > your point of view.
> > > > >
> > > > > It may or may not be too costly, it's just that we need real users of 
> > > > > RDMA
> > > > > taking some care of it.  Having it broken easily for >1 releases 
> > > > > definitely
> > > > > is a sign of lack of users.  It is an implication to the community 
> > > > > that we
> > > > > should consider dropping some features so that we can get the best 
> > > > > use of
> > > > > the community resources for the things that may have a broader 
> > > > > audience.
> > > > >
> > > > > One thing majorly missing is a RDMA tester to guard all the merges to 
> > > > > not
> > > > > break RDMA paths, hopefully in CI.  That should not rely on RDMA 
> > > > > hardwares
> > > > > but just to sanity check the migration+rdma code running all fine.  
> > > > > RDMA
> > > > > taught us the lesson so we're requesting CI coverage for all other new
> > > > > features that will be merged at least for migration subsystem, so 
> > > > > that we
> > > > > plan to not merge anything that is not covered by CI unless extremely
> > > > > necessary in the future.
> > > > >
> > > > > For sure CI is not the only missing part, but I'd say we should start 
> > > > > with
> > > > > it, then someone should also take care of the code even if only in
> > > > > maintenance mode (no new feature to add on top).
> > > > >
> > > > > >
> > > > > > My concern is, this plan will forces a few QEMU users (not sure how
> > > > > > many) like us
> > > > > > either to stick to the RDMA migration by using an increasingly older
> > > > > > version of QEMU,
> > > > > > or to abandon the currently used RDMA migration.
> > > > >
> > > > > RDMA doesn't get new features anyway, if there's specific use case 
> > > > > for RDMA
> > > > > migrations, would it work if such a scenario uses the old binary?  Is 
> > > > > it
> > > > > possible to switch to the TCP protocol with some good NICs?
> > > > We have used rdma migration with HCA from Nvidia for years, our
> > > > experience is RDMA migration works better than tcp (over ipoib).
> > >
> > > Please bare with me, as I know little on rdma stuff.
> > >
> > > I'm actually pretty confused (and since a long time ago..) on why we need
> > > to operation with rdma contexts when ipoib seems to provide all the tcp
> > > layers.  I meant, can it work with the current "tcp:" protocol with ipoib
> > > even if there's rdma/ib hardwares underneath?  Is it because of 
> > > performance
> > > improvements so that we must use a separate path comparing to generic
> > > "tcp:" protocol here?
> > using rdma protocol with ib verbs , we can leverage the full benefit of 
> > RDMA by
> > talking directly to NIC which bypasses the kernel overhead, less cpu
> > utilization and better performance.
> >
> > While IPoIB is more for compatibility to  applications using tcp, but
> > can't get full benefit of RDMA.  When you have mix generation of IB
> > devices, there are performance issue on IPoIB, we've seen 40G HCA can
> > only reach 2 Gb/s on IPoIB, but with raw RDMA can reach full line
> > speed.
> >
> > I just run a simple iperf3 test via ipoib and ib_send_bw on same hosts:
> >
> > iperf 3.9
> >

Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-04-11 Thread Peter Xu 
On Wed, Apr 10, 2024 at 09:49:15AM -0400, Peter Xu wrote:
> On Wed, Apr 10, 2024 at 02:28:59AM +, Zhijian Li (Fujitsu) via wrote:
> > 
> > 
> > on 4/10/2024 3:46 AM, Peter Xu wrote:
> > 
> > >> Is there document/link about the unittest/CI for migration tests, Why
> > >> are those tests missing?
> > >> Is it hard or very special to set up an environment for that? maybe we
> > >> can help in this regards.
> > > See tests/qtest/migration-test.c.  We put most of our migration tests
> > > there and that's covered in CI.
> > >
> > > I think one major issue is CI systems don't normally have rdma devices.
> > > Can rdma migration test be carried out without a real hardware?
> > 
> > Yeah,  RXE aka. SOFT-RoCE is able to emulate the RDMA, for example
> > $ sudo rdma link add rxe_eth0 type rxe netdev eth0  # on host
> > then we can get a new RDMA interface "rxe_eth0".
> > This new RDMA interface is able to do the QEMU RDMA migration.
> > 
> > Also, the loopback(lo) device is able to emulate the RDMA interface 
> > "rxe_lo", however when
> > I tried(years ago) to do RDMA migration over this 
> > interface(rdma:127.0.0.1:) , it got something wrong.
> > So i gave up enabling the RDMA migration qtest at that time.
> 
> Thanks, Zhijian.
> 
> I'm not sure adding an emu-link for rdma is doable for CI systems, though.
> Maybe someone more familiar with how CI works can chim in.

Some people got dropped on the cc list for unknown reason, I'm adding them
back (Fabiano, Peter Maydell, Phil).  Let's make sure nobody is dropped by
accident.

I'll try to summarize what is still missing, and I think these will be
greatly helpful if we don't want to deprecate rdma migration:

  1) Either a CI test covering at least the major RDMA paths, or at least
 periodically tests for each QEMU release will be needed.

  2) Some performance tests between modern RDMA and NIC devices are
 welcomed.  The current knowledge is modern NIC can work similarly to
 RDMA in performance, then it's debatable why we still maintain so much
 rdma specific code.

  3) No need to be soild patchsets for this one, but some plan to improve
 RDMA migration code so that it is not almost isolated from the rest
 protocols.

  4) Someone to look after this code for real.

For 2) and 3) more info is here:

https://lore.kernel.org/r/ZhWa0YeAb9ySVKD1@x1n

Here 4) can be the most important as Markus pointed out.  We just didn't
get there yet on the discussions, but maybe Markus is right that we should
talk that first.

Thanks,

-- 
Peter Xu

Re: [PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()

2024-04-11 Thread Marc-André Lureau 
On Thu, Apr 11, 2024 at 2:16 PM 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é 

Reviewed-by: Marc-André Lureau 

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

[PATCH v2 09/13] util/hexdump: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread 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é 
---
 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

[PATCH v2 05/13] hw/mips/malta: Replace sprintf() by snprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 13/13] backends/tpm: Use qemu_hexdump_line() to avoid sprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
Reviewed-by: Stefan Berger 
---
 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

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

2024-04-11 Thread Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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

[PATCH v2 12/13] hw/dma/pl330: Use qemu_hexdump_line() to avoid sprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 02/13] hw/vfio/pci: Replace sprintf() by snprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 11/13] hw/ide/atapi: Use qemu_hexdump_line() to avoid sprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 07/13] util/hexdump: Rename @offset argument in qemu_hexdump_line()

2024-04-11 Thread Philippe Mathieu-Daudé 
@offset argument is more descriptive than @b.

Inverse @bufptr <-> @offset arguments order.

Document qemu_hexdump_line().

Signed-off-by: Philippe Mathieu-Daudé 
---
 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

[PATCH v2 00/13] misc: Remove sprintf() due to macOS deprecation

2024-04-11 Thread Philippe Mathieu-Daudé 
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

[PATCH v2 10/13] hw/scsi/scsi-disk: Use qemu_hexdump_line() to avoid sprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 06/13] system/qtest: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread 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 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([i * 2], "%02x", data[i]);
^
  1 warning generated.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
---
 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([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

[PATCH v2 01/13] ui/console-vc: Replace sprintf() by snprintf()

2024-04-11 Thread Philippe Mathieu-Daudé 
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é 
---
 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

[PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method

2024-04-11 Thread Philippe Mathieu-Daudé 
Extract common code from reinitialize_rng_seed() and
load_kernel() to rng_seed_hex_new().

Signed-off-by: Philippe Mathieu-Daudé 
---
 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

Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Gerd Hoffmann 
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

Re: [PATCH 00/12] misc: Remove sprintf() due to macOS deprecation

2024-04-11 Thread Philippe Mathieu-Daudé 

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

Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Philippe Mathieu-Daudé 

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

Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Gerd Hoffmann 
  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

Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Marc-André Lureau 
On Wed, Apr 10, 2024 at 8:06 PM Philippe Mathieu-Daudé
 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é 

Reviewed-by: Marc-André Lureau 

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

Re: [PATCH 05/12] system/qtest: Replace sprintf() by g_string_append_printf()

2024-04-11 Thread Thomas Huth 

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([i * 2], "%02x", data[i]);
 ^
   1 warning generated.

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


Reviewed-by: Thomas Huth