* [PATCH v3 00/27] misc: Replace sprintf
@ 2024-04-12 7:33 Richard Henderson
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
` (26 more replies)
0 siblings, 27 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Hi Phil,
This incorporates my comments against
20240411104340.6617-1-philmd@linaro.org
and
20240411101550.99392-1-philmd@linaro.org
Especially cleanups to hexdup and disas.
r~
Philippe Mathieu-Daudé (13):
hw/mips/malta: Add re-usable rng_seed_hex_new() method
system/qtest: Replace sprintf by qemu_hexdump_line
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
disas/m68k: Replace sprintf() by snprintf()
disas/microblaze: Replace sprintf() by snprintf()
linux-user/flatload: Replace sprintf() by snprintf()
hw/misc/imx: Replace sprintf() by snprintf()
hw/net/rocker: Replace sprintf() by snprintf()
hw/riscv/virt: Replace sprintf by g_strdup_printf
target/arm: Replace sprintf() by snprintf()
Richard Henderson (14):
util/hexdump: Remove b parameter from qemu_hexdump_line
util/hexdump: Remove ascii parameter from qemu_hexdump_line
util/hexdump: Use a GString for qemu_hexdump_line
util/hexdump: Add unit_len and block_len to qemu_hexdump_line
util/hexdump: Inline g_string_append_printf "%02x"
disas/microblaze: Split out print_immval_addr
target/microblaze: Re-indent print_insn_microblaze
disas/microblaze: Merge op->name output into each fprintf
disas/microblaze: Print registers directly with PRIreg
disas/microblaze: Print immediates directly with PRIimm
disas/microblaze: Print registers directly with PRIrfsl
disas/microblaze: Split get_field_special
disas/riscv: Use GString in format_inst
target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
include/qemu/cutils.h | 20 +-
backends/tpm/tpm_util.c | 24 +-
disas/m68k.c | 2 +-
disas/microblaze.c | 544 +++++++++++++++++-----------------------
disas/riscv.c | 209 +++++++--------
hw/dma/pl330.c | 23 +-
hw/ide/atapi.c | 12 +-
hw/mips/malta.c | 25 +-
hw/misc/imx25_ccm.c | 2 +-
hw/misc/imx31_ccm.c | 2 +-
hw/misc/imx6_ccm.c | 4 +-
hw/misc/imx6_src.c | 2 +-
hw/misc/imx6ul_ccm.c | 4 +-
hw/misc/imx7_src.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/net/rocker/rocker.c | 24 +-
hw/riscv/virt.c | 17 +-
hw/scsi/scsi-disk.c | 13 +-
hw/ssi/imx_spi.c | 2 +-
hw/virtio/vhost-vdpa.c | 14 +-
linux-user/flatload.c | 2 +-
system/qtest.c | 12 +-
target/arm/cpu64.c | 4 +-
target/i386/kvm/kvm.c | 12 +-
util/hexdump.c | 98 +++++---
hw/virtio/trace-events | 2 +-
26 files changed, 494 insertions(+), 583 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 10:00 ` Philippe Mathieu-Daudé
2024-06-03 19:40 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 02/27] util/hexdump: Remove ascii " Richard Henderson
` (25 subsequent siblings)
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Require that the caller output the offset and increment bufptr.
Use QEMU_HEXDUMP_LINE_BYTES in vhost_vdpa_dump_config instead
of raw integer.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/cutils.h | 2 +-
hw/virtio/vhost-vdpa.c | 4 ++--
util/hexdump.c | 13 ++++++-------
hw/virtio/trace-events | 2 +-
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 92c927a6a3..3415f5d249 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -257,7 +257,7 @@ int parse_debug_env(const char *name, int max, int initial);
*/
#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 int len, bool ascii);
/*
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index e827b9175f..b4afa142f6 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -946,8 +946,8 @@ static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
for (b = 0; b < config_len; b += 16) {
len = config_len - b;
- qemu_hexdump_line(line, b, config, len, false);
- trace_vhost_vdpa_dump_config(dev, line);
+ qemu_hexdump_line(line, config + b, len, false);
+ trace_vhost_vdpa_dump_config(dev, b, line);
}
}
diff --git a/util/hexdump.c b/util/hexdump.c
index 9921114b3c..7324e7b126 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 int len, bool ascii)
{
const char *buf = bufptr;
@@ -26,13 +26,12 @@ void qemu_hexdump_line(char *line, unsigned int b, const void *bufptr,
len = QEMU_HEXDUMP_LINE_BYTES;
}
- line += snprintf(line, 6, "%04x:", b);
for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
- if ((i % 4) == 0) {
+ if (i != 0 && (i % 4) == 0) {
*line++ = ' ';
}
if (i < len) {
- line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
+ line += sprintf(line, " %02x", (unsigned char)buf[i]);
} else {
line += sprintf(line, " ");
}
@@ -40,7 +39,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[i];
if (c < ' ' || c > '~') {
c = '.';
}
@@ -58,8 +57,8 @@ void qemu_hexdump(FILE *fp, const char *prefix,
for (b = 0; b < size; b += QEMU_HEXDUMP_LINE_BYTES) {
len = size - b;
- qemu_hexdump_line(line, b, bufptr, len, true);
- fprintf(fp, "%s: %s\n", prefix, line);
+ qemu_hexdump_line(line, bufptr + b, len, true);
+ fprintf(fp, "%s: %04x: %s\n", prefix, b, line);
}
}
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 96632fd026..d01bc85037 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -50,7 +50,7 @@ vhost_vdpa_get_device_id(void *dev, uint32_t device_id) "dev: %p device_id %"PRI
vhost_vdpa_reset_device(void *dev) "dev: %p"
vhost_vdpa_get_vq_index(void *dev, int idx, int vq_idx) "dev: %p idx: %d vq idx: %d"
vhost_vdpa_set_vring_enable_one(void *dev, unsigned i, int enable, int r) "dev: %p, idx: %u, enable: %u, r: %d"
-vhost_vdpa_dump_config(void *dev, const char *line) "dev: %p %s"
+vhost_vdpa_dump_config(void *dev, unsigned ofs, const char *line) "dev: %p %04x: %s"
vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, uint32_t flags) "dev: %p offset: %"PRIu32" size: %"PRIu32" flags: 0x%"PRIx32
vhost_vdpa_get_config(void *dev, void *config, uint32_t config_len) "dev: %p config: %p config_len: %"PRIu32
vhost_vdpa_suspend(void *dev) "dev: %p"
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 02/27] util/hexdump: Remove ascii parameter from qemu_hexdump_line
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 10:03 ` Philippe Mathieu-Daudé
2024-06-03 17:42 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line Richard Henderson
` (24 subsequent siblings)
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Split out asciidump_line as a separate function, local to hexdump.c,
for use by qemu_hexdump. Use "%-*s" to generate the alignment
between the hex and the ascii, rather than explicit spaces.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/cutils.h | 3 +--
hw/virtio/vhost-vdpa.c | 2 +-
util/hexdump.c | 54 ++++++++++++++++++++++++------------------
3 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 3415f5d249..d0c5386e6c 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -257,8 +257,7 @@ int parse_debug_env(const char *name, int max, int initial);
*/
#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 int len, bool ascii);
+void qemu_hexdump_line(char *line, const void *bufptr, size_t len);
/*
* 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 b4afa142f6..32bad5ce68 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -946,7 +946,7 @@ static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
for (b = 0; b < config_len; b += 16) {
len = config_len - b;
- qemu_hexdump_line(line, config + b, len, false);
+ qemu_hexdump_line(line, config + b, len);
trace_vhost_vdpa_dump_config(dev, b, line);
}
}
diff --git a/util/hexdump.c b/util/hexdump.c
index 7324e7b126..dbc536fe84 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,8 +16,7 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
-void qemu_hexdump_line(char *line, const void *bufptr,
- unsigned int len, bool ascii)
+void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
{
const char *buf = bufptr;
int i, c;
@@ -26,39 +25,48 @@ void qemu_hexdump_line(char *line, const void *bufptr,
len = QEMU_HEXDUMP_LINE_BYTES;
}
- for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
+ for (i = 0; i < len; i++) {
if (i != 0 && (i % 4) == 0) {
*line++ = ' ';
}
- if (i < len) {
- line += sprintf(line, " %02x", (unsigned char)buf[i]);
- } else {
- line += sprintf(line, " ");
- }
- }
- if (ascii) {
- *line++ = ' ';
- for (i = 0; i < len; i++) {
- c = buf[i];
- if (c < ' ' || c > '~') {
- c = '.';
- }
- *line++ = c;
- }
+ line += sprintf(line, " %02x", (unsigned char)buf[i]);
}
*line = '\0';
}
+static void asciidump_line(char *line, const void *bufptr, size_t len)
+{
+ const char *buf = bufptr;
+
+ for (size_t i = 0; i < len; i++) {
+ char c = buf[i];
+
+ if (c < ' ' || c > '~') {
+ c = '.';
+ }
+ *line++ = c;
+ }
+ *line = '\0';
+}
+
+#define QEMU_HEXDUMP_LINE_WIDTH \
+ (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
+
void qemu_hexdump(FILE *fp, const char *prefix,
const void *bufptr, size_t size)
{
- unsigned int b, len;
char line[QEMU_HEXDUMP_LINE_LEN];
+ char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
+ size_t b, len;
- for (b = 0; b < size; b += QEMU_HEXDUMP_LINE_BYTES) {
- len = size - b;
- qemu_hexdump_line(line, bufptr + b, len, true);
- fprintf(fp, "%s: %04x: %s\n", prefix, b, line);
+ for (b = 0; b < size; b += len) {
+ len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
+
+ qemu_hexdump_line(line, bufptr + b, len);
+ asciidump_line(ascii, bufptr + b, len);
+
+ fprintf(fp, "%s: %04x: %-*s %s\n",
+ prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
2024-04-12 7:33 ` [PATCH v3 02/27] util/hexdump: Remove ascii " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:41 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line Richard Henderson
` (23 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Allocate a new, or append to an existing GString instead of
using a fixed sized buffer. Require the caller to determine
the length of the line -- do not bound len here.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/cutils.h | 15 ++++++++++-----
hw/virtio/vhost-vdpa.c | 14 ++++++++------
util/hexdump.c | 29 +++++++++++++++++------------
3 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index d0c5386e6c..7311fb36ca 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -252,12 +252,17 @@ static inline const char *yes_no(bool b)
*/
int parse_debug_env(const char *name, int max, int initial);
-/*
- * Hexdump a line of a byte buffer into a hexadecimal/ASCII buffer
+/**
+ * qemu_hexdump_line:
+ * @str: GString into which to append
+ * @buf: buffer to dump
+ * @len: number of bytes to dump
+ *
+ * Append @len bytes of @buf as hexadecimal into @str.
+ * If @str is NULL, allocate a new string and return it;
+ * otherwise return @str.
*/
-#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, size_t len);
+GString *qemu_hexdump_line(GString *str, const void *buf, size_t len);
/*
* 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 32bad5ce68..ab047d8ee0 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -941,13 +941,15 @@ 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;
- char line[QEMU_HEXDUMP_LINE_LEN];
+ g_autoptr(GString) str = g_string_sized_new(4 * 16);
+ size_t b, len;
- for (b = 0; b < config_len; b += 16) {
- len = config_len - b;
- qemu_hexdump_line(line, config + b, len);
- trace_vhost_vdpa_dump_config(dev, b, line);
+ for (b = 0; b < config_len; b += len) {
+ len = MIN(config_len - b, 16);
+
+ g_string_truncate(str, 0);
+ qemu_hexdump_line(str, config + b, len);
+ trace_vhost_vdpa_dump_config(dev, b, str->str);
}
}
diff --git a/util/hexdump.c b/util/hexdump.c
index dbc536fe84..521e346bc6 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,22 +16,25 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
-void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
+GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
{
- const char *buf = bufptr;
- int i, c;
+ const uint8_t *buf = vbuf;
+ size_t i;
- if (len > QEMU_HEXDUMP_LINE_BYTES) {
- len = QEMU_HEXDUMP_LINE_BYTES;
+ if (str == NULL) {
+ /* Estimate the length of the output to avoid reallocs. */
+ i = len * 3 + len / 4;
+ str = g_string_sized_new(i + 1);
}
for (i = 0; i < len; i++) {
if (i != 0 && (i % 4) == 0) {
- *line++ = ' ';
+ g_string_append_c(str, ' ');
}
- line += sprintf(line, " %02x", (unsigned char)buf[i]);
+ g_string_append_printf(str, " %02x", buf[i]);
}
- *line = '\0';
+
+ return str;
}
static void asciidump_line(char *line, const void *bufptr, size_t len)
@@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
*line = '\0';
}
+#define QEMU_HEXDUMP_LINE_BYTES 16
#define QEMU_HEXDUMP_LINE_WIDTH \
(QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
void qemu_hexdump(FILE *fp, const char *prefix,
const void *bufptr, size_t size)
{
- char line[QEMU_HEXDUMP_LINE_LEN];
+ g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
size_t b, len;
for (b = 0; b < size; b += len) {
len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
- qemu_hexdump_line(line, bufptr + b, len);
+ g_string_truncate(str, 0);
+ qemu_hexdump_line(str, bufptr + b, len);
asciidump_line(ascii, bufptr + b, len);
- fprintf(fp, "%s: %04x: %-*s %s\n",
- prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
+ fprintf(fp, "%s: %04zx: %-*s %s\n",
+ prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (2 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:42 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x" Richard Henderson
` (22 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Generalize the current 1 byte unit and 4 byte blocking
within the output.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/cutils.h | 6 +++++-
hw/virtio/vhost-vdpa.c | 2 +-
util/hexdump.c | 30 +++++++++++++++++++++---------
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 7311fb36ca..e28adedab9 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -257,12 +257,16 @@ int parse_debug_env(const char *name, int max, int initial);
* @str: GString into which to append
* @buf: buffer to dump
* @len: number of bytes to dump
+ * @unit_len: add a space between every @unit_len bytes
+ * @block_len: add an extra space between every @block_len bytes
*
* Append @len bytes of @buf as hexadecimal into @str.
+ * Add spaces between every @unit_len and @block_len bytes.
* If @str is NULL, allocate a new string and return it;
* otherwise return @str.
*/
-GString *qemu_hexdump_line(GString *str, const void *buf, size_t len);
+GString *qemu_hexdump_line(GString *str, const void *buf, size_t len,
+ size_t unit_len, size_t block_len);
/*
* 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 ab047d8ee0..911742e291 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -948,7 +948,7 @@ static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
len = MIN(config_len - b, 16);
g_string_truncate(str, 0);
- qemu_hexdump_line(str, config + b, len);
+ qemu_hexdump_line(str, config + b, len, 1, 4);
trace_vhost_vdpa_dump_config(dev, b, str->str);
}
}
diff --git a/util/hexdump.c b/util/hexdump.c
index 521e346bc6..b29326b7f2 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -1,5 +1,5 @@
/*
- * Helper to hexdump a buffer
+* Helper to hexdump a buffer
*
* Copyright (c) 2013 Red Hat, Inc.
* Copyright (c) 2013 Gerd Hoffmann <kraxel@redhat.com>
@@ -16,22 +16,34 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
-GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
+GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
+ size_t unit_len, size_t block_len)
{
const uint8_t *buf = vbuf;
- size_t i;
+ size_t u, b;
if (str == NULL) {
/* Estimate the length of the output to avoid reallocs. */
- i = len * 3 + len / 4;
- str = g_string_sized_new(i + 1);
+ size_t est = len * 2;
+ if (unit_len) {
+ est += len / unit_len;
+ }
+ if (block_len) {
+ est += len / block_len;
+ }
+ str = g_string_sized_new(est + 1);
}
- for (i = 0; i < len; i++) {
- if (i != 0 && (i % 4) == 0) {
+ for (u = 0, b = 0; len; u++, b++, len--, buf++) {
+ if (unit_len && u == unit_len) {
g_string_append_c(str, ' ');
+ u = 0;
}
- g_string_append_printf(str, " %02x", buf[i]);
+ if (block_len && b == block_len) {
+ g_string_append_c(str, ' ');
+ b = 0;
+ }
+ g_string_append_printf(str, "%02x", *buf);
}
return str;
@@ -67,7 +79,7 @@ void qemu_hexdump(FILE *fp, const char *prefix,
len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
g_string_truncate(str, 0);
- qemu_hexdump_line(str, bufptr + b, len);
+ qemu_hexdump_line(str, bufptr + b, len, 1, 4);
asciidump_line(ascii, bufptr + b, len);
fprintf(fp, "%s: %04zx: %-*s %s\n",
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x"
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (3 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:43 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method Richard Henderson
` (21 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Trivial arithmetic can be used for emitting the nibbles,
rather than full-blown printf formatting.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/hexdump.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/util/hexdump.c b/util/hexdump.c
index b29326b7f2..ae0d4992dc 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,6 +16,11 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
+static inline char hexdump_nibble(unsigned x)
+{
+ return (x < 10 ? '0' : 'a' - 10) + x;
+}
+
GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
size_t unit_len, size_t block_len)
{
@@ -35,6 +40,8 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
}
for (u = 0, b = 0; len; u++, b++, len--, buf++) {
+ uint8_t c;
+
if (unit_len && u == unit_len) {
g_string_append_c(str, ' ');
u = 0;
@@ -43,7 +50,10 @@ GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len,
g_string_append_c(str, ' ');
b = 0;
}
- g_string_append_printf(str, "%02x", *buf);
+
+ c = *buf;
+ g_string_append_c(str, hexdump_nibble(c / 16));
+ g_string_append_c(str, hexdump_nibble(c % 16));
}
return str;
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (4 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x" Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 18:56 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line Richard Henderson
` (20 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Extract common code from reinitialize_rng_seed and load_kernel
to rng_seed_hex_new. Using qemu_hexdump_line both fixes the
deprecation warning and simplifies the code base.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[rth: Use qemu_hexdump_line.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/mips/malta.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index af74008c82..664a2ae0a9 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -26,6 +26,7 @@
#include "qemu/units.h"
#include "qemu/bitops.h"
#include "qemu/datadir.h"
+#include "qemu/cutils.h"
#include "qemu/guest-random.h"
#include "hw/clock.h"
#include "hw/southbridge/piix.h"
@@ -850,15 +851,18 @@ 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 GString *rng_seed_hex_new(void)
{
- char *rng_seed_hex = opaque;
uint8_t rng_seed[32];
qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
- for (size_t i = 0; i < sizeof(rng_seed); ++i) {
- sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
- }
+ return qemu_hexdump_line(NULL, rng_seed, sizeof(rng_seed), 0, 0);
+}
+
+static void reinitialize_rng_seed(void *opaque)
+{
+ g_autoptr(GString) hex = rng_seed_hex_new();
+ memcpy(opaque, hex->str, hex->len);
}
/* Kernel */
@@ -870,8 +874,6 @@ 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];
size_t rng_seed_prom_offset;
kernel_size = load_elf(loaderparams.kernel_filename, NULL,
@@ -946,14 +948,13 @@ 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;
- prom_set(prom_buf, prom_index++, "%s", rng_seed_hex);
+ {
+ g_autoptr(GString) hex = rng_seed_hex_new();
+ prom_set(prom_buf, prom_index++, "%s", hex->str);
+ }
prom_set(prom_buf, prom_index++, NULL);
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (5 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:45 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf Richard Henderson
` (19 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>`
[rth: Use qemu_hexdump_line]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
system/qtest.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/system/qtest.c b/system/qtest.c
index 6da58b3874..507a358f3b 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) {
- uint64_t addr, len, i;
+ g_autoptr(GString) enc = NULL;
+ uint64_t addr, len;
uint8_t *data;
- char *enc;
int ret;
g_assert(words[1] && words[2]);
@@ -618,16 +618,12 @@ 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]);
- }
+ enc = qemu_hexdump_line(NULL, data, len, 0, 0);
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.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (6 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:45 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 09/27] hw/ide/atapi: " Richard Henderson
` (18 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.
Note that this drops the "0x" prefix to every byte, which should
be of no consequence to tracing.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/scsi/scsi-disk.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 4bd7af9d0c..f386a2f01c 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2648,19 +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;
+ g_autoptr(GString) str = NULL;
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]);
- }
- trace_scsi_disk_new_request(lun, tag, line_buffer);
-
- g_free(line_buffer);
+ str = qemu_hexdump_line(NULL, buf, len, 1, 0);
+ trace_scsi_disk_new_request(lun, tag, str->str);
}
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 09/27] hw/ide/atapi: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (7 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 18:57 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 10/27] hw/dma/pl330: " Richard Henderson
` (17 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/ide/atapi.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 73ec373184..fcb6cca157 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,14 +1310,9 @@ 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]);
- }
- trace_ide_atapi_cmd_packet(s, s->lcyl | (s->hcyl << 8), ppacket);
- g_free(ppacket);
+ g_autoptr(GString) str =
+ qemu_hexdump_line(NULL, buf, ATAPI_PACKET_SIZE, 1, 0);
+ trace_ide_atapi_cmd_packet(s, s->lcyl | (s->hcyl << 8), str->str);
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 10/27] hw/dma/pl330: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (8 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 09/27] hw/ide/atapi: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 18:59 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 11/27] backends/tpm: " Richard Henderson
` (16 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/dma/pl330.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index 70a502d245..5f89295af3 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,22 +318,14 @@ typedef struct PL330InsnDesc {
static void pl330_hexdump(uint8_t *buf, size_t size)
{
- unsigned int b, i, len;
- char tmpbuf[80];
+ g_autoptr(GString) str = g_string_sized_new(64);
+ size_t 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]);
- }
- trace_pl330_hexdump(b, tmpbuf);
+ for (b = 0; b < size; b += len) {
+ len = MIN(16, size - b);
+ g_string_truncate(str, 0);
+ qemu_hexdump_line(str, buf + b, len, 1, 4);
+ trace_pl330_hexdump(b, str->str);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 11/27] backends/tpm: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (9 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 10/27] hw/dma/pl330: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 17:47 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 12/27] disas/m68k: Replace sprintf() by snprintf() Richard Henderson
` (15 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Stefan Berger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[rth: Keep the linebreaks every 16 bytes]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
backends/tpm/tpm_util.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 1856589c3b..bf3378fbbe 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,22 @@ 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;
+ g_autoptr(GString) str = NULL;
+ size_t len, i, l;
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);
-
- for (i = 0, p = line_buffer; i < len; i++) {
- if (i && !(i % 16)) {
- p += sprintf(p, "\n");
+ for (i = 0; i < len; i += l) {
+ if (str) {
+ g_string_append_c(str, '\n');
}
- p += sprintf(p, "%.2X ", buffer[i]);
+ l = MIN(len, 16);
+ str = qemu_hexdump_line(str, buffer, l, 1, 0);
}
- trace_tpm_util_show_buffer(string, len, line_buffer);
- g_free(line_buffer);
+ g_string_ascii_up(str);
+ trace_tpm_util_show_buffer(string, len, str->str);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 12/27] disas/m68k: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (10 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 11/27] backends/tpm: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 7:33 ` [PATCH v3 13/27] disas/microblaze: " Richard Henderson
` (14 subsequent siblings)
26 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Peter Maydell
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20240411104340.6617-2-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/m68k.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disas/m68k.c b/disas/m68k.c
index 1f16e295ab..800b4145ac 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -1000,7 +1000,7 @@ print_indexed (int basereg,
/* Generate the text for the index register.
Where this will be output is not yet determined. */
- sprintf (buf, "%s:%c%s",
+ snprintf(buf, sizeof(buf), "%s:%c%s",
reg_names[(word >> 12) & 0xf],
(word & 0x800) ? 'l' : 'w',
scales[(word >> 9) & 3]);
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 13/27] disas/microblaze: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (11 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 12/27] disas/m68k: Replace sprintf() by snprintf() Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 7:33 ` [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr Richard Henderson
` (13 subsequent siblings)
26 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Edgar E . Iglesias
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Message-Id: <20240411104340.6617-3-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index 0b89b9c4fa..49a4c0fd40 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -600,7 +600,8 @@ static char *
get_field (long instr, long mask, unsigned short low)
{
char tmpstr[25];
- sprintf(tmpstr, "%s%d", register_prefix, (int)((instr & mask) >> low));
+ snprintf(tmpstr, sizeof(tmpstr), "%s%d", register_prefix,
+ (int)((instr & mask) >> low));
return(strdup(tmpstr));
}
@@ -608,7 +609,8 @@ static char *
get_field_imm (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%d", (short)((instr & IMM_MASK) >> IMM_LOW));
+ snprintf(tmpstr, sizeof(tmpstr), "%d",
+ (short)((instr & IMM_MASK) >> IMM_LOW));
return(strdup(tmpstr));
}
@@ -616,7 +618,8 @@ static char *
get_field_imm5 (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%d", (short)((instr & IMM5_MASK) >> IMM_LOW));
+ snprintf(tmpstr, sizeof(tmpstr), "%d",
+ (short)((instr & IMM5_MASK) >> IMM_LOW));
return(strdup(tmpstr));
}
@@ -624,7 +627,8 @@ static char *
get_field_rfsl (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%s%d", fsl_register_prefix, (short)((instr & RFSL_MASK) >> IMM_LOW));
+ snprintf(tmpstr, sizeof(tmpstr), "%s%d", fsl_register_prefix,
+ (short)((instr & RFSL_MASK) >> IMM_LOW));
return(strdup(tmpstr));
}
@@ -632,7 +636,8 @@ static char *
get_field_imm15 (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%d", (short)((instr & IMM15_MASK) >> IMM_LOW));
+ snprintf(tmpstr, sizeof(tmpstr), "%d",
+ (short)((instr & IMM15_MASK) >> IMM_LOW));
return(strdup(tmpstr));
}
@@ -641,7 +646,8 @@ static char *
get_field_unsigned_imm (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%d", (int)((instr & IMM_MASK) >> IMM_LOW));
+ snprintf(tmpstr, sizeof(tmpstr), "%d",
+ (int)((instr & IMM_MASK) >> IMM_LOW));
return(strdup(tmpstr));
}
#endif
@@ -653,7 +659,8 @@ get_field_unsigned_imm (long instr)
{
char tmpstr[25];
- sprintf(tmpstr, "%s%s", register_prefix, (((instr & IMM_MASK) >> IMM_LOW) & REG_MSR_MASK) == 0 ? "pc" : "msr");
+ snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix,
+ (((instr & IMM_MASK) >> IMM_LOW) & REG_MSR_MASK) == 0 ? "pc" : "msr");
return(strdup(tmpstr));
}
@@ -709,7 +716,7 @@ get_field_special(long instr, const struct op_code_struct *op)
default :
{
if ( ((((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask) & 0xE000) == REG_PVR_MASK) {
- sprintf(tmpstr, "%s%u", pvr_register_prefix,
+ snprintf(tmpstr, sizeof(tmpstr), "%s%u", pvr_register_prefix,
(unsigned short)(((instr & IMM_MASK) >> IMM_LOW) ^
op->immval_mask) ^ REG_PVR_MASK);
return(strdup(tmpstr));
@@ -720,7 +727,7 @@ get_field_special(long instr, const struct op_code_struct *op)
break;
}
- sprintf(tmpstr, "%s%s", register_prefix, spr);
+ snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix, spr);
return(strdup(tmpstr));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (12 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 13/27] disas/microblaze: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:14 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze Richard Henderson
` (12 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Unify the code blocks that try to print a symbolic address.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 89 +++++++++++++++-------------------------------
1 file changed, 29 insertions(+), 60 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index 49a4c0fd40..3473c94164 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -767,6 +767,24 @@ read_insn_microblaze (bfd_vma memaddr,
return inst;
}
+static void print_immval_addr(struct disassemble_info *info, bool immfound,
+ int immval, unsigned inst, int addend)
+{
+ if (info->print_address_func && info->symbol_at_address_func) {
+ if (immfound) {
+ immval |= get_int_field_imm(inst) & 0x0000ffff;
+ } else {
+ immval = (int16_t)get_int_field_imm(inst);
+ }
+ immval += addend;
+ if (immval != 0 && info->symbol_at_address_func(immval, info)) {
+ info->fprintf_func(info->stream, "\t// ");
+ info->print_address_func (immval, info);
+ } else if (addend) {
+ info->fprintf_func(info->stream, "\t// %x", immval);
+ }
+ }
+}
int
print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
@@ -821,18 +839,8 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
break;
case INST_TYPE_RD_R1_IMM:
fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm(inst));
- if (info->print_address_func && get_int_field_r1(inst) == 0 && info->symbol_at_address_func) {
- if (immfound)
- immval |= (get_int_field_imm(inst) & 0x0000ffff);
- else {
- immval = get_int_field_imm(inst);
- if (immval & 0x8000)
- immval |= 0xFFFF0000;
- }
- if (immval > 0 && info->symbol_at_address_func(immval, info)) {
- fprintf_func (stream, "\t// ");
- info->print_address_func (immval, info);
- }
+ if (get_int_field_r1(inst) == 0) {
+ print_immval_addr(info, immfound, immval, inst, 0);
}
break;
case INST_TYPE_RD_R1_IMM5:
@@ -860,61 +868,22 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_imm(inst));
/* The non-pc relative instructions are returns, which shouldn't
have a label printed */
- if (info->print_address_func && op->inst_offset_type == INST_PC_OFFSET && info->symbol_at_address_func) {
- if (immfound)
- immval |= (get_int_field_imm(inst) & 0x0000ffff);
- else {
- immval = get_int_field_imm(inst);
- if (immval & 0x8000)
- immval |= 0xFFFF0000;
- }
- immval += memaddr;
- if (immval > 0 && info->symbol_at_address_func(immval, info)) {
- fprintf_func (stream, "\t// ");
- info->print_address_func (immval, info);
- } else {
- fprintf_func (stream, "\t\t// ");
- fprintf_func (stream, "%x", immval);
- }
+ if (op->inst_offset_type == INST_PC_OFFSET) {
+ print_immval_addr(info, immfound, immval, inst, memaddr);
}
break;
case INST_TYPE_RD_IMM:
fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm(inst));
- if (info->print_address_func && info->symbol_at_address_func) {
- if (immfound)
- immval |= (get_int_field_imm(inst) & 0x0000ffff);
- else {
- immval = get_int_field_imm(inst);
- if (immval & 0x8000)
- immval |= 0xFFFF0000;
- }
- if (op->inst_offset_type == INST_PC_OFFSET)
- immval += (int) memaddr;
- if (info->symbol_at_address_func(immval, info)) {
- fprintf_func (stream, "\t// ");
- info->print_address_func (immval, info);
- }
- }
+ print_immval_addr(info, immfound, immval, inst,
+ op->inst_offset_type == INST_PC_OFFSET
+ ? memaddr : 0);
break;
case INST_TYPE_IMM:
fprintf_func(stream, "\t%s", get_field_imm(inst));
- if (info->print_address_func && info->symbol_at_address_func && op->instr != imm) {
- if (immfound)
- immval |= (get_int_field_imm(inst) & 0x0000ffff);
- else {
- immval = get_int_field_imm(inst);
- if (immval & 0x8000)
- immval |= 0xFFFF0000;
- }
- if (op->inst_offset_type == INST_PC_OFFSET)
- immval += (int) memaddr;
- if (immval > 0 && info->symbol_at_address_func(immval, info)) {
- fprintf_func (stream, "\t// ");
- info->print_address_func (immval, info);
- } else if (op->inst_offset_type == INST_PC_OFFSET) {
- fprintf_func (stream, "\t\t// ");
- fprintf_func (stream, "%x", immval);
- }
+ if (op->instr != imm) {
+ print_immval_addr(info, immfound, immval, inst,
+ op->inst_offset_type == INST_PC_OFFSET
+ ? memaddr : 0);
}
break;
case INST_TYPE_RD_R2:
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (13 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:14 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf Richard Henderson
` (11 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 263 ++++++++++++++++++++++++---------------------
1 file changed, 141 insertions(+), 122 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index 3473c94164..c729c76585 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -787,134 +787,153 @@ static void print_immval_addr(struct disassemble_info *info, bool immfound,
}
int
-print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
+print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
{
- fprintf_function fprintf_func = info->fprintf_func;
- void * stream = info->stream;
- unsigned long inst, prev_inst;
- const struct op_code_struct *op, *pop;
- int immval = 0;
- bfd_boolean immfound = FALSE;
- static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
- static int prev_insn_vma = -1; /*init the prev insn vma */
- int curr_insn_vma = info->buffer_vma;
+ fprintf_function fprintf_func = info->fprintf_func;
+ void *stream = info->stream;
+ unsigned long inst, prev_inst;
+ const struct op_code_struct *op, *pop;
+ int immval = 0;
+ bool immfound = false;
+ static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
+ static int prev_insn_vma = -1; /*init the prev insn vma */
+ int curr_insn_vma = info->buffer_vma;
- info->bytes_per_chunk = 4;
+ info->bytes_per_chunk = 4;
- inst = read_insn_microblaze (memaddr, info, &op);
- if (inst == 0) {
- return -1;
- }
+ inst = read_insn_microblaze (memaddr, info, &op);
+ if (inst == 0) {
+ return -1;
+ }
- if (prev_insn_vma == curr_insn_vma) {
- if (memaddr-(info->bytes_per_chunk) == prev_insn_addr) {
- prev_inst = read_insn_microblaze (prev_insn_addr, info, &pop);
- if (prev_inst == 0)
- return -1;
- if (pop->instr == imm) {
- immval = (get_int_field_imm(prev_inst) << 16) & 0xffff0000;
- immfound = TRUE;
+ if (prev_insn_vma == curr_insn_vma) {
+ if (memaddr - info->bytes_per_chunk == prev_insn_addr) {
+ prev_inst = read_insn_microblaze (prev_insn_addr, info, &pop);
+ if (prev_inst == 0)
+ return -1;
+ if (pop->instr == imm) {
+ immval = (get_int_field_imm(prev_inst) << 16) & 0xffff0000;
+ immfound = TRUE;
+ }
+ else {
+ immval = 0;
+ immfound = FALSE;
+ }
+ }
}
- else {
- immval = 0;
- immfound = FALSE;
- }
- }
- }
- /* make curr insn as prev insn */
- prev_insn_addr = memaddr;
- prev_insn_vma = curr_insn_vma;
+ /* make curr insn as prev insn */
+ prev_insn_addr = memaddr;
+ prev_insn_vma = curr_insn_vma;
- if (op->name == 0) {
- fprintf_func (stream, ".short 0x%04lx", inst);
- }
- else
- {
- fprintf_func (stream, "%s", op->name);
+ if (op->name == 0) {
+ fprintf_func (stream, ".short 0x%04lx", inst);
+ return 4;
+ }
+
+ fprintf_func (stream, "%s", op->name);
- switch (op->inst_type)
- {
- case INST_TYPE_RD_R1_R2:
- fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_r2(inst));
- break;
- case INST_TYPE_RD_R1_IMM:
- fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm(inst));
- if (get_int_field_r1(inst) == 0) {
- print_immval_addr(info, immfound, immval, inst, 0);
- }
- break;
- case INST_TYPE_RD_R1_IMM5:
- fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm5(inst));
- break;
- case INST_TYPE_RD_RFSL:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_rfsl(inst));
- break;
- case INST_TYPE_R1_RFSL:
- fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_rfsl(inst));
- break;
- case INST_TYPE_RD_SPECIAL:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_special(inst, op));
- break;
- case INST_TYPE_SPECIAL_R1:
- fprintf_func(stream, "\t%s, %s", get_field_special(inst, op), get_field_r1(inst));
- break;
- case INST_TYPE_RD_R1:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r1(inst));
- break;
- case INST_TYPE_R1_R2:
- fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_r2(inst));
- break;
- case INST_TYPE_R1_IMM:
- fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_imm(inst));
- /* The non-pc relative instructions are returns, which shouldn't
- have a label printed */
- if (op->inst_offset_type == INST_PC_OFFSET) {
- print_immval_addr(info, immfound, immval, inst, memaddr);
- }
- break;
- case INST_TYPE_RD_IMM:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm(inst));
- print_immval_addr(info, immfound, immval, inst,
- op->inst_offset_type == INST_PC_OFFSET
- ? memaddr : 0);
- break;
- case INST_TYPE_IMM:
- fprintf_func(stream, "\t%s", get_field_imm(inst));
- if (op->instr != imm) {
- print_immval_addr(info, immfound, immval, inst,
- op->inst_offset_type == INST_PC_OFFSET
- ? memaddr : 0);
- }
- break;
- case INST_TYPE_RD_R2:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r2(inst));
- break;
- case INST_TYPE_R2:
- fprintf_func(stream, "\t%s", get_field_r2(inst));
- break;
- case INST_TYPE_R1:
- fprintf_func(stream, "\t%s", get_field_r1(inst));
- break;
- case INST_TYPE_RD_R1_SPECIAL:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r2(inst));
- break;
- case INST_TYPE_RD_IMM15:
- fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm15(inst));
- break;
- /* For tuqula instruction */
- case INST_TYPE_RD:
- fprintf_func(stream, "\t%s", get_field_rd(inst));
- break;
- case INST_TYPE_RFSL:
- fprintf_func(stream, "\t%s", get_field_rfsl(inst));
- break;
- default:
- /* if the disassembler lags the instruction set */
- fprintf_func (stream, "\tundecoded operands, inst is 0x%04lx", inst);
- break;
- }
+ switch (op->inst_type) {
+ case INST_TYPE_RD_R1_R2:
+ fprintf_func(stream, "\t%s, %s, %s",
+ get_field_rd(inst), get_field_r1(inst),
+ get_field_r2(inst));
+ break;
+ case INST_TYPE_RD_R1_IMM:
+ fprintf_func(stream, "\t%s, %s, %s",
+ get_field_rd(inst), get_field_r1(inst),
+ get_field_imm(inst));
+ if (get_int_field_r1(inst) == 0) {
+ print_immval_addr(info, immfound, immval, inst, 0);
+ }
+ break;
+ case INST_TYPE_RD_R1_IMM5:
+ fprintf_func(stream, "\t%s, %s, %s",
+ get_field_rd(inst), get_field_r1(inst),
+ get_field_imm5(inst));
+ break;
+ case INST_TYPE_RD_RFSL:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_rfsl(inst));
+ break;
+ case INST_TYPE_R1_RFSL:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_r1(inst), get_field_rfsl(inst));
+ break;
+ case INST_TYPE_RD_SPECIAL:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_special(inst, op));
+ break;
+ case INST_TYPE_SPECIAL_R1:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_special(inst, op), get_field_r1(inst));
+ break;
+ case INST_TYPE_RD_R1:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_r1(inst));
+ break;
+ case INST_TYPE_R1_R2:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_r1(inst), get_field_r2(inst));
+ break;
+ case INST_TYPE_R1_IMM:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_r1(inst), get_field_imm(inst));
+ /*
+ * The non-pc relative instructions are returns,
+ * which shouldn't have a label printed.
+ */
+ if (op->inst_offset_type == INST_PC_OFFSET) {
+ print_immval_addr(info, immfound, immval, inst, memaddr);
+ }
+ break;
+ case INST_TYPE_RD_IMM:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_imm(inst));
+ print_immval_addr(info, immfound, immval, inst,
+ op->inst_offset_type == INST_PC_OFFSET
+ ? memaddr : 0);
+ break;
+ case INST_TYPE_IMM:
+ fprintf_func(stream, "\t%s", get_field_imm(inst));
+ if (op->instr != imm) {
+ print_immval_addr(info, immfound, immval, inst,
+ op->inst_offset_type == INST_PC_OFFSET
+ ? memaddr : 0);
+ }
+ break;
+ case INST_TYPE_RD_R2:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_r2(inst));
+ break;
+ case INST_TYPE_R2:
+ fprintf_func(stream, "\t%s",
+ get_field_r2(inst));
+ break;
+ case INST_TYPE_R1:
+ fprintf_func(stream, "\t%s",
+ get_field_r1(inst));
+ break;
+ case INST_TYPE_RD_R1_SPECIAL:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_r2(inst));
+ break;
+ case INST_TYPE_RD_IMM15:
+ fprintf_func(stream, "\t%s, %s",
+ get_field_rd(inst), get_field_imm15(inst));
+ break;
+ /* For tuqula instruction */
+ case INST_TYPE_RD:
+ fprintf_func(stream, "\t%s",
+ get_field_rd(inst));
+ break;
+ case INST_TYPE_RFSL:
+ fprintf_func(stream, "\t%s",
+ get_field_rfsl(inst));
+ break;
+ default:
+ /* if the disassembler lags the instruction set */
+ fprintf_func(stream, "\tundecoded operands, inst is 0x%04lx", inst);
+ break;
}
-
- /* Say how many bytes we consumed? */
- return 4;
+ return 4;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (14 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:15 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg Richard Henderson
` (10 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
In the common case, issue one single fprintf.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 80 +++++++++++++++++++++++-----------------------
1 file changed, 40 insertions(+), 40 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index c729c76585..a537ac65dd 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -830,54 +830,52 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
return 4;
}
- fprintf_func (stream, "%s", op->name);
-
switch (op->inst_type) {
case INST_TYPE_RD_R1_R2:
- fprintf_func(stream, "\t%s, %s, %s",
- get_field_rd(inst), get_field_r1(inst),
+ fprintf_func(stream, "%s\t%s, %s, %s",
+ op->name, get_field_rd(inst), get_field_r1(inst),
get_field_r2(inst));
break;
case INST_TYPE_RD_R1_IMM:
- fprintf_func(stream, "\t%s, %s, %s",
- get_field_rd(inst), get_field_r1(inst),
+ fprintf_func(stream, "%s\t%s, %s, %s",
+ op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm(inst));
if (get_int_field_r1(inst) == 0) {
print_immval_addr(info, immfound, immval, inst, 0);
}
break;
case INST_TYPE_RD_R1_IMM5:
- fprintf_func(stream, "\t%s, %s, %s",
- get_field_rd(inst), get_field_r1(inst),
+ fprintf_func(stream, "%s\t%s, %s, %s",
+ op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm5(inst));
break;
case INST_TYPE_RD_RFSL:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_rfsl(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_rfsl(inst));
break;
case INST_TYPE_R1_RFSL:
- fprintf_func(stream, "\t%s, %s",
- get_field_r1(inst), get_field_rfsl(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_r1(inst), get_field_rfsl(inst));
break;
case INST_TYPE_RD_SPECIAL:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_special(inst, op));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_special(inst, op));
break;
case INST_TYPE_SPECIAL_R1:
- fprintf_func(stream, "\t%s, %s",
- get_field_special(inst, op), get_field_r1(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_special(inst, op), get_field_r1(inst));
break;
case INST_TYPE_RD_R1:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_r1(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_r1(inst));
break;
case INST_TYPE_R1_R2:
- fprintf_func(stream, "\t%s, %s",
- get_field_r1(inst), get_field_r2(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_r1(inst), get_field_r2(inst));
break;
case INST_TYPE_R1_IMM:
- fprintf_func(stream, "\t%s, %s",
- get_field_r1(inst), get_field_imm(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_r1(inst), get_field_imm(inst));
/*
* The non-pc relative instructions are returns,
* which shouldn't have a label printed.
@@ -887,14 +885,15 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_IMM:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_imm(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_imm(inst));
print_immval_addr(info, immfound, immval, inst,
op->inst_offset_type == INST_PC_OFFSET
? memaddr : 0);
break;
case INST_TYPE_IMM:
- fprintf_func(stream, "\t%s", get_field_imm(inst));
+ fprintf_func(stream, "%s\t%s",
+ op->name, get_field_imm(inst));
if (op->instr != imm) {
print_immval_addr(info, immfound, immval, inst,
op->inst_offset_type == INST_PC_OFFSET
@@ -902,37 +901,38 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_R2:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_r2(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_r2(inst));
break;
case INST_TYPE_R2:
- fprintf_func(stream, "\t%s",
- get_field_r2(inst));
+ fprintf_func(stream, "%s\t%s",
+ op->name, get_field_r2(inst));
break;
case INST_TYPE_R1:
- fprintf_func(stream, "\t%s",
- get_field_r1(inst));
+ fprintf_func(stream, "%s\t%s",
+ op->name, get_field_r1(inst));
break;
case INST_TYPE_RD_R1_SPECIAL:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_r2(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_r2(inst));
break;
case INST_TYPE_RD_IMM15:
- fprintf_func(stream, "\t%s, %s",
- get_field_rd(inst), get_field_imm15(inst));
+ fprintf_func(stream, "%s\t%s, %s",
+ op->name, get_field_rd(inst), get_field_imm15(inst));
break;
/* For tuqula instruction */
case INST_TYPE_RD:
- fprintf_func(stream, "\t%s",
- get_field_rd(inst));
+ fprintf_func(stream, "%s\t%s",
+ op->name, get_field_rd(inst));
break;
case INST_TYPE_RFSL:
- fprintf_func(stream, "\t%s",
- get_field_rfsl(inst));
+ fprintf_func(stream, "%s\t%s",
+ op->name, get_field_rfsl(inst));
break;
default:
/* if the disassembler lags the instruction set */
- fprintf_func(stream, "\tundecoded operands, inst is 0x%04lx", inst);
+ fprintf_func(stream, "%s\tundecoded operands, inst is 0x%04lx",
+ op->name, inst);
break;
}
return 4;
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (15 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm Richard Henderson
` (9 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Use a printf format instead of sprintf into a buffer.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 54 ++++++++++++++++++++--------------------------
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index a537ac65dd..c12968f3b9 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -563,7 +563,7 @@ static const struct op_code_struct {
};
/* prefix for register names */
-static const char register_prefix[] = "r";
+#define register_prefix "r"
static const char fsl_register_prefix[] = "rfsl";
static const char pvr_register_prefix[] = "rpvr";
@@ -579,15 +579,16 @@ static const char pvr_register_prefix[] = "rpvr";
#include "disas/dis-asm.h"
-#define get_field_rd(instr) get_field(instr, RD_MASK, RD_LOW)
-#define get_field_r1(instr) get_field(instr, RA_MASK, RA_LOW)
-#define get_field_r2(instr) get_field(instr, RB_MASK, RB_LOW)
+#define PRIreg register_prefix "%ld"
+
+#define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
+#define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
+#define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
#define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
#define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
/* Local function prototypes. */
-static char * get_field (long instr, long mask, unsigned short low);
static char * get_field_imm (long instr);
static char * get_field_imm5 (long instr);
static char * get_field_rfsl (long instr);
@@ -596,15 +597,6 @@ static char * get_field_imm15 (long instr);
static char * get_field_unsigned_imm (long instr);
#endif
-static char *
-get_field (long instr, long mask, unsigned short low)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%s%d", register_prefix,
- (int)((instr & mask) >> low));
- return(strdup(tmpstr));
-}
-
static char *
get_field_imm (long instr)
{
@@ -832,12 +824,12 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
switch (op->inst_type) {
case INST_TYPE_RD_R1_R2:
- fprintf_func(stream, "%s\t%s, %s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIreg,
op->name, get_field_rd(inst), get_field_r1(inst),
get_field_r2(inst));
break;
case INST_TYPE_RD_R1_IMM:
- fprintf_func(stream, "%s\t%s, %s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm(inst));
if (get_int_field_r1(inst) == 0) {
@@ -845,36 +837,36 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_R1_IMM5:
- fprintf_func(stream, "%s\t%s, %s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm5(inst));
break;
case INST_TYPE_RD_RFSL:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_rd(inst), get_field_rfsl(inst));
break;
case INST_TYPE_R1_RFSL:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_r1(inst), get_field_rfsl(inst));
break;
case INST_TYPE_RD_SPECIAL:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_rd(inst), get_field_special(inst, op));
break;
case INST_TYPE_SPECIAL_R1:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t%s, " PRIreg,
op->name, get_field_special(inst, op), get_field_r1(inst));
break;
case INST_TYPE_RD_R1:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
op->name, get_field_rd(inst), get_field_r1(inst));
break;
case INST_TYPE_R1_R2:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
op->name, get_field_r1(inst), get_field_r2(inst));
break;
case INST_TYPE_R1_IMM:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_r1(inst), get_field_imm(inst));
/*
* The non-pc relative instructions are returns,
@@ -885,7 +877,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_IMM:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_rd(inst), get_field_imm(inst));
print_immval_addr(info, immfound, immval, inst,
op->inst_offset_type == INST_PC_OFFSET
@@ -901,28 +893,28 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_R2:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
op->name, get_field_rd(inst), get_field_r2(inst));
break;
case INST_TYPE_R2:
- fprintf_func(stream, "%s\t%s",
+ fprintf_func(stream, "%s\t" PRIreg,
op->name, get_field_r2(inst));
break;
case INST_TYPE_R1:
- fprintf_func(stream, "%s\t%s",
+ fprintf_func(stream, "%s\t" PRIreg,
op->name, get_field_r1(inst));
break;
case INST_TYPE_RD_R1_SPECIAL:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
op->name, get_field_rd(inst), get_field_r2(inst));
break;
case INST_TYPE_RD_IMM15:
- fprintf_func(stream, "%s\t%s, %s",
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
op->name, get_field_rd(inst), get_field_imm15(inst));
break;
/* For tuqula instruction */
case INST_TYPE_RD:
- fprintf_func(stream, "%s\t%s",
+ fprintf_func(stream, "%s\t" PRIreg,
op->name, get_field_rd(inst));
break;
case INST_TYPE_RFSL:
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (16 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl Richard Henderson
` (8 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Use a printf format instead of sprintf into a buffer.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 61 +++++++++-------------------------------------
1 file changed, 11 insertions(+), 50 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index c12968f3b9..390f98c0a3 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -580,40 +580,21 @@ static const char pvr_register_prefix[] = "rpvr";
#include "disas/dis-asm.h"
#define PRIreg register_prefix "%ld"
+#define PRIimm "%d"
#define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
#define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
#define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
+#define get_field_imm(instr) ((int16_t)instr)
+#define get_field_imm5(instr) ((int)instr & IMM5_MASK)
+#define get_field_imm15(instr) ((int)instr & IMM15_MASK)
+
#define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
#define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
/* Local function prototypes. */
-static char * get_field_imm (long instr);
-static char * get_field_imm5 (long instr);
static char * get_field_rfsl (long instr);
-static char * get_field_imm15 (long instr);
-#if 0
-static char * get_field_unsigned_imm (long instr);
-#endif
-
-static char *
-get_field_imm (long instr)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%d",
- (short)((instr & IMM_MASK) >> IMM_LOW));
- return(strdup(tmpstr));
-}
-
-static char *
-get_field_imm5 (long instr)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%d",
- (short)((instr & IMM5_MASK) >> IMM_LOW));
- return(strdup(tmpstr));
-}
static char *
get_field_rfsl (long instr)
@@ -624,26 +605,6 @@ get_field_rfsl (long instr)
return(strdup(tmpstr));
}
-static char *
-get_field_imm15 (long instr)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%d",
- (short)((instr & IMM15_MASK) >> IMM_LOW));
- return(strdup(tmpstr));
-}
-
-#if 0
-static char *
-get_field_unsigned_imm (long instr)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%d",
- (int)((instr & IMM_MASK) >> IMM_LOW));
- return(strdup(tmpstr));
-}
-#endif
-
/*
char *
get_field_special (instr)
@@ -829,7 +790,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
get_field_r2(inst));
break;
case INST_TYPE_RD_R1_IMM:
- fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIimm,
op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm(inst));
if (get_int_field_r1(inst) == 0) {
@@ -837,7 +798,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_R1_IMM5:
- fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIimm,
op->name, get_field_rd(inst), get_field_r1(inst),
get_field_imm5(inst));
break;
@@ -866,7 +827,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
op->name, get_field_r1(inst), get_field_r2(inst));
break;
case INST_TYPE_R1_IMM:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
op->name, get_field_r1(inst), get_field_imm(inst));
/*
* The non-pc relative instructions are returns,
@@ -877,14 +838,14 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
}
break;
case INST_TYPE_RD_IMM:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
op->name, get_field_rd(inst), get_field_imm(inst));
print_immval_addr(info, immfound, immval, inst,
op->inst_offset_type == INST_PC_OFFSET
? memaddr : 0);
break;
case INST_TYPE_IMM:
- fprintf_func(stream, "%s\t%s",
+ fprintf_func(stream, "%s\t" PRIimm,
op->name, get_field_imm(inst));
if (op->instr != imm) {
print_immval_addr(info, immfound, immval, inst,
@@ -909,7 +870,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
op->name, get_field_rd(inst), get_field_r2(inst));
break;
case INST_TYPE_RD_IMM15:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
op->name, get_field_rd(inst), get_field_imm15(inst));
break;
/* For tuqula instruction */
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (17 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-06-03 19:18 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 20/27] disas/microblaze: Split get_field_special Richard Henderson
` (7 subsequent siblings)
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Use a printf format instead of sprintf into a buffer.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index 390f98c0a3..24febfdea9 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -564,7 +564,6 @@ static const struct op_code_struct {
/* prefix for register names */
#define register_prefix "r"
-static const char fsl_register_prefix[] = "rfsl";
static const char pvr_register_prefix[] = "rpvr";
@@ -580,11 +579,13 @@ static const char pvr_register_prefix[] = "rpvr";
#include "disas/dis-asm.h"
#define PRIreg register_prefix "%ld"
+#define PRIrfsl register_prefix "fsl%ld"
#define PRIimm "%d"
#define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
#define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
#define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
+#define get_field_rfsl(instr) (instr & RFSL_MASK)
#define get_field_imm(instr) ((int16_t)instr)
#define get_field_imm5(instr) ((int)instr & IMM5_MASK)
#define get_field_imm15(instr) ((int)instr & IMM15_MASK)
@@ -592,19 +593,6 @@ static const char pvr_register_prefix[] = "rpvr";
#define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
#define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
-/* Local function prototypes. */
-
-static char * get_field_rfsl (long instr);
-
-static char *
-get_field_rfsl (long instr)
-{
- char tmpstr[25];
- snprintf(tmpstr, sizeof(tmpstr), "%s%d", fsl_register_prefix,
- (short)((instr & RFSL_MASK) >> IMM_LOW));
- return(strdup(tmpstr));
-}
-
/*
char *
get_field_special (instr)
@@ -803,11 +791,11 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
get_field_imm5(inst));
break;
case INST_TYPE_RD_RFSL:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
op->name, get_field_rd(inst), get_field_rfsl(inst));
break;
case INST_TYPE_R1_RFSL:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
op->name, get_field_r1(inst), get_field_rfsl(inst));
break;
case INST_TYPE_RD_SPECIAL:
@@ -879,7 +867,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
op->name, get_field_rd(inst));
break;
case INST_TYPE_RFSL:
- fprintf_func(stream, "%s\t%s",
+ fprintf_func(stream, "%s\t" PRIrfsl,
op->name, get_field_rfsl(inst));
break;
default:
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 20/27] disas/microblaze: Split get_field_special
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (18 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:20 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 21/27] disas/riscv: Use GString in format_inst Richard Henderson
` (6 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Extract the raw special index and a function to lookup a name.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/microblaze.c | 142 +++++++++++++++++++--------------------------
1 file changed, 61 insertions(+), 81 deletions(-)
diff --git a/disas/microblaze.c b/disas/microblaze.c
index 24febfdea9..197327fae4 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -564,8 +564,6 @@ static const struct op_code_struct {
/* prefix for register names */
#define register_prefix "r"
-static const char pvr_register_prefix[] = "rpvr";
-
/* #defines for valid immediate range */
#define MIN_IMM ((int) 0x80000000)
@@ -580,6 +578,7 @@ static const char pvr_register_prefix[] = "rpvr";
#define PRIreg register_prefix "%ld"
#define PRIrfsl register_prefix "fsl%ld"
+#define PRIpvr register_prefix "pvr%d"
#define PRIimm "%d"
#define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
@@ -593,83 +592,48 @@ static const char pvr_register_prefix[] = "rpvr";
#define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
#define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
-/*
- char *
- get_field_special (instr)
- long instr;
- {
- char tmpstr[25];
-
- snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix,
- (((instr & IMM_MASK) >> IMM_LOW) & REG_MSR_MASK) == 0 ? "pc" : "msr");
-
- return(strdup(tmpstr));
- }
-*/
-
-static char *
-get_field_special(long instr, const struct op_code_struct *op)
+static int get_field_special(long instr, const struct op_code_struct *op)
{
- char tmpstr[25];
- char spr[6];
+ return ((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask;
+}
- switch ( (((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask) ) {
-
- case REG_MSR_MASK :
- strcpy(spr, "msr");
- break;
- case REG_PC_MASK :
- strcpy(spr, "pc");
- break;
- case REG_EAR_MASK :
- strcpy(spr, "ear");
- break;
- case REG_ESR_MASK :
- strcpy(spr, "esr");
- break;
- case REG_FSR_MASK :
- strcpy(spr, "fsr");
- break;
- case REG_BTR_MASK :
- strcpy(spr, "btr");
- break;
- case REG_EDR_MASK :
- strcpy(spr, "edr");
- break;
- case REG_PID_MASK :
- strcpy(spr, "pid");
- break;
- case REG_ZPR_MASK :
- strcpy(spr, "zpr");
- break;
- case REG_TLBX_MASK :
- strcpy(spr, "tlbx");
- break;
- case REG_TLBLO_MASK :
- strcpy(spr, "tlblo");
- break;
- case REG_TLBHI_MASK :
- strcpy(spr, "tlbhi");
- break;
- case REG_TLBSX_MASK :
- strcpy(spr, "tlbsx");
- break;
- default :
- {
- if ( ((((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask) & 0xE000) == REG_PVR_MASK) {
- snprintf(tmpstr, sizeof(tmpstr), "%s%u", pvr_register_prefix,
- (unsigned short)(((instr & IMM_MASK) >> IMM_LOW) ^
- op->immval_mask) ^ REG_PVR_MASK);
- return(strdup(tmpstr));
- } else {
- strcpy(spr, "pc");
- }
- }
- break;
- }
-
- snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix, spr);
- return(strdup(tmpstr));
+/* Returns NULL for PVR registers, which should be rendered differently. */
+static const char *get_special_name(int special)
+{
+ switch (special) {
+ case REG_MSR_MASK:
+ return register_prefix "msr";
+ case REG_PC_MASK:
+ return register_prefix "pc";
+ case REG_EAR_MASK:
+ return register_prefix "ear";
+ case REG_ESR_MASK:
+ return register_prefix "esr";
+ case REG_FSR_MASK:
+ return register_prefix "fsr";
+ case REG_BTR_MASK:
+ return register_prefix "btr";
+ case REG_EDR_MASK:
+ return register_prefix "edr";
+ case REG_PID_MASK:
+ return register_prefix "pid";
+ case REG_ZPR_MASK:
+ return register_prefix "zpr";
+ case REG_TLBX_MASK:
+ return register_prefix "tlbx";
+ case REG_TLBLO_MASK:
+ return register_prefix "tlblo";
+ case REG_TLBHI_MASK:
+ return register_prefix "tlbhi";
+ case REG_TLBSX_MASK:
+ return register_prefix "tlbsx";
+ default:
+ if ((special & 0xE000) == REG_PVR_MASK) {
+ /* pvr register */
+ return NULL;
+ }
+ return register_prefix "pc";
+ }
}
static unsigned long
@@ -739,6 +703,8 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
static int prev_insn_vma = -1; /*init the prev insn vma */
int curr_insn_vma = info->buffer_vma;
+ int special;
+ const char *special_name;
info->bytes_per_chunk = 4;
@@ -799,12 +765,26 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
op->name, get_field_r1(inst), get_field_rfsl(inst));
break;
case INST_TYPE_RD_SPECIAL:
- fprintf_func(stream, "%s\t" PRIreg ", %s",
- op->name, get_field_rd(inst), get_field_special(inst, op));
+ special = get_field_special(inst, op);
+ special_name = get_special_name(special);
+ if (special_name) {
+ fprintf_func(stream, "%s\t" PRIreg ", %s",
+ op->name, get_field_rd(inst), special_name);
+ } else {
+ fprintf_func(stream, "%s\t" PRIreg ", " PRIpvr,
+ op->name, get_field_rd(inst), special ^ REG_PVR_MASK);
+ }
break;
case INST_TYPE_SPECIAL_R1:
- fprintf_func(stream, "%s\t%s, " PRIreg,
- op->name, get_field_special(inst, op), get_field_r1(inst));
+ special = get_field_special(inst, op);
+ special_name = get_special_name(special);
+ if (special_name) {
+ fprintf_func(stream, "%s\t%s, " PRIreg,
+ op->name, special_name, get_field_r1(inst));
+ } else {
+ fprintf_func(stream, "%s\t" PRIpvr ", " PRIreg,
+ op->name, special ^ REG_PVR_MASK, get_field_r1(inst));
+ }
break;
case INST_TYPE_RD_R1:
fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 21/27] disas/riscv: Use GString in format_inst
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (19 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 20/27] disas/microblaze: Split get_field_special Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:24 ` Pierrick Bouvier
2024-06-04 0:15 ` Alistair Francis
2024-04-12 7:33 ` [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf() Richard Henderson
` (5 subsequent siblings)
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Allocate and fill a GString instead of snprintf and
appending to a fixed sized buffer.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/riscv.c | 209 ++++++++++++++++++++++----------------------------
1 file changed, 92 insertions(+), 117 deletions(-)
diff --git a/disas/riscv.c b/disas/riscv.c
index e236c8b5b7..f60f9bda43 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -4757,272 +4757,249 @@ static size_t inst_length(rv_inst inst)
/* format instruction */
-static void append(char *s1, const char *s2, size_t n)
-{
- size_t l1 = strlen(s1);
- if (n - l1 - 1 > 0) {
- strncat(s1, s2, n - l1);
- }
-}
-
-static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
+static GString *format_inst(size_t tab, rv_decode *dec)
{
const rv_opcode_data *opcode_data = dec->opcode_data;
- char tmp[64];
+ GString *buf = g_string_sized_new(64);
const char *fmt;
fmt = opcode_data[dec->op].format;
while (*fmt) {
switch (*fmt) {
case 'O':
- append(buf, opcode_data[dec->op].name, buflen);
+ g_string_append(buf, opcode_data[dec->op].name);
break;
case '(':
- append(buf, "(", buflen);
- break;
case ',':
- append(buf, ",", buflen);
- break;
case ')':
- append(buf, ")", buflen);
- break;
case '-':
- append(buf, "-", buflen);
+ g_string_append_c(buf, *fmt);
break;
case 'b':
- snprintf(tmp, sizeof(tmp), "%d", dec->bs);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->bs);
break;
case 'n':
- snprintf(tmp, sizeof(tmp), "%d", dec->rnum);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->rnum);
break;
case '0':
- append(buf, rv_ireg_name_sym[dec->rd], buflen);
+ g_string_append(buf, rv_ireg_name_sym[dec->rd]);
break;
case '1':
- append(buf, rv_ireg_name_sym[dec->rs1], buflen);
+ g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
break;
case '2':
- append(buf, rv_ireg_name_sym[dec->rs2], buflen);
+ g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
break;
case '3':
- append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rd] :
- rv_freg_name_sym[dec->rd],
- buflen);
+ if (dec->cfg->ext_zfinx) {
+ g_string_append(buf, rv_ireg_name_sym[dec->rd]);
+ } else {
+ g_string_append(buf, rv_freg_name_sym[dec->rd]);
+ }
break;
case '4':
- append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs1] :
- rv_freg_name_sym[dec->rs1],
- buflen);
+ if (dec->cfg->ext_zfinx) {
+ g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
+ } else {
+ g_string_append(buf, rv_freg_name_sym[dec->rs1]);
+ }
break;
case '5':
- append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs2] :
- rv_freg_name_sym[dec->rs2],
- buflen);
+ if (dec->cfg->ext_zfinx) {
+ g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
+ } else {
+ g_string_append(buf, rv_freg_name_sym[dec->rs2]);
+ }
break;
case '6':
- append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs3] :
- rv_freg_name_sym[dec->rs3],
- buflen);
+ if (dec->cfg->ext_zfinx) {
+ g_string_append(buf, rv_ireg_name_sym[dec->rs3]);
+ } else {
+ g_string_append(buf, rv_freg_name_sym[dec->rs3]);
+ }
break;
case '7':
- snprintf(tmp, sizeof(tmp), "%d", dec->rs1);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->rs1);
break;
case 'i':
- snprintf(tmp, sizeof(tmp), "%d", dec->imm);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->imm);
break;
case 'u':
- snprintf(tmp, sizeof(tmp), "%u", ((uint32_t)dec->imm & 0b111111));
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%u", ((uint32_t)dec->imm & 0b111111));
break;
case 'j':
- snprintf(tmp, sizeof(tmp), "%d", dec->imm1);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->imm1);
break;
case 'o':
- snprintf(tmp, sizeof(tmp), "%d", dec->imm);
- append(buf, tmp, buflen);
- while (strlen(buf) < tab * 2) {
- append(buf, " ", buflen);
+ g_string_append_printf(buf, "%d", dec->imm);
+ while (buf->len < tab * 2) {
+ g_string_append_c(buf, ' ');
}
- snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
- dec->pc + dec->imm);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
break;
case 'U':
fmt++;
- snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "%d", dec->imm >> 12);
if (*fmt == 'o') {
- while (strlen(buf) < tab * 2) {
- append(buf, " ", buflen);
+ while (buf->len < tab * 2) {
+ g_string_append_c(buf, ' ');
}
- snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
- dec->pc + dec->imm);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
}
break;
case 'c': {
const char *name = csr_name(dec->imm & 0xfff);
if (name) {
- append(buf, name, buflen);
+ g_string_append(buf, name);
} else {
- snprintf(tmp, sizeof(tmp), "0x%03x", dec->imm & 0xfff);
- append(buf, tmp, buflen);
+ g_string_append_printf(buf, "0x%03x", dec->imm & 0xfff);
}
break;
}
case 'r':
switch (dec->rm) {
case rv_rm_rne:
- append(buf, "rne", buflen);
+ g_string_append(buf, "rne");
break;
case rv_rm_rtz:
- append(buf, "rtz", buflen);
+ g_string_append(buf, "rtz");
break;
case rv_rm_rdn:
- append(buf, "rdn", buflen);
+ g_string_append(buf, "rdn");
break;
case rv_rm_rup:
- append(buf, "rup", buflen);
+ g_string_append(buf, "rup");
break;
case rv_rm_rmm:
- append(buf, "rmm", buflen);
+ g_string_append(buf, "rmm");
break;
case rv_rm_dyn:
- append(buf, "dyn", buflen);
+ g_string_append(buf, "dyn");
break;
default:
- append(buf, "inv", buflen);
+ g_string_append(buf, "inv");
break;
}
break;
case 'p':
if (dec->pred & rv_fence_i) {
- append(buf, "i", buflen);
+ g_string_append_c(buf, 'i');
}
if (dec->pred & rv_fence_o) {
- append(buf, "o", buflen);
+ g_string_append_c(buf, 'o');
}
if (dec->pred & rv_fence_r) {
- append(buf, "r", buflen);
+ g_string_append_c(buf, 'r');
}
if (dec->pred & rv_fence_w) {
- append(buf, "w", buflen);
+ g_string_append_c(buf, 'w');
}
break;
case 's':
if (dec->succ & rv_fence_i) {
- append(buf, "i", buflen);
+ g_string_append_c(buf, 'i');
}
if (dec->succ & rv_fence_o) {
- append(buf, "o", buflen);
+ g_string_append_c(buf, 'o');
}
if (dec->succ & rv_fence_r) {
- append(buf, "r", buflen);
+ g_string_append_c(buf, 'r');
}
if (dec->succ & rv_fence_w) {
- append(buf, "w", buflen);
+ g_string_append_c(buf, 'w');
}
break;
case '\t':
- while (strlen(buf) < tab) {
- append(buf, " ", buflen);
+ while (buf->len < tab) {
+ g_string_append_c(buf, ' ');
}
break;
case 'A':
if (dec->aq) {
- append(buf, ".aq", buflen);
+ g_string_append(buf, ".aq");
}
break;
case 'R':
if (dec->rl) {
- append(buf, ".rl", buflen);
+ g_string_append(buf, ".rl");
}
break;
case 'l':
- append(buf, ",v0", buflen);
+ g_string_append(buf, ",v0");
break;
case 'm':
if (dec->vm == 0) {
- append(buf, ",v0.t", buflen);
+ g_string_append(buf, ",v0.t");
}
break;
case 'D':
- append(buf, rv_vreg_name_sym[dec->rd], buflen);
+ g_string_append(buf, rv_vreg_name_sym[dec->rd]);
break;
case 'E':
- append(buf, rv_vreg_name_sym[dec->rs1], buflen);
+ g_string_append(buf, rv_vreg_name_sym[dec->rs1]);
break;
case 'F':
- append(buf, rv_vreg_name_sym[dec->rs2], buflen);
+ g_string_append(buf, rv_vreg_name_sym[dec->rs2]);
break;
case 'G':
- append(buf, rv_vreg_name_sym[dec->rs3], buflen);
+ g_string_append(buf, rv_vreg_name_sym[dec->rs3]);
break;
case 'v': {
- char nbuf[32] = {0};
const int sew = 1 << (((dec->vzimm >> 3) & 0b111) + 3);
- sprintf(nbuf, "%d", sew);
const int lmul = dec->vzimm & 0b11;
const int flmul = (dec->vzimm >> 2) & 1;
const char *vta = (dec->vzimm >> 6) & 1 ? "ta" : "tu";
const char *vma = (dec->vzimm >> 7) & 1 ? "ma" : "mu";
- append(buf, "e", buflen);
- append(buf, nbuf, buflen);
- append(buf, ",m", buflen);
+
+ g_string_append_printf(buf, "e%d,m", sew);
if (flmul) {
switch (lmul) {
case 3:
- sprintf(nbuf, "f2");
+ g_string_append(buf, "f2");
break;
case 2:
- sprintf(nbuf, "f4");
+ g_string_append(buf, "f4");
break;
case 1:
- sprintf(nbuf, "f8");
- break;
+ g_string_append(buf, "f8");
+ break;
}
- append(buf, nbuf, buflen);
} else {
- sprintf(nbuf, "%d", 1 << lmul);
- append(buf, nbuf, buflen);
+ g_string_append_printf(buf, "%d", 1 << lmul);
}
- append(buf, ",", buflen);
- append(buf, vta, buflen);
- append(buf, ",", buflen);
- append(buf, vma, buflen);
+ g_string_append_c(buf, ',');
+ g_string_append(buf, vta);
+ g_string_append_c(buf, ',');
+ g_string_append(buf, vma);
break;
}
case 'x': {
switch (dec->rlist) {
case 4:
- snprintf(tmp, sizeof(tmp), "{ra}");
+ g_string_append(buf, "{ra}");
break;
case 5:
- snprintf(tmp, sizeof(tmp), "{ra, s0}");
+ g_string_append(buf, "{ra, s0}");
break;
case 15:
- snprintf(tmp, sizeof(tmp), "{ra, s0-s11}");
+ g_string_append(buf, "{ra, s0-s11}");
break;
default:
- snprintf(tmp, sizeof(tmp), "{ra, s0-s%d}", dec->rlist - 5);
+ g_string_append_printf(buf, "{ra, s0-s%d}", dec->rlist - 5);
break;
}
- append(buf, tmp, buflen);
break;
}
case 'h':
- append(buf, rv_fli_name_const[dec->imm], buflen);
+ g_string_append(buf, rv_fli_name_const[dec->imm]);
break;
default:
break;
}
fmt++;
}
+
+ return buf;
}
/* lift instruction to pseudo-instruction */
@@ -5108,9 +5085,8 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
/* disassemble instruction */
-static void
-disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
- RISCVCPUConfig *cfg)
+static GString *disasm_inst(rv_isa isa, uint64_t pc, rv_inst inst,
+ RISCVCPUConfig *cfg)
{
rv_decode dec = { 0 };
dec.pc = pc;
@@ -5157,7 +5133,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
decode_inst_operands(&dec, isa);
decode_inst_decompress(&dec, isa);
decode_inst_lift_pseudo(&dec);
- format_inst(buf, buflen, 24, &dec);
+ return format_inst(24, &dec);
}
#define INST_FMT_2 "%04" PRIx64 " "
@@ -5168,7 +5144,6 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
static int
print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
{
- char buf[128] = { 0 };
bfd_byte packet[2];
rv_inst inst = 0;
size_t len = 2;
@@ -5209,9 +5184,9 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
}
}
- disasm_inst(buf, sizeof(buf), isa, memaddr, inst,
- (RISCVCPUConfig *)info->target_info);
- (*info->fprintf_func)(info->stream, "%s", buf);
+ g_autoptr(GString) str =
+ disasm_inst(isa, memaddr, inst, (RISCVCPUConfig *)info->target_info);
+ (*info->fprintf_func)(info->stream, "%s", str->str);
return len;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (20 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 21/27] disas/riscv: Use GString in format_inst Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 9:48 ` Peter Maydell
2024-06-03 19:02 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 23/27] hw/misc/imx: " Richard Henderson
` (4 subsequent siblings)
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Message-Id: <20240411104340.6617-5-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/flatload.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 5b62aa0a2b..1bb2f400bc 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -692,7 +692,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
char buf[16];
/* Create the file name */
- sprintf(buf, "/lib/lib%d.so", id);
+ snprintf(buf, sizeof(buf), "/lib/lib%d.so", id);
/* Open the file up */
bprm.filename = buf;
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 23/27] hw/misc/imx: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (21 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf() Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 7:33 ` [PATCH v3 24/27] hw/net/rocker: " Richard Henderson
` (3 subsequent siblings)
26 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Peter Maydell
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20240411104340.6617-6-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/misc/imx25_ccm.c | 2 +-
hw/misc/imx31_ccm.c | 2 +-
hw/misc/imx6_ccm.c | 4 ++--
hw/misc/imx6_src.c | 2 +-
hw/misc/imx6ul_ccm.c | 4 ++--
hw/misc/imx7_src.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/ssi/imx_spi.c | 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/misc/imx25_ccm.c b/hw/misc/imx25_ccm.c
index d888966014..faa726a86a 100644
--- a/hw/misc/imx25_ccm.c
+++ b/hw/misc/imx25_ccm.c
@@ -91,7 +91,7 @@ static const char *imx25_ccm_reg_name(uint32_t reg)
case IMX25_CCM_LPIMR1_REG:
return "lpimr1";
default:
- sprintf(unknown, "[%u ?]", reg);
+ snprintf(unknown, sizeof(unknown), "[%u ?]", reg);
return unknown;
}
}
diff --git a/hw/misc/imx31_ccm.c b/hw/misc/imx31_ccm.c
index a9059bb1f7..125d4fceeb 100644
--- a/hw/misc/imx31_ccm.c
+++ b/hw/misc/imx31_ccm.c
@@ -89,7 +89,7 @@ static const char *imx31_ccm_reg_name(uint32_t reg)
case IMX31_CCM_PDR2_REG:
return "PDR2";
default:
- sprintf(unknown, "[%u ?]", reg);
+ snprintf(unknown, sizeof(unknown), "[%u ?]", reg);
return unknown;
}
}
diff --git a/hw/misc/imx6_ccm.c b/hw/misc/imx6_ccm.c
index 56489d8b57..b1def7f05b 100644
--- a/hw/misc/imx6_ccm.c
+++ b/hw/misc/imx6_ccm.c
@@ -85,7 +85,7 @@ static const char *imx6_ccm_reg_name(uint32_t reg)
case CCM_CMEOR:
return "CMEOR";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
@@ -224,7 +224,7 @@ static const char *imx6_analog_reg_name(uint32_t reg)
case USB_ANALOG_DIGPROG:
return "USB_ANALOG_DIGPROG";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
diff --git a/hw/misc/imx6_src.c b/hw/misc/imx6_src.c
index 0c6003559f..3766bdf561 100644
--- a/hw/misc/imx6_src.c
+++ b/hw/misc/imx6_src.c
@@ -68,7 +68,7 @@ static const char *imx6_src_reg_name(uint32_t reg)
case SRC_GPR10:
return "SRC_GPR10";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
diff --git a/hw/misc/imx6ul_ccm.c b/hw/misc/imx6ul_ccm.c
index bbc0be9921..0ac49ea34b 100644
--- a/hw/misc/imx6ul_ccm.c
+++ b/hw/misc/imx6ul_ccm.c
@@ -143,7 +143,7 @@ static const char *imx6ul_ccm_reg_name(uint32_t reg)
case CCM_CMEOR:
return "CMEOR";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
@@ -274,7 +274,7 @@ static const char *imx6ul_analog_reg_name(uint32_t reg)
case USB_ANALOG_DIGPROG:
return "USB_ANALOG_DIGPROG";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
index b3725ff6e7..d19f0450d4 100644
--- a/hw/misc/imx7_src.c
+++ b/hw/misc/imx7_src.c
@@ -75,7 +75,7 @@ static const char *imx7_src_reg_name(uint32_t reg)
case SRC_GPR10:
return "SRC_GPR10";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index cee84af7ba..8c91d20d44 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -41,7 +41,7 @@
static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
{
static char tmp[20];
- sprintf(tmp, "index %d", index);
+ snprintf(tmp, sizeof(tmp), "index %d", index);
return tmp;
}
diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index d8a7583ff3..12d897d306 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -53,7 +53,7 @@ static const char *imx_spi_reg_name(uint32_t reg)
case ECSPI_MSGDATA:
return "ECSPI_MSGDATA";
default:
- sprintf(unknown, "%u ?", reg);
+ snprintf(unknown, sizeof(unknown), "%u ?", reg);
return unknown;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 24/27] hw/net/rocker: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (22 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 23/27] hw/misc/imx: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 10:58 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf Richard Henderson
` (2 subsequent siblings)
26 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Message-Id: <20240411104340.6617-7-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/net/rocker/rocker.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 7ea8eb6ba5..03ac7a7ae9 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -915,40 +915,40 @@ static const char *rocker_reg_name(void *opaque, hwaddr addr)
switch (index) {
case 0:
- sprintf(ring_name, "cmd");
+ snprintf(ring_name, sizeof(ring_name), "cmd");
break;
case 1:
- sprintf(ring_name, "event");
+ snprintf(ring_name, sizeof(ring_name), "event");
break;
default:
- sprintf(ring_name, "%s-%d", index % 2 ? "rx" : "tx",
- (index - 2) / 2);
+ snprintf(ring_name, sizeof(ring_name), "%s-%d",
+ index % 2 ? "rx" : "tx", (index - 2) / 2);
}
switch (offset) {
case ROCKER_DMA_DESC_ADDR_OFFSET:
- sprintf(buf, "Ring[%s] ADDR", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] ADDR", ring_name);
return buf;
case ROCKER_DMA_DESC_ADDR_OFFSET+4:
- sprintf(buf, "Ring[%s] ADDR+4", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] ADDR+4", ring_name);
return buf;
case ROCKER_DMA_DESC_SIZE_OFFSET:
- sprintf(buf, "Ring[%s] SIZE", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] SIZE", ring_name);
return buf;
case ROCKER_DMA_DESC_HEAD_OFFSET:
- sprintf(buf, "Ring[%s] HEAD", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] HEAD", ring_name);
return buf;
case ROCKER_DMA_DESC_TAIL_OFFSET:
- sprintf(buf, "Ring[%s] TAIL", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] TAIL", ring_name);
return buf;
case ROCKER_DMA_DESC_CTRL_OFFSET:
- sprintf(buf, "Ring[%s] CTRL", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] CTRL", ring_name);
return buf;
case ROCKER_DMA_DESC_CREDITS_OFFSET:
- sprintf(buf, "Ring[%s] CREDITS", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] CREDITS", ring_name);
return buf;
default:
- sprintf(buf, "Ring[%s] ???", ring_name);
+ snprintf(buf, sizeofbuf), "Ring[%s] ???", ring_name);
return buf;
}
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (23 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 24/27] hw/net/rocker: " Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-06-03 19:06 ` Pierrick Bouvier
2024-06-04 0:16 ` Alistair Francis
2024-04-12 7:33 ` [PATCH v3 26/27] target/arm: Replace sprintf() by snprintf() Richard Henderson
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
26 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Use g_strdup_printf instead.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240411104340.6617-8-philmd@linaro.org>
[rth: Use g_strdup_printf]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/riscv/virt.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d171e74f7b..4fdb660525 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1617,10 +1617,8 @@ static void virt_machine_instance_init(Object *obj)
static char *virt_get_aia_guests(Object *obj, Error **errp)
{
RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
- char val[32];
- sprintf(val, "%d", s->aia_guests);
- return g_strdup(val);
+ return g_strdup_printf("%d", s->aia_guests);
}
static void virt_set_aia_guests(Object *obj, const char *val, Error **errp)
@@ -1741,7 +1739,6 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
static void virt_machine_class_init(ObjectClass *oc, void *data)
{
- char str[128];
MachineClass *mc = MACHINE_CLASS(oc);
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
@@ -1767,7 +1764,6 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
#endif
-
object_class_property_add_bool(oc, "aclint", virt_get_aclint,
virt_set_aclint);
object_class_property_set_description(oc, "aclint",
@@ -1785,9 +1781,14 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
object_class_property_add_str(oc, "aia-guests",
virt_get_aia_guests,
virt_set_aia_guests);
- sprintf(str, "Set number of guest MMIO pages for AIA IMSIC. Valid value "
- "should be between 0 and %d.", VIRT_IRQCHIP_MAX_GUESTS);
- object_class_property_set_description(oc, "aia-guests", str);
+ {
+ g_autofree char *str =
+ g_strdup_printf("Set number of guest MMIO pages for AIA IMSIC. "
+ "Valid value should be between 0 and %d.",
+ VIRT_IRQCHIP_MAX_GUESTS);
+ object_class_property_set_description(oc, "aia-guests", str);
+ }
+
object_class_property_add(oc, "acpi", "OnOffAuto",
virt_get_acpi, virt_set_acpi,
NULL, NULL);
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 26/27] target/arm: Replace sprintf() by snprintf()
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (24 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
26 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Peter Maydell
From: Philippe Mathieu-Daudé <philmd@linaro.org>
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20240411104340.6617-9-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 985b1efe16..f0f4fe6714 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -433,7 +433,7 @@ void aarch64_add_sve_properties(Object *obj)
for (vq = 1; vq <= ARM_MAX_VQ; ++vq) {
char name[8];
- sprintf(name, "sve%d", vq * 128);
+ snprintf(name, sizeof(name), "sve%d", vq * 128);
object_property_add(obj, name, "bool", cpu_arm_get_vq,
cpu_arm_set_vq, NULL, &cpu->sve_vq);
}
@@ -458,7 +458,7 @@ void aarch64_add_sme_properties(Object *obj)
for (vq = 1; vq <= ARM_MAX_VQ; vq <<= 1) {
char name[8];
- sprintf(name, "sme%d", vq * 128);
+ snprintf(name, sizeof(name), "sme%d", vq * 128);
object_property_add(obj, name, "bool", cpu_arm_get_vq,
cpu_arm_set_vq, NULL, &cpu->sme_vq);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
` (25 preceding siblings ...)
2024-04-12 7:33 ` [PATCH v3 26/27] target/arm: Replace sprintf() by snprintf() Richard Henderson
@ 2024-04-12 7:33 ` Richard Henderson
2024-04-12 10:59 ` Philippe Mathieu-Daudé
` (2 more replies)
26 siblings, 3 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 7:33 UTC (permalink / raw
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/kvm/kvm.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index e68cbe9293..1fc809b2f9 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -5275,7 +5275,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
uint64_t code;
int ret;
bool ctx_invalid;
- char str[256];
KVMState *state;
switch (run->exit_reason) {
@@ -5335,15 +5334,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
case KVM_EXIT_NOTIFY:
ctx_invalid = !!(run->notify.flags & KVM_NOTIFY_CONTEXT_INVALID);
state = KVM_STATE(current_accel());
- sprintf(str, "Encounter a notify exit with %svalid context in"
- " guest. There can be possible misbehaves in guest."
- " Please have a look.", ctx_invalid ? "in" : "");
if (ctx_invalid ||
state->notify_vmexit == NOTIFY_VMEXIT_OPTION_INTERNAL_ERROR) {
- warn_report("KVM internal error: %s", str);
+ warn_report("KVM internal error: Encountered a notify exit "
+ "with invalid context in guest.");
ret = -1;
} else {
- warn_report_once("KVM: %s", str);
+ warn_report_once("KVM: Encounterd a notify exit with valid "
+ "context in guest. "
+ "The guest could be misbehaving. "
+ "Please have a look.");
ret = 0;
}
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* Re: [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf()
2024-04-12 7:33 ` [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf() Richard Henderson
@ 2024-04-12 9:48 ` Peter Maydell
2024-06-03 19:02 ` Pierrick Bouvier
1 sibling, 0 replies; 63+ messages in thread
From: Peter Maydell @ 2024-04-12 9:48 UTC (permalink / raw
To: Richard Henderson; +Cc: qemu-devel, Philippe Mathieu-Daudé
On Fri, 12 Apr 2024 at 08:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> 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>
> Message-Id: <20240411104340.6617-5-philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Rather than this, I think we should use my patch which
drops this dead code entirely:
https://patchew.org/QEMU/20240411115313.680433-1-peter.maydell@linaro.org/
-- PMM
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
@ 2024-04-12 10:00 ` Philippe Mathieu-Daudé
2024-06-03 19:40 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 10:00 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Require that the caller output the offset and increment bufptr.
> Use QEMU_HEXDUMP_LINE_BYTES in vhost_vdpa_dump_config instead
> of raw integer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 2 +-
> hw/virtio/vhost-vdpa.c | 4 ++--
> util/hexdump.c | 13 ++++++-------
> hw/virtio/trace-events | 2 +-
> 4 files changed, 10 insertions(+), 11 deletions(-)
> @@ -58,8 +57,8 @@ void qemu_hexdump(FILE *fp, const char *prefix,
>
> for (b = 0; b < size; b += QEMU_HEXDUMP_LINE_BYTES) {
> len = size - b;
> - qemu_hexdump_line(line, b, bufptr, len, true);
> - fprintf(fp, "%s: %s\n", prefix, line);
> + qemu_hexdump_line(line, bufptr + b, len, true);
> + fprintf(fp, "%s: %04x: %s\n", prefix, b, line);
Clever :)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 02/27] util/hexdump: Remove ascii parameter from qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 02/27] util/hexdump: Remove ascii " Richard Henderson
@ 2024-04-12 10:03 ` Philippe Mathieu-Daudé
2024-06-03 17:42 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 10:03 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Split out asciidump_line as a separate function, local to hexdump.c,
> for use by qemu_hexdump. Use "%-*s" to generate the alignment
> between the hex and the ascii, rather than explicit spaces.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 3 +--
> hw/virtio/vhost-vdpa.c | 2 +-
> util/hexdump.c | 54 ++++++++++++++++++++++++------------------
> 3 files changed, 33 insertions(+), 26 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 24/27] hw/net/rocker: Replace sprintf() by snprintf()
2024-04-12 7:33 ` [PATCH v3 24/27] hw/net/rocker: " Richard Henderson
@ 2024-04-12 10:58 ` Philippe Mathieu-Daudé
2024-04-12 14:51 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 10:58 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> 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>
> Message-Id: <20240411104340.6617-7-philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/net/rocker/rocker.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
> switch (offset) {
> case ROCKER_DMA_DESC_ADDR_OFFSET:
> - sprintf(buf, "Ring[%s] ADDR", ring_name);
> + snprintf(buf, sizeofbuf), "Ring[%s] ADDR", ring_name);
Ideally we should convert the DEBUG_FOO guards to trace events,
to avoid to maintain dead code.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
@ 2024-04-12 10:59 ` Philippe Mathieu-Daudé
2024-04-16 15:32 ` Peter Maydell
2024-04-16 15:32 ` Zhao Liu
2 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 10:59 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/kvm/kvm.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 24/27] hw/net/rocker: Replace sprintf() by snprintf()
2024-04-12 10:58 ` Philippe Mathieu-Daudé
@ 2024-04-12 14:51 ` Richard Henderson
0 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 14:51 UTC (permalink / raw
To: Philippe Mathieu-Daudé, qemu-devel
On 4/12/24 03:58, Philippe Mathieu-Daudé wrote:
> On 12/4/24 09:33, Richard Henderson wrote:
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> 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>
>> Message-Id: <20240411104340.6617-7-philmd@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> hw/net/rocker/rocker.c | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)
>
>
>> switch (offset) {
>> case ROCKER_DMA_DESC_ADDR_OFFSET:
>> - sprintf(buf, "Ring[%s] ADDR", ring_name);
>> + snprintf(buf, sizeofbuf), "Ring[%s] ADDR", ring_name);
>
> Ideally we should convert the DEBUG_FOO guards to trace events,
> to avoid to maintain dead code.
Grr, I knew there was another of these that needed fixing up.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line Richard Henderson
@ 2024-04-12 17:41 ` Philippe Mathieu-Daudé
2024-04-12 18:59 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:41 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Allocate a new, or append to an existing GString instead of
> using a fixed sized buffer. Require the caller to determine
> the length of the line -- do not bound len here.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 15 ++++++++++-----
> hw/virtio/vhost-vdpa.c | 14 ++++++++------
> util/hexdump.c | 29 +++++++++++++++++------------
> 3 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index d0c5386e6c..7311fb36ca 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -252,12 +252,17 @@ static inline const char *yes_no(bool b)
> */
> int parse_debug_env(const char *name, int max, int initial);
>
> -/*
> - * Hexdump a line of a byte buffer into a hexadecimal/ASCII buffer
> +/**
> + * qemu_hexdump_line:
> + * @str: GString into which to append
> + * @buf: buffer to dump
> + * @len: number of bytes to dump
> + *
> + * Append @len bytes of @buf as hexadecimal into @str.
> + * If @str is NULL, allocate a new string and return it;
> + * otherwise return @str.
> */
> -#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, size_t len);
> +GString *qemu_hexdump_line(GString *str, const void *buf, size_t len);
> diff --git a/util/hexdump.c b/util/hexdump.c
> index dbc536fe84..521e346bc6 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -16,22 +16,25 @@
> #include "qemu/osdep.h"
> #include "qemu/cutils.h"
>
> -void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
> +GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
> {
> - const char *buf = bufptr;
> - int i, c;
> + const uint8_t *buf = vbuf;
> + size_t i;
>
> - if (len > QEMU_HEXDUMP_LINE_BYTES) {
> - len = QEMU_HEXDUMP_LINE_BYTES;
> + if (str == NULL) {
> + /* Estimate the length of the output to avoid reallocs. */
> + i = len * 3 + len / 4;
> + str = g_string_sized_new(i + 1);
> }
[*]
else {
g_string_truncate(str, 0);
}
>
> for (i = 0; i < len; i++) {
> if (i != 0 && (i % 4) == 0) {
> - *line++ = ' ';
> + g_string_append_c(str, ' ');
> }
> - line += sprintf(line, " %02x", (unsigned char)buf[i]);
> + g_string_append_printf(str, " %02x", buf[i]);
> }
> - *line = '\0';
> +
> + return str;
> }
>
> static void asciidump_line(char *line, const void *bufptr, size_t len)
> @@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
> *line = '\0';
> }
>
> +#define QEMU_HEXDUMP_LINE_BYTES 16
> #define QEMU_HEXDUMP_LINE_WIDTH \
> (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
>
> void qemu_hexdump(FILE *fp, const char *prefix,
> const void *bufptr, size_t size)
> {
> - char line[QEMU_HEXDUMP_LINE_LEN];
> + g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
> size_t b, len;
>
> for (b = 0; b < size; b += len) {
> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
>
> - qemu_hexdump_line(line, bufptr + b, len);
> + g_string_truncate(str, 0);
Shouldn't we truncate in [*] ?
> + qemu_hexdump_line(str, bufptr + b, len);
> asciidump_line(ascii, bufptr + b, len);
>
> - fprintf(fp, "%s: %04x: %-*s %s\n",
> - prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
> + fprintf(fp, "%s: %04zx: %-*s %s\n",
> + prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
> }
>
> }
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line Richard Henderson
@ 2024-04-12 17:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:42 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Generalize the current 1 byte unit and 4 byte blocking
> within the output.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 6 +++++-
> hw/virtio/vhost-vdpa.c | 2 +-
> util/hexdump.c | 30 +++++++++++++++++++++---------
> 3 files changed, 27 insertions(+), 11 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x"
2024-04-12 7:33 ` [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x" Richard Henderson
@ 2024-04-12 17:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:43 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Trivial arithmetic can be used for emitting the nibbles,
> rather than full-blown printf formatting.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> util/hexdump.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line Richard Henderson
@ 2024-04-12 17:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:45 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Using qemu_hexdump_line both fixes the deprecation warning and
> simplifies the code base.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>`
> [rth: Use qemu_hexdump_line]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> system/qtest.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 ` [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf Richard Henderson
@ 2024-04-12 17:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:45 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Using qemu_hexdump_line both fixes the deprecation warning and
> simplifies the code base.
>
> Note that this drops the "0x" prefix to every byte, which should
> be of no consequence to tracing.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/scsi/scsi-disk.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 11/27] backends/tpm: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 ` [PATCH v3 11/27] backends/tpm: " Richard Henderson
@ 2024-04-12 17:47 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-12 17:47 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Stefan Berger
On 12/4/24 09:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Using qemu_hexdump_line both fixes the deprecation warning and
> simplifies the code base.
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> [rth: Keep the linebreaks every 16 bytes]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> backends/tpm/tpm_util.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line
2024-04-12 17:41 ` Philippe Mathieu-Daudé
@ 2024-04-12 18:59 ` Richard Henderson
2024-04-13 9:45 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2024-04-12 18:59 UTC (permalink / raw
To: Philippe Mathieu-Daudé, qemu-devel
On 4/12/24 10:41, Philippe Mathieu-Daudé wrote:
>> -void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
>> +GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
>> {
>> - const char *buf = bufptr;
>> - int i, c;
>> + const uint8_t *buf = vbuf;
>> + size_t i;
>> - if (len > QEMU_HEXDUMP_LINE_BYTES) {
>> - len = QEMU_HEXDUMP_LINE_BYTES;
>> + if (str == NULL) {
>> + /* Estimate the length of the output to avoid reallocs. */
>> + i = len * 3 + len / 4;
>> + str = g_string_sized_new(i + 1);
>> }
>
> [*]
> else {
> g_string_truncate(str, 0);
> }
>
...
>> @@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void *bufptr, size_t len)
>> *line = '\0';
>> }
>> +#define QEMU_HEXDUMP_LINE_BYTES 16
>> #define QEMU_HEXDUMP_LINE_WIDTH \
>> (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
>> void qemu_hexdump(FILE *fp, const char *prefix,
>> const void *bufptr, size_t size)
>> {
>> - char line[QEMU_HEXDUMP_LINE_LEN];
>> + g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
>> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
>> size_t b, len;
>> for (b = 0; b < size; b += len) {
>> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
>> - qemu_hexdump_line(line, bufptr + b, len);
>> + g_string_truncate(str, 0);
>
> Shouldn't we truncate in [*] ?
The usage in tpm puts several lines together in one string,
adding \n in between, for output in one go.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line
2024-04-12 18:59 ` Richard Henderson
@ 2024-04-13 9:45 ` Philippe Mathieu-Daudé
2024-04-13 12:02 ` Stefan Hajnoczi
0 siblings, 1 reply; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-13 9:45 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Stefan Hajnoczi
On 12/4/24 20:59, Richard Henderson wrote:
> On 4/12/24 10:41, Philippe Mathieu-Daudé wrote:
>>> -void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
>>> +GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
>>> {
>>> - const char *buf = bufptr;
>>> - int i, c;
>>> + const uint8_t *buf = vbuf;
>>> + size_t i;
>>> - if (len > QEMU_HEXDUMP_LINE_BYTES) {
>>> - len = QEMU_HEXDUMP_LINE_BYTES;
>>> + if (str == NULL) {
>>> + /* Estimate the length of the output to avoid reallocs. */
>>> + i = len * 3 + len / 4;
>>> + str = g_string_sized_new(i + 1);
>>> }
>>
>> [*]
>> else {
>> g_string_truncate(str, 0);
>> }
>>
> ...
>>> @@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void
>>> *bufptr, size_t len)
>>> *line = '\0';
>>> }
>>> +#define QEMU_HEXDUMP_LINE_BYTES 16
>>> #define QEMU_HEXDUMP_LINE_WIDTH \
>>> (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
>>> void qemu_hexdump(FILE *fp, const char *prefix,
>>> const void *bufptr, size_t size)
>>> {
>>> - char line[QEMU_HEXDUMP_LINE_LEN];
>>> + g_autoptr(GString) str =
>>> g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
>>> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
>>> size_t b, len;
>>> for (b = 0; b < size; b += len) {
>>> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
>>> - qemu_hexdump_line(line, bufptr + b, len);
>>> + g_string_truncate(str, 0);
>>
>> Shouldn't we truncate in [*] ?
>
> The usage in tpm puts several lines together in one string,
> adding \n in between, for output in one go.
I see the trace_tpm_util_show_buffer() call. However this
isn't a recommended use of the tracing API (Cc'ing Stefan).
It breaks the "log" backend output, and is sub-optimal for
all other backends.
IMHO the TPM buffer should be traced by multiple calls of
(offset, hexbuf) instead.
Regards,
Phil.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line
2024-04-13 9:45 ` Philippe Mathieu-Daudé
@ 2024-04-13 12:02 ` Stefan Hajnoczi
0 siblings, 0 replies; 63+ messages in thread
From: Stefan Hajnoczi @ 2024-04-13 12:02 UTC (permalink / raw
To: Philippe Mathieu-Daudé
Cc: Richard Henderson, qemu-devel, Stefan Hajnoczi
On Sat, 13 Apr 2024 at 05:46, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 12/4/24 20:59, Richard Henderson wrote:
> > On 4/12/24 10:41, Philippe Mathieu-Daudé wrote:
> >>> -void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
> >>> +GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
> >>> {
> >>> - const char *buf = bufptr;
> >>> - int i, c;
> >>> + const uint8_t *buf = vbuf;
> >>> + size_t i;
> >>> - if (len > QEMU_HEXDUMP_LINE_BYTES) {
> >>> - len = QEMU_HEXDUMP_LINE_BYTES;
> >>> + if (str == NULL) {
> >>> + /* Estimate the length of the output to avoid reallocs. */
> >>> + i = len * 3 + len / 4;
> >>> + str = g_string_sized_new(i + 1);
> >>> }
> >>
> >> [*]
> >> else {
> >> g_string_truncate(str, 0);
> >> }
> >>
> > ...
> >>> @@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void
> >>> *bufptr, size_t len)
> >>> *line = '\0';
> >>> }
> >>> +#define QEMU_HEXDUMP_LINE_BYTES 16
> >>> #define QEMU_HEXDUMP_LINE_WIDTH \
> >>> (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
> >>> void qemu_hexdump(FILE *fp, const char *prefix,
> >>> const void *bufptr, size_t size)
> >>> {
> >>> - char line[QEMU_HEXDUMP_LINE_LEN];
> >>> + g_autoptr(GString) str =
> >>> g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
> >>> char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
> >>> size_t b, len;
> >>> for (b = 0; b < size; b += len) {
> >>> len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
> >>> - qemu_hexdump_line(line, bufptr + b, len);
> >>> + g_string_truncate(str, 0);
> >>
> >> Shouldn't we truncate in [*] ?
> >
> > The usage in tpm puts several lines together in one string,
> > adding \n in between, for output in one go.
>
> I see the trace_tpm_util_show_buffer() call. However this
> isn't a recommended use of the tracing API (Cc'ing Stefan).
> It breaks the "log" backend output, and is sub-optimal for
> all other backends.
>
> IMHO the TPM buffer should be traced by multiple calls of
> (offset, hexbuf) instead.
I think so too.
Stefan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
2024-04-12 10:59 ` Philippe Mathieu-Daudé
@ 2024-04-16 15:32 ` Peter Maydell
2024-04-16 15:32 ` Zhao Liu
2 siblings, 0 replies; 63+ messages in thread
From: Peter Maydell @ 2024-04-16 15:32 UTC (permalink / raw
To: Richard Henderson; +Cc: qemu-devel
On Fri, 12 Apr 2024 at 08:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/kvm/kvm.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index e68cbe9293..1fc809b2f9 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -5275,7 +5275,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> uint64_t code;
> int ret;
> bool ctx_invalid;
> - char str[256];
> KVMState *state;
>
> switch (run->exit_reason) {
> @@ -5335,15 +5334,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> case KVM_EXIT_NOTIFY:
> ctx_invalid = !!(run->notify.flags & KVM_NOTIFY_CONTEXT_INVALID);
> state = KVM_STATE(current_accel());
> - sprintf(str, "Encounter a notify exit with %svalid context in"
> - " guest. There can be possible misbehaves in guest."
> - " Please have a look.", ctx_invalid ? "in" : "");
> if (ctx_invalid ||
> state->notify_vmexit == NOTIFY_VMEXIT_OPTION_INTERNAL_ERROR) {
> - warn_report("KVM internal error: %s", str);
> + warn_report("KVM internal error: Encountered a notify exit "
> + "with invalid context in guest.");
> ret = -1;
> } else {
> - warn_report_once("KVM: %s", str);
> + warn_report_once("KVM: Encounterd a notify exit with valid "
"Encountered"
> + "context in guest. "
> + "The guest could be misbehaving. "
> + "Please have a look.");
"Please have a look" is a bit more chatty than we usually are
in warning messages...
thanks
-- PMM
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
2024-04-12 10:59 ` Philippe Mathieu-Daudé
2024-04-16 15:32 ` Peter Maydell
@ 2024-04-16 15:32 ` Zhao Liu
2 siblings, 0 replies; 63+ messages in thread
From: Zhao Liu @ 2024-04-16 15:32 UTC (permalink / raw
To: Richard Henderson; +Cc: qemu-devel
On Fri, Apr 12, 2024 at 12:33:46AM -0700, Richard Henderson wrote:
> Date: Fri, 12 Apr 2024 00:33:46 -0700
> From: Richard Henderson <richard.henderson@linaro.org>
> Subject: [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings
> X-Mailer: git-send-email 2.34.1
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/kvm/kvm.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 02/27] util/hexdump: Remove ascii parameter from qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 02/27] util/hexdump: Remove ascii " Richard Henderson
2024-04-12 10:03 ` Philippe Mathieu-Daudé
@ 2024-06-03 17:42 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-03 17:42 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Split out asciidump_line as a separate function, local to hexdump.c,
> for use by qemu_hexdump. Use "%-*s" to generate the alignment
> between the hex and the ascii, rather than explicit spaces.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 3 +--
> hw/virtio/vhost-vdpa.c | 2 +-
> util/hexdump.c | 54 ++++++++++++++++++++++++------------------
> 3 files changed, 33 insertions(+), 26 deletions(-)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 3415f5d249..d0c5386e6c 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -257,8 +257,7 @@ int parse_debug_env(const char *name, int max, int initial);
> */
> #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 int len, bool ascii);
> +void qemu_hexdump_line(char *line, const void *bufptr, size_t len);
>
> /*
> * 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 b4afa142f6..32bad5ce68 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -946,7 +946,7 @@ static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config,
>
> for (b = 0; b < config_len; b += 16) {
> len = config_len - b;
> - qemu_hexdump_line(line, config + b, len, false);
> + qemu_hexdump_line(line, config + b, len);
> trace_vhost_vdpa_dump_config(dev, b, line);
> }
> }
> diff --git a/util/hexdump.c b/util/hexdump.c
> index 7324e7b126..dbc536fe84 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -16,8 +16,7 @@
> #include "qemu/osdep.h"
> #include "qemu/cutils.h"
>
> -void qemu_hexdump_line(char *line, const void *bufptr,
> - unsigned int len, bool ascii)
> +void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
> {
> const char *buf = bufptr;
> int i, c;
'c' ...
> @@ -26,39 +25,48 @@ void qemu_hexdump_line(char *line, const void *bufptr,
> len = QEMU_HEXDUMP_LINE_BYTES;
> }
>
> - for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
> + for (i = 0; i < len; i++) {
> if (i != 0 && (i % 4) == 0) {
> *line++ = ' ';
> }
> - if (i < len) {
> - line += sprintf(line, " %02x", (unsigned char)buf[i]);
> - } else {
> - line += sprintf(line, " ");
> - }
> - }
> - if (ascii) {
> - *line++ = ' ';
> - for (i = 0; i < len; i++) {
> - c = buf[i];
> - if (c < ' ' || c > '~') {
> - c = '.';
> - }
> - *line++ = c;
... is no more used;
> - }
> + line += sprintf(line, " %02x", (unsigned char)buf[i]);
> }
> *line = '\0';
> }
>
> +static void asciidump_line(char *line, const void *bufptr, size_t len)
> +{
> + const char *buf = bufptr;
> +
> + for (size_t i = 0; i < len; i++) {
> + char c = buf[i];
> +
> + if (c < ' ' || c > '~') {
> + c = '.';
> + }
> + *line++ = c;
> + }
> + *line = '\0';
> +}
> +
> +#define QEMU_HEXDUMP_LINE_WIDTH \
> + (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
> +
> void qemu_hexdump(FILE *fp, const char *prefix,
> const void *bufptr, size_t size)
> {
> - unsigned int b, len;
> char line[QEMU_HEXDUMP_LINE_LEN];
> + char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
> + size_t b, len;
>
> - for (b = 0; b < size; b += QEMU_HEXDUMP_LINE_BYTES) {
> - len = size - b;
> - qemu_hexdump_line(line, bufptr + b, len, true);
> - fprintf(fp, "%s: %04x: %s\n", prefix, b, line);
> + for (b = 0; b < size; b += len) {
> + len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
> +
> + qemu_hexdump_line(line, bufptr + b, len);
> + asciidump_line(ascii, bufptr + b, len);
> +
> + fprintf(fp, "%s: %04x: %-*s %s\n",
'b' is size_t.
> + prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
> }
>
> }
Queued squashing:
-- >8 --
diff --git a/util/hexdump.c b/util/hexdump.c
index dbc536fe84..0f943e31e5 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -19,7 +19,7 @@
void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
{
const char *buf = bufptr;
- int i, c;
+ int i;
if (len > QEMU_HEXDUMP_LINE_BYTES) {
len = QEMU_HEXDUMP_LINE_BYTES;
@@ -65,7 +65,7 @@ void qemu_hexdump(FILE *fp, const char *prefix,
qemu_hexdump_line(line, bufptr + b, len);
asciidump_line(ascii, bufptr + b, len);
- fprintf(fp, "%s: %04x: %-*s %s\n",
+ fprintf(fp, "%s: %04zx: %-*s %s\n",
prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
}
---
^ permalink raw reply related [flat|nested] 63+ messages in thread
* Re: [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method
2024-04-12 7:33 ` [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method Richard Henderson
@ 2024-06-03 18:56 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 18:56 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 4/12/24 00:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
>
> Extract common code from reinitialize_rng_seed and load_kernel
> to rng_seed_hex_new. Using qemu_hexdump_line both fixes the
> deprecation warning and simplifies the code base.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> [rth: Use qemu_hexdump_line.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/mips/malta.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index af74008c82..664a2ae0a9 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -26,6 +26,7 @@
> #include "qemu/units.h"
> #include "qemu/bitops.h"
> #include "qemu/datadir.h"
> +#include "qemu/cutils.h"
> #include "qemu/guest-random.h"
> #include "hw/clock.h"
> #include "hw/southbridge/piix.h"
> @@ -850,15 +851,18 @@ 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 GString *rng_seed_hex_new(void)
> {
> - char *rng_seed_hex = opaque;
> uint8_t rng_seed[32];
>
> qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> - for (size_t i = 0; i < sizeof(rng_seed); ++i) {
> - sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
> - }
> + return qemu_hexdump_line(NULL, rng_seed, sizeof(rng_seed), 0, 0);
> +}
> +
> +static void reinitialize_rng_seed(void *opaque)
> +{
> + g_autoptr(GString) hex = rng_seed_hex_new();
> + memcpy(opaque, hex->str, hex->len);
> }
>
> /* Kernel */
> @@ -870,8 +874,6 @@ 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];
> size_t rng_seed_prom_offset;
>
> kernel_size = load_elf(loaderparams.kernel_filename, NULL,
> @@ -946,14 +948,13 @@ 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;
> - prom_set(prom_buf, prom_index++, "%s", rng_seed_hex);
> + {
> + g_autoptr(GString) hex = rng_seed_hex_new();
> + prom_set(prom_buf, prom_index++, "%s", hex->str);
> + }
>
> prom_set(prom_buf, prom_index++, NULL);
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 09/27] hw/ide/atapi: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 ` [PATCH v3 09/27] hw/ide/atapi: " Richard Henderson
@ 2024-06-03 18:57 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 18:57 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 4/12/24 00:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Using qemu_hexdump_line both fixes the deprecation warning and
> simplifies the code base.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/ide/atapi.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 73ec373184..fcb6cca157 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,14 +1310,9 @@ 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]);
> - }
> - trace_ide_atapi_cmd_packet(s, s->lcyl | (s->hcyl << 8), ppacket);
> - g_free(ppacket);
> + g_autoptr(GString) str =
> + qemu_hexdump_line(NULL, buf, ATAPI_PACKET_SIZE, 1, 0);
> + trace_ide_atapi_cmd_packet(s, s->lcyl | (s->hcyl << 8), str->str);
> }
>
> /*
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 10/27] hw/dma/pl330: Use qemu_hexdump_line to avoid sprintf
2024-04-12 7:33 ` [PATCH v3 10/27] hw/dma/pl330: " Richard Henderson
@ 2024-06-03 18:59 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 18:59 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 4/12/24 00:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Using qemu_hexdump_line both fixes the deprecation warning and
> simplifies the code base.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/dma/pl330.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
> index 70a502d245..5f89295af3 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,22 +318,14 @@ typedef struct PL330InsnDesc {
>
> static void pl330_hexdump(uint8_t *buf, size_t size)
> {
> - unsigned int b, i, len;
> - char tmpbuf[80];
> + g_autoptr(GString) str = g_string_sized_new(64);
> + size_t 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]);
> - }
> - trace_pl330_hexdump(b, tmpbuf);
> + for (b = 0; b < size; b += len) {
> + len = MIN(16, size - b);
> + g_string_truncate(str, 0);
> + qemu_hexdump_line(str, buf + b, len, 1, 4);
> + trace_pl330_hexdump(b, str->str);
> }
> }
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf()
2024-04-12 7:33 ` [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf() Richard Henderson
2024-04-12 9:48 ` Peter Maydell
@ 2024-06-03 19:02 ` Pierrick Bouvier
1 sibling, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:02 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 4/12/24 00:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> 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>
> Message-Id: <20240411104340.6617-5-philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/flatload.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 5b62aa0a2b..1bb2f400bc 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -692,7 +692,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
> char buf[16];
>
> /* Create the file name */
> - sprintf(buf, "/lib/lib%d.so", id);
> + snprintf(buf, sizeof(buf), "/lib/lib%d.so", id);
>
> /* Open the file up */
> bprm.filename = buf;
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf
2024-04-12 7:33 ` [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf Richard Henderson
@ 2024-06-03 19:06 ` Pierrick Bouvier
2024-06-04 0:16 ` Alistair Francis
1 sibling, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:06 UTC (permalink / raw
To: Richard Henderson, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 4/12/24 00:33, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Use g_strdup_printf instead.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20240411104340.6617-8-philmd@linaro.org>
> [rth: Use g_strdup_printf]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/riscv/virt.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d171e74f7b..4fdb660525 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1617,10 +1617,8 @@ static void virt_machine_instance_init(Object *obj)
> static char *virt_get_aia_guests(Object *obj, Error **errp)
> {
> RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
> - char val[32];
>
> - sprintf(val, "%d", s->aia_guests);
> - return g_strdup(val);
> + return g_strdup_printf("%d", s->aia_guests);
> }
>
> static void virt_set_aia_guests(Object *obj, const char *val, Error **errp)
> @@ -1741,7 +1739,6 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>
> static void virt_machine_class_init(ObjectClass *oc, void *data)
> {
> - char str[128];
> MachineClass *mc = MACHINE_CLASS(oc);
> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
> @@ -1767,7 +1764,6 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> #endif
>
> -
> object_class_property_add_bool(oc, "aclint", virt_get_aclint,
> virt_set_aclint);
> object_class_property_set_description(oc, "aclint",
> @@ -1785,9 +1781,14 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> object_class_property_add_str(oc, "aia-guests",
> virt_get_aia_guests,
> virt_set_aia_guests);
> - sprintf(str, "Set number of guest MMIO pages for AIA IMSIC. Valid value "
> - "should be between 0 and %d.", VIRT_IRQCHIP_MAX_GUESTS);
> - object_class_property_set_description(oc, "aia-guests", str);
> + {
> + g_autofree char *str =
> + g_strdup_printf("Set number of guest MMIO pages for AIA IMSIC. "
> + "Valid value should be between 0 and %d.",
> + VIRT_IRQCHIP_MAX_GUESTS);
> + object_class_property_set_description(oc, "aia-guests", str);
> + }
> +
> object_class_property_add(oc, "acpi", "OnOffAuto",
> virt_get_acpi, virt_set_acpi,
> NULL, NULL);
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr
2024-04-12 7:33 ` [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr Richard Henderson
@ 2024-06-03 19:14 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:14 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Unify the code blocks that try to print a symbolic address.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 89 +++++++++++++++-------------------------------
> 1 file changed, 29 insertions(+), 60 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index 49a4c0fd40..3473c94164 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -767,6 +767,24 @@ read_insn_microblaze (bfd_vma memaddr,
> return inst;
> }
>
> +static void print_immval_addr(struct disassemble_info *info, bool immfound,
> + int immval, unsigned inst, int addend)
> +{
> + if (info->print_address_func && info->symbol_at_address_func) {
> + if (immfound) {
> + immval |= get_int_field_imm(inst) & 0x0000ffff;
> + } else {
> + immval = (int16_t)get_int_field_imm(inst);
> + }
> + immval += addend;
> + if (immval != 0 && info->symbol_at_address_func(immval, info)) {
> + info->fprintf_func(info->stream, "\t// ");
> + info->print_address_func (immval, info);
> + } else if (addend) {
> + info->fprintf_func(info->stream, "\t// %x", immval);
> + }
> + }
> +}
>
> int
> print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
> @@ -821,18 +839,8 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
> break;
> case INST_TYPE_RD_R1_IMM:
> fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm(inst));
> - if (info->print_address_func && get_int_field_r1(inst) == 0 && info->symbol_at_address_func) {
> - if (immfound)
> - immval |= (get_int_field_imm(inst) & 0x0000ffff);
> - else {
> - immval = get_int_field_imm(inst);
> - if (immval & 0x8000)
> - immval |= 0xFFFF0000;
> - }
> - if (immval > 0 && info->symbol_at_address_func(immval, info)) {
> - fprintf_func (stream, "\t// ");
> - info->print_address_func (immval, info);
> - }
> + if (get_int_field_r1(inst) == 0) {
> + print_immval_addr(info, immfound, immval, inst, 0);
> }
> break;
> case INST_TYPE_RD_R1_IMM5:
> @@ -860,61 +868,22 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
> fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_imm(inst));
> /* The non-pc relative instructions are returns, which shouldn't
> have a label printed */
> - if (info->print_address_func && op->inst_offset_type == INST_PC_OFFSET && info->symbol_at_address_func) {
> - if (immfound)
> - immval |= (get_int_field_imm(inst) & 0x0000ffff);
> - else {
> - immval = get_int_field_imm(inst);
> - if (immval & 0x8000)
> - immval |= 0xFFFF0000;
> - }
> - immval += memaddr;
> - if (immval > 0 && info->symbol_at_address_func(immval, info)) {
> - fprintf_func (stream, "\t// ");
> - info->print_address_func (immval, info);
> - } else {
> - fprintf_func (stream, "\t\t// ");
> - fprintf_func (stream, "%x", immval);
> - }
> + if (op->inst_offset_type == INST_PC_OFFSET) {
> + print_immval_addr(info, immfound, immval, inst, memaddr);
> }
> break;
> case INST_TYPE_RD_IMM:
> fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm(inst));
> - if (info->print_address_func && info->symbol_at_address_func) {
> - if (immfound)
> - immval |= (get_int_field_imm(inst) & 0x0000ffff);
> - else {
> - immval = get_int_field_imm(inst);
> - if (immval & 0x8000)
> - immval |= 0xFFFF0000;
> - }
> - if (op->inst_offset_type == INST_PC_OFFSET)
> - immval += (int) memaddr;
> - if (info->symbol_at_address_func(immval, info)) {
> - fprintf_func (stream, "\t// ");
> - info->print_address_func (immval, info);
> - }
> - }
> + print_immval_addr(info, immfound, immval, inst,
> + op->inst_offset_type == INST_PC_OFFSET
> + ? memaddr : 0);
> break;
> case INST_TYPE_IMM:
> fprintf_func(stream, "\t%s", get_field_imm(inst));
> - if (info->print_address_func && info->symbol_at_address_func && op->instr != imm) {
> - if (immfound)
> - immval |= (get_int_field_imm(inst) & 0x0000ffff);
> - else {
> - immval = get_int_field_imm(inst);
> - if (immval & 0x8000)
> - immval |= 0xFFFF0000;
> - }
> - if (op->inst_offset_type == INST_PC_OFFSET)
> - immval += (int) memaddr;
> - if (immval > 0 && info->symbol_at_address_func(immval, info)) {
> - fprintf_func (stream, "\t// ");
> - info->print_address_func (immval, info);
> - } else if (op->inst_offset_type == INST_PC_OFFSET) {
> - fprintf_func (stream, "\t\t// ");
> - fprintf_func (stream, "%x", immval);
> - }
> + if (op->instr != imm) {
> + print_immval_addr(info, immfound, immval, inst,
> + op->inst_offset_type == INST_PC_OFFSET
> + ? memaddr : 0);
> }
> break;
> case INST_TYPE_RD_R2:
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze
2024-04-12 7:33 ` [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze Richard Henderson
@ 2024-06-03 19:14 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:14 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 263 ++++++++++++++++++++++++---------------------
> 1 file changed, 141 insertions(+), 122 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index 3473c94164..c729c76585 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -787,134 +787,153 @@ static void print_immval_addr(struct disassemble_info *info, bool immfound,
> }
>
> int
> -print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
> +print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> {
> - fprintf_function fprintf_func = info->fprintf_func;
> - void * stream = info->stream;
> - unsigned long inst, prev_inst;
> - const struct op_code_struct *op, *pop;
> - int immval = 0;
> - bfd_boolean immfound = FALSE;
> - static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
> - static int prev_insn_vma = -1; /*init the prev insn vma */
> - int curr_insn_vma = info->buffer_vma;
> + fprintf_function fprintf_func = info->fprintf_func;
> + void *stream = info->stream;
> + unsigned long inst, prev_inst;
> + const struct op_code_struct *op, *pop;
> + int immval = 0;
> + bool immfound = false;
> + static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
> + static int prev_insn_vma = -1; /*init the prev insn vma */
> + int curr_insn_vma = info->buffer_vma;
>
> - info->bytes_per_chunk = 4;
> + info->bytes_per_chunk = 4;
>
> - inst = read_insn_microblaze (memaddr, info, &op);
> - if (inst == 0) {
> - return -1;
> - }
> + inst = read_insn_microblaze (memaddr, info, &op);
> + if (inst == 0) {
> + return -1;
> + }
>
> - if (prev_insn_vma == curr_insn_vma) {
> - if (memaddr-(info->bytes_per_chunk) == prev_insn_addr) {
> - prev_inst = read_insn_microblaze (prev_insn_addr, info, &pop);
> - if (prev_inst == 0)
> - return -1;
> - if (pop->instr == imm) {
> - immval = (get_int_field_imm(prev_inst) << 16) & 0xffff0000;
> - immfound = TRUE;
> + if (prev_insn_vma == curr_insn_vma) {
> + if (memaddr - info->bytes_per_chunk == prev_insn_addr) {
> + prev_inst = read_insn_microblaze (prev_insn_addr, info, &pop);
> + if (prev_inst == 0)
> + return -1;
> + if (pop->instr == imm) {
> + immval = (get_int_field_imm(prev_inst) << 16) & 0xffff0000;
> + immfound = TRUE;
> + }
> + else {
> + immval = 0;
> + immfound = FALSE;
> + }
> + }
> }
> - else {
> - immval = 0;
> - immfound = FALSE;
> - }
> - }
> - }
> - /* make curr insn as prev insn */
> - prev_insn_addr = memaddr;
> - prev_insn_vma = curr_insn_vma;
> + /* make curr insn as prev insn */
> + prev_insn_addr = memaddr;
> + prev_insn_vma = curr_insn_vma;
>
> - if (op->name == 0) {
> - fprintf_func (stream, ".short 0x%04lx", inst);
> - }
> - else
> - {
> - fprintf_func (stream, "%s", op->name);
> + if (op->name == 0) {
> + fprintf_func (stream, ".short 0x%04lx", inst);
> + return 4;
> + }
> +
> + fprintf_func (stream, "%s", op->name);
>
> - switch (op->inst_type)
> - {
> - case INST_TYPE_RD_R1_R2:
> - fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_r2(inst));
> - break;
> - case INST_TYPE_RD_R1_IMM:
> - fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm(inst));
> - if (get_int_field_r1(inst) == 0) {
> - print_immval_addr(info, immfound, immval, inst, 0);
> - }
> - break;
> - case INST_TYPE_RD_R1_IMM5:
> - fprintf_func(stream, "\t%s, %s, %s", get_field_rd(inst), get_field_r1(inst), get_field_imm5(inst));
> - break;
> - case INST_TYPE_RD_RFSL:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_rfsl(inst));
> - break;
> - case INST_TYPE_R1_RFSL:
> - fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_rfsl(inst));
> - break;
> - case INST_TYPE_RD_SPECIAL:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_special(inst, op));
> - break;
> - case INST_TYPE_SPECIAL_R1:
> - fprintf_func(stream, "\t%s, %s", get_field_special(inst, op), get_field_r1(inst));
> - break;
> - case INST_TYPE_RD_R1:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r1(inst));
> - break;
> - case INST_TYPE_R1_R2:
> - fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_r2(inst));
> - break;
> - case INST_TYPE_R1_IMM:
> - fprintf_func(stream, "\t%s, %s", get_field_r1(inst), get_field_imm(inst));
> - /* The non-pc relative instructions are returns, which shouldn't
> - have a label printed */
> - if (op->inst_offset_type == INST_PC_OFFSET) {
> - print_immval_addr(info, immfound, immval, inst, memaddr);
> - }
> - break;
> - case INST_TYPE_RD_IMM:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm(inst));
> - print_immval_addr(info, immfound, immval, inst,
> - op->inst_offset_type == INST_PC_OFFSET
> - ? memaddr : 0);
> - break;
> - case INST_TYPE_IMM:
> - fprintf_func(stream, "\t%s", get_field_imm(inst));
> - if (op->instr != imm) {
> - print_immval_addr(info, immfound, immval, inst,
> - op->inst_offset_type == INST_PC_OFFSET
> - ? memaddr : 0);
> - }
> - break;
> - case INST_TYPE_RD_R2:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r2(inst));
> - break;
> - case INST_TYPE_R2:
> - fprintf_func(stream, "\t%s", get_field_r2(inst));
> - break;
> - case INST_TYPE_R1:
> - fprintf_func(stream, "\t%s", get_field_r1(inst));
> - break;
> - case INST_TYPE_RD_R1_SPECIAL:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_r2(inst));
> - break;
> - case INST_TYPE_RD_IMM15:
> - fprintf_func(stream, "\t%s, %s", get_field_rd(inst), get_field_imm15(inst));
> - break;
> - /* For tuqula instruction */
> - case INST_TYPE_RD:
> - fprintf_func(stream, "\t%s", get_field_rd(inst));
> - break;
> - case INST_TYPE_RFSL:
> - fprintf_func(stream, "\t%s", get_field_rfsl(inst));
> - break;
> - default:
> - /* if the disassembler lags the instruction set */
> - fprintf_func (stream, "\tundecoded operands, inst is 0x%04lx", inst);
> - break;
> - }
> + switch (op->inst_type) {
> + case INST_TYPE_RD_R1_R2:
> + fprintf_func(stream, "\t%s, %s, %s",
> + get_field_rd(inst), get_field_r1(inst),
> + get_field_r2(inst));
> + break;
> + case INST_TYPE_RD_R1_IMM:
> + fprintf_func(stream, "\t%s, %s, %s",
> + get_field_rd(inst), get_field_r1(inst),
> + get_field_imm(inst));
> + if (get_int_field_r1(inst) == 0) {
> + print_immval_addr(info, immfound, immval, inst, 0);
> + }
> + break;
> + case INST_TYPE_RD_R1_IMM5:
> + fprintf_func(stream, "\t%s, %s, %s",
> + get_field_rd(inst), get_field_r1(inst),
> + get_field_imm5(inst));
> + break;
> + case INST_TYPE_RD_RFSL:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_rfsl(inst));
> + break;
> + case INST_TYPE_R1_RFSL:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_r1(inst), get_field_rfsl(inst));
> + break;
> + case INST_TYPE_RD_SPECIAL:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_special(inst, op));
> + break;
> + case INST_TYPE_SPECIAL_R1:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_special(inst, op), get_field_r1(inst));
> + break;
> + case INST_TYPE_RD_R1:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_r1(inst));
> + break;
> + case INST_TYPE_R1_R2:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_r1(inst), get_field_r2(inst));
> + break;
> + case INST_TYPE_R1_IMM:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_r1(inst), get_field_imm(inst));
> + /*
> + * The non-pc relative instructions are returns,
> + * which shouldn't have a label printed.
> + */
> + if (op->inst_offset_type == INST_PC_OFFSET) {
> + print_immval_addr(info, immfound, immval, inst, memaddr);
> + }
> + break;
> + case INST_TYPE_RD_IMM:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_imm(inst));
> + print_immval_addr(info, immfound, immval, inst,
> + op->inst_offset_type == INST_PC_OFFSET
> + ? memaddr : 0);
> + break;
> + case INST_TYPE_IMM:
> + fprintf_func(stream, "\t%s", get_field_imm(inst));
> + if (op->instr != imm) {
> + print_immval_addr(info, immfound, immval, inst,
> + op->inst_offset_type == INST_PC_OFFSET
> + ? memaddr : 0);
> + }
> + break;
> + case INST_TYPE_RD_R2:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_r2(inst));
> + break;
> + case INST_TYPE_R2:
> + fprintf_func(stream, "\t%s",
> + get_field_r2(inst));
> + break;
> + case INST_TYPE_R1:
> + fprintf_func(stream, "\t%s",
> + get_field_r1(inst));
> + break;
> + case INST_TYPE_RD_R1_SPECIAL:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_r2(inst));
> + break;
> + case INST_TYPE_RD_IMM15:
> + fprintf_func(stream, "\t%s, %s",
> + get_field_rd(inst), get_field_imm15(inst));
> + break;
> + /* For tuqula instruction */
> + case INST_TYPE_RD:
> + fprintf_func(stream, "\t%s",
> + get_field_rd(inst));
> + break;
> + case INST_TYPE_RFSL:
> + fprintf_func(stream, "\t%s",
> + get_field_rfsl(inst));
> + break;
> + default:
> + /* if the disassembler lags the instruction set */
> + fprintf_func(stream, "\tundecoded operands, inst is 0x%04lx", inst);
> + break;
> }
> -
> - /* Say how many bytes we consumed? */
> - return 4;
> + return 4;
> }
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf
2024-04-12 7:33 ` [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf Richard Henderson
@ 2024-06-03 19:15 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:15 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> In the common case, issue one single fprintf.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 80 +++++++++++++++++++++++-----------------------
> 1 file changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index c729c76585..a537ac65dd 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -830,54 +830,52 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> return 4;
> }
>
> - fprintf_func (stream, "%s", op->name);
> -
> switch (op->inst_type) {
> case INST_TYPE_RD_R1_R2:
> - fprintf_func(stream, "\t%s, %s, %s",
> - get_field_rd(inst), get_field_r1(inst),
> + fprintf_func(stream, "%s\t%s, %s, %s",
> + op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_r2(inst));
> break;
> case INST_TYPE_RD_R1_IMM:
> - fprintf_func(stream, "\t%s, %s, %s",
> - get_field_rd(inst), get_field_r1(inst),
> + fprintf_func(stream, "%s\t%s, %s, %s",
> + op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm(inst));
> if (get_int_field_r1(inst) == 0) {
> print_immval_addr(info, immfound, immval, inst, 0);
> }
> break;
> case INST_TYPE_RD_R1_IMM5:
> - fprintf_func(stream, "\t%s, %s, %s",
> - get_field_rd(inst), get_field_r1(inst),
> + fprintf_func(stream, "%s\t%s, %s, %s",
> + op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm5(inst));
> break;
> case INST_TYPE_RD_RFSL:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_rfsl(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_R1_RFSL:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_r1(inst), get_field_rfsl(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_r1(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_RD_SPECIAL:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_special(inst, op));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_special(inst, op));
> break;
> case INST_TYPE_SPECIAL_R1:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_special(inst, op), get_field_r1(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_special(inst, op), get_field_r1(inst));
> break;
> case INST_TYPE_RD_R1:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_r1(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_r1(inst));
> break;
> case INST_TYPE_R1_R2:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_r1(inst), get_field_r2(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_r1(inst), get_field_r2(inst));
> break;
> case INST_TYPE_R1_IMM:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_r1(inst), get_field_imm(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_r1(inst), get_field_imm(inst));
> /*
> * The non-pc relative instructions are returns,
> * which shouldn't have a label printed.
> @@ -887,14 +885,15 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_IMM:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_imm(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_imm(inst));
> print_immval_addr(info, immfound, immval, inst,
> op->inst_offset_type == INST_PC_OFFSET
> ? memaddr : 0);
> break;
> case INST_TYPE_IMM:
> - fprintf_func(stream, "\t%s", get_field_imm(inst));
> + fprintf_func(stream, "%s\t%s",
> + op->name, get_field_imm(inst));
> if (op->instr != imm) {
> print_immval_addr(info, immfound, immval, inst,
> op->inst_offset_type == INST_PC_OFFSET
> @@ -902,37 +901,38 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_R2:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_r2(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_r2(inst));
> break;
> case INST_TYPE_R2:
> - fprintf_func(stream, "\t%s",
> - get_field_r2(inst));
> + fprintf_func(stream, "%s\t%s",
> + op->name, get_field_r2(inst));
> break;
> case INST_TYPE_R1:
> - fprintf_func(stream, "\t%s",
> - get_field_r1(inst));
> + fprintf_func(stream, "%s\t%s",
> + op->name, get_field_r1(inst));
> break;
> case INST_TYPE_RD_R1_SPECIAL:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_r2(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_r2(inst));
> break;
> case INST_TYPE_RD_IMM15:
> - fprintf_func(stream, "\t%s, %s",
> - get_field_rd(inst), get_field_imm15(inst));
> + fprintf_func(stream, "%s\t%s, %s",
> + op->name, get_field_rd(inst), get_field_imm15(inst));
> break;
> /* For tuqula instruction */
> case INST_TYPE_RD:
> - fprintf_func(stream, "\t%s",
> - get_field_rd(inst));
> + fprintf_func(stream, "%s\t%s",
> + op->name, get_field_rd(inst));
> break;
> case INST_TYPE_RFSL:
> - fprintf_func(stream, "\t%s",
> - get_field_rfsl(inst));
> + fprintf_func(stream, "%s\t%s",
> + op->name, get_field_rfsl(inst));
> break;
> default:
> /* if the disassembler lags the instruction set */
> - fprintf_func(stream, "\tundecoded operands, inst is 0x%04lx", inst);
> + fprintf_func(stream, "%s\tundecoded operands, inst is 0x%04lx",
> + op->name, inst);
> break;
> }
> return 4;
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg
2024-04-12 7:33 ` [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg Richard Henderson
@ 2024-06-03 19:17 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:17 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Use a printf format instead of sprintf into a buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 54 ++++++++++++++++++++--------------------------
> 1 file changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index a537ac65dd..c12968f3b9 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -563,7 +563,7 @@ static const struct op_code_struct {
> };
>
> /* prefix for register names */
> -static const char register_prefix[] = "r";
> +#define register_prefix "r"
> static const char fsl_register_prefix[] = "rfsl";
> static const char pvr_register_prefix[] = "rpvr";
>
> @@ -579,15 +579,16 @@ static const char pvr_register_prefix[] = "rpvr";
>
> #include "disas/dis-asm.h"
>
> -#define get_field_rd(instr) get_field(instr, RD_MASK, RD_LOW)
> -#define get_field_r1(instr) get_field(instr, RA_MASK, RA_LOW)
> -#define get_field_r2(instr) get_field(instr, RB_MASK, RB_LOW)
> +#define PRIreg register_prefix "%ld"
> +
> +#define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
> +#define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
> +#define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
> #define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
> #define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
>
> /* Local function prototypes. */
>
> -static char * get_field (long instr, long mask, unsigned short low);
> static char * get_field_imm (long instr);
> static char * get_field_imm5 (long instr);
> static char * get_field_rfsl (long instr);
> @@ -596,15 +597,6 @@ static char * get_field_imm15 (long instr);
> static char * get_field_unsigned_imm (long instr);
> #endif
>
> -static char *
> -get_field (long instr, long mask, unsigned short low)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%s%d", register_prefix,
> - (int)((instr & mask) >> low));
> - return(strdup(tmpstr));
> -}
> -
> static char *
> get_field_imm (long instr)
> {
> @@ -832,12 +824,12 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
>
> switch (op->inst_type) {
> case INST_TYPE_RD_R1_R2:
> - fprintf_func(stream, "%s\t%s, %s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIreg,
> op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_r2(inst));
> break;
> case INST_TYPE_RD_R1_IMM:
> - fprintf_func(stream, "%s\t%s, %s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm(inst));
> if (get_int_field_r1(inst) == 0) {
> @@ -845,36 +837,36 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_R1_IMM5:
> - fprintf_func(stream, "%s\t%s, %s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm5(inst));
> break;
> case INST_TYPE_RD_RFSL:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_R1_RFSL:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_r1(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_RD_SPECIAL:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_special(inst, op));
> break;
> case INST_TYPE_SPECIAL_R1:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t%s, " PRIreg,
> op->name, get_field_special(inst, op), get_field_r1(inst));
> break;
> case INST_TYPE_RD_R1:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
> op->name, get_field_rd(inst), get_field_r1(inst));
> break;
> case INST_TYPE_R1_R2:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
> op->name, get_field_r1(inst), get_field_r2(inst));
> break;
> case INST_TYPE_R1_IMM:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_r1(inst), get_field_imm(inst));
> /*
> * The non-pc relative instructions are returns,
> @@ -885,7 +877,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_IMM:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_imm(inst));
> print_immval_addr(info, immfound, immval, inst,
> op->inst_offset_type == INST_PC_OFFSET
> @@ -901,28 +893,28 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_R2:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
> op->name, get_field_rd(inst), get_field_r2(inst));
> break;
> case INST_TYPE_R2:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIreg,
> op->name, get_field_r2(inst));
> break;
> case INST_TYPE_R1:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIreg,
> op->name, get_field_r1(inst));
> break;
> case INST_TYPE_RD_R1_SPECIAL:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
> op->name, get_field_rd(inst), get_field_r2(inst));
> break;
> case INST_TYPE_RD_IMM15:
> - fprintf_func(stream, "%s\t%s, %s",
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> op->name, get_field_rd(inst), get_field_imm15(inst));
> break;
> /* For tuqula instruction */
> case INST_TYPE_RD:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIreg,
> op->name, get_field_rd(inst));
> break;
> case INST_TYPE_RFSL:
I like PRIreg definition :)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm
2024-04-12 7:33 ` [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm Richard Henderson
@ 2024-06-03 19:17 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:17 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Use a printf format instead of sprintf into a buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 61 +++++++++-------------------------------------
> 1 file changed, 11 insertions(+), 50 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index c12968f3b9..390f98c0a3 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -580,40 +580,21 @@ static const char pvr_register_prefix[] = "rpvr";
> #include "disas/dis-asm.h"
>
> #define PRIreg register_prefix "%ld"
> +#define PRIimm "%d"
>
> #define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
> #define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
> #define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
> +#define get_field_imm(instr) ((int16_t)instr)
> +#define get_field_imm5(instr) ((int)instr & IMM5_MASK)
> +#define get_field_imm15(instr) ((int)instr & IMM15_MASK)
> +
> #define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
> #define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
>
> /* Local function prototypes. */
>
> -static char * get_field_imm (long instr);
> -static char * get_field_imm5 (long instr);
> static char * get_field_rfsl (long instr);
> -static char * get_field_imm15 (long instr);
> -#if 0
> -static char * get_field_unsigned_imm (long instr);
> -#endif
> -
> -static char *
> -get_field_imm (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%d",
> - (short)((instr & IMM_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
> -
> -static char *
> -get_field_imm5 (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%d",
> - (short)((instr & IMM5_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
>
> static char *
> get_field_rfsl (long instr)
> @@ -624,26 +605,6 @@ get_field_rfsl (long instr)
> return(strdup(tmpstr));
> }
>
> -static char *
> -get_field_imm15 (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%d",
> - (short)((instr & IMM15_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
> -
> -#if 0
> -static char *
> -get_field_unsigned_imm (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%d",
> - (int)((instr & IMM_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
> -#endif
> -
> /*
> char *
> get_field_special (instr)
> @@ -829,7 +790,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> get_field_r2(inst));
> break;
> case INST_TYPE_RD_R1_IMM:
> - fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIimm,
> op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm(inst));
> if (get_int_field_r1(inst) == 0) {
> @@ -837,7 +798,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_R1_IMM5:
> - fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIreg ", " PRIimm,
> op->name, get_field_rd(inst), get_field_r1(inst),
> get_field_imm5(inst));
> break;
> @@ -866,7 +827,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> op->name, get_field_r1(inst), get_field_r2(inst));
> break;
> case INST_TYPE_R1_IMM:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
> op->name, get_field_r1(inst), get_field_imm(inst));
> /*
> * The non-pc relative instructions are returns,
> @@ -877,14 +838,14 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> }
> break;
> case INST_TYPE_RD_IMM:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
> op->name, get_field_rd(inst), get_field_imm(inst));
> print_immval_addr(info, immfound, immval, inst,
> op->inst_offset_type == INST_PC_OFFSET
> ? memaddr : 0);
> break;
> case INST_TYPE_IMM:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIimm,
> op->name, get_field_imm(inst));
> if (op->instr != imm) {
> print_immval_addr(info, immfound, immval, inst,
> @@ -909,7 +870,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> op->name, get_field_rd(inst), get_field_r2(inst));
> break;
> case INST_TYPE_RD_IMM15:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIimm,
> op->name, get_field_rd(inst), get_field_imm15(inst));
> break;
> /* For tuqula instruction */
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl
2024-04-12 7:33 ` [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl Richard Henderson
@ 2024-06-03 19:17 ` Pierrick Bouvier
2024-06-03 19:18 ` Pierrick Bouvier
1 sibling, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:17 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Use a printf format instead of sprintf into a buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 22 +++++-----------------
> 1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index 390f98c0a3..24febfdea9 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -564,7 +564,6 @@ static const struct op_code_struct {
>
> /* prefix for register names */
> #define register_prefix "r"
> -static const char fsl_register_prefix[] = "rfsl";
> static const char pvr_register_prefix[] = "rpvr";
>
>
> @@ -580,11 +579,13 @@ static const char pvr_register_prefix[] = "rpvr";
> #include "disas/dis-asm.h"
>
> #define PRIreg register_prefix "%ld"
> +#define PRIrfsl register_prefix "fsl%ld"
> #define PRIimm "%d"
>
> #define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
> #define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
> #define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
> +#define get_field_rfsl(instr) (instr & RFSL_MASK)
> #define get_field_imm(instr) ((int16_t)instr)
> #define get_field_imm5(instr) ((int)instr & IMM5_MASK)
> #define get_field_imm15(instr) ((int)instr & IMM15_MASK)
> @@ -592,19 +593,6 @@ static const char pvr_register_prefix[] = "rpvr";
> #define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
> #define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
>
> -/* Local function prototypes. */
> -
> -static char * get_field_rfsl (long instr);
> -
> -static char *
> -get_field_rfsl (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%s%d", fsl_register_prefix,
> - (short)((instr & RFSL_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
> -
> /*
> char *
> get_field_special (instr)
> @@ -803,11 +791,11 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> get_field_imm5(inst));
> break;
> case INST_TYPE_RD_RFSL:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
> op->name, get_field_rd(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_R1_RFSL:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
> op->name, get_field_r1(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_RD_SPECIAL:
> @@ -879,7 +867,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> op->name, get_field_rd(inst));
> break;
> case INST_TYPE_RFSL:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIrfsl,
> op->name, get_field_rfsl(inst));
> break;
> default:
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl
2024-04-12 7:33 ` [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
@ 2024-06-03 19:18 ` Pierrick Bouvier
1 sibling, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:18 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Use a printf format instead of sprintf into a buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 22 +++++-----------------
> 1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index 390f98c0a3..24febfdea9 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -564,7 +564,6 @@ static const struct op_code_struct {
>
> /* prefix for register names */
> #define register_prefix "r"
> -static const char fsl_register_prefix[] = "rfsl";
> static const char pvr_register_prefix[] = "rpvr";
>
>
> @@ -580,11 +579,13 @@ static const char pvr_register_prefix[] = "rpvr";
> #include "disas/dis-asm.h"
>
> #define PRIreg register_prefix "%ld"
> +#define PRIrfsl register_prefix "fsl%ld"
> #define PRIimm "%d"
>
> #define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
> #define get_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
> #define get_field_r2(instr) ((instr & RB_MASK) >> RB_LOW)
> +#define get_field_rfsl(instr) (instr & RFSL_MASK)
> #define get_field_imm(instr) ((int16_t)instr)
> #define get_field_imm5(instr) ((int)instr & IMM5_MASK)
> #define get_field_imm15(instr) ((int)instr & IMM15_MASK)
> @@ -592,19 +593,6 @@ static const char pvr_register_prefix[] = "rpvr";
> #define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
> #define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
>
> -/* Local function prototypes. */
> -
> -static char * get_field_rfsl (long instr);
> -
> -static char *
> -get_field_rfsl (long instr)
> -{
> - char tmpstr[25];
> - snprintf(tmpstr, sizeof(tmpstr), "%s%d", fsl_register_prefix,
> - (short)((instr & RFSL_MASK) >> IMM_LOW));
> - return(strdup(tmpstr));
> -}
> -
> /*
> char *
> get_field_special (instr)
> @@ -803,11 +791,11 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> get_field_imm5(inst));
> break;
> case INST_TYPE_RD_RFSL:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
> op->name, get_field_rd(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_R1_RFSL:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIrfsl,
> op->name, get_field_r1(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_RD_SPECIAL:
> @@ -879,7 +867,7 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> op->name, get_field_rd(inst));
> break;
> case INST_TYPE_RFSL:
> - fprintf_func(stream, "%s\t%s",
> + fprintf_func(stream, "%s\t" PRIrfsl,
> op->name, get_field_rfsl(inst));
> break;
> default:
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 20/27] disas/microblaze: Split get_field_special
2024-04-12 7:33 ` [PATCH v3 20/27] disas/microblaze: Split get_field_special Richard Henderson
@ 2024-06-03 19:20 ` Pierrick Bouvier
0 siblings, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:20 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Extract the raw special index and a function to lookup a name.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/microblaze.c | 142 +++++++++++++++++++--------------------------
> 1 file changed, 61 insertions(+), 81 deletions(-)
>
> diff --git a/disas/microblaze.c b/disas/microblaze.c
> index 24febfdea9..197327fae4 100644
> --- a/disas/microblaze.c
> +++ b/disas/microblaze.c
> @@ -564,8 +564,6 @@ static const struct op_code_struct {
>
> /* prefix for register names */
> #define register_prefix "r"
> -static const char pvr_register_prefix[] = "rpvr";
> -
>
> /* #defines for valid immediate range */
> #define MIN_IMM ((int) 0x80000000)
> @@ -580,6 +578,7 @@ static const char pvr_register_prefix[] = "rpvr";
>
> #define PRIreg register_prefix "%ld"
> #define PRIrfsl register_prefix "fsl%ld"
> +#define PRIpvr register_prefix "pvr%d"
> #define PRIimm "%d"
>
> #define get_field_rd(instr) ((instr & RD_MASK) >> RD_LOW)
> @@ -593,83 +592,48 @@ static const char pvr_register_prefix[] = "rpvr";
> #define get_int_field_imm(instr) ((instr & IMM_MASK) >> IMM_LOW)
> #define get_int_field_r1(instr) ((instr & RA_MASK) >> RA_LOW)
>
> -/*
> - char *
> - get_field_special (instr)
> - long instr;
> - {
> - char tmpstr[25];
> -
> - snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix,
> - (((instr & IMM_MASK) >> IMM_LOW) & REG_MSR_MASK) == 0 ? "pc" : "msr");
> -
> - return(strdup(tmpstr));
> - }
> -*/
> -
> -static char *
> -get_field_special(long instr, const struct op_code_struct *op)
> +static int get_field_special(long instr, const struct op_code_struct *op)
> {
> - char tmpstr[25];
> - char spr[6];
> + return ((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask;
> +}
>
> - switch ( (((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask) ) {
> -
> - case REG_MSR_MASK :
> - strcpy(spr, "msr");
> - break;
> - case REG_PC_MASK :
> - strcpy(spr, "pc");
> - break;
> - case REG_EAR_MASK :
> - strcpy(spr, "ear");
> - break;
> - case REG_ESR_MASK :
> - strcpy(spr, "esr");
> - break;
> - case REG_FSR_MASK :
> - strcpy(spr, "fsr");
> - break;
> - case REG_BTR_MASK :
> - strcpy(spr, "btr");
> - break;
> - case REG_EDR_MASK :
> - strcpy(spr, "edr");
> - break;
> - case REG_PID_MASK :
> - strcpy(spr, "pid");
> - break;
> - case REG_ZPR_MASK :
> - strcpy(spr, "zpr");
> - break;
> - case REG_TLBX_MASK :
> - strcpy(spr, "tlbx");
> - break;
> - case REG_TLBLO_MASK :
> - strcpy(spr, "tlblo");
> - break;
> - case REG_TLBHI_MASK :
> - strcpy(spr, "tlbhi");
> - break;
> - case REG_TLBSX_MASK :
> - strcpy(spr, "tlbsx");
> - break;
> - default :
> - {
> - if ( ((((instr & IMM_MASK) >> IMM_LOW) ^ op->immval_mask) & 0xE000) == REG_PVR_MASK) {
> - snprintf(tmpstr, sizeof(tmpstr), "%s%u", pvr_register_prefix,
> - (unsigned short)(((instr & IMM_MASK) >> IMM_LOW) ^
> - op->immval_mask) ^ REG_PVR_MASK);
> - return(strdup(tmpstr));
> - } else {
> - strcpy(spr, "pc");
> - }
> - }
> - break;
> - }
> -
> - snprintf(tmpstr, sizeof(tmpstr), "%s%s", register_prefix, spr);
> - return(strdup(tmpstr));
> +/* Returns NULL for PVR registers, which should be rendered differently. */
> +static const char *get_special_name(int special)
> +{
> + switch (special) {
> + case REG_MSR_MASK:
> + return register_prefix "msr";
> + case REG_PC_MASK:
> + return register_prefix "pc";
> + case REG_EAR_MASK:
> + return register_prefix "ear";
> + case REG_ESR_MASK:
> + return register_prefix "esr";
> + case REG_FSR_MASK:
> + return register_prefix "fsr";
> + case REG_BTR_MASK:
> + return register_prefix "btr";
> + case REG_EDR_MASK:
> + return register_prefix "edr";
> + case REG_PID_MASK:
> + return register_prefix "pid";
> + case REG_ZPR_MASK:
> + return register_prefix "zpr";
> + case REG_TLBX_MASK:
> + return register_prefix "tlbx";
> + case REG_TLBLO_MASK:
> + return register_prefix "tlblo";
> + case REG_TLBHI_MASK:
> + return register_prefix "tlbhi";
> + case REG_TLBSX_MASK:
> + return register_prefix "tlbsx";
> + default:
> + if ((special & 0xE000) == REG_PVR_MASK) {
> + /* pvr register */
> + return NULL;
> + }
> + return register_prefix "pc";
> + }
> }
>
> static unsigned long
> @@ -739,6 +703,8 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> static bfd_vma prev_insn_addr = -1; /*init the prev insn addr */
> static int prev_insn_vma = -1; /*init the prev insn vma */
> int curr_insn_vma = info->buffer_vma;
> + int special;
> + const char *special_name;
>
> info->bytes_per_chunk = 4;
>
> @@ -799,12 +765,26 @@ print_insn_microblaze(bfd_vma memaddr, struct disassemble_info *info)
> op->name, get_field_r1(inst), get_field_rfsl(inst));
> break;
> case INST_TYPE_RD_SPECIAL:
> - fprintf_func(stream, "%s\t" PRIreg ", %s",
> - op->name, get_field_rd(inst), get_field_special(inst, op));
> + special = get_field_special(inst, op);
> + special_name = get_special_name(special);
> + if (special_name) {
> + fprintf_func(stream, "%s\t" PRIreg ", %s",
> + op->name, get_field_rd(inst), special_name);
> + } else {
> + fprintf_func(stream, "%s\t" PRIreg ", " PRIpvr,
> + op->name, get_field_rd(inst), special ^ REG_PVR_MASK);
> + }
> break;
> case INST_TYPE_SPECIAL_R1:
> - fprintf_func(stream, "%s\t%s, " PRIreg,
> - op->name, get_field_special(inst, op), get_field_r1(inst));
> + special = get_field_special(inst, op);
> + special_name = get_special_name(special);
> + if (special_name) {
> + fprintf_func(stream, "%s\t%s, " PRIreg,
> + op->name, special_name, get_field_r1(inst));
> + } else {
> + fprintf_func(stream, "%s\t" PRIpvr ", " PRIreg,
> + op->name, special ^ REG_PVR_MASK, get_field_r1(inst));
> + }
> break;
> case INST_TYPE_RD_R1:
> fprintf_func(stream, "%s\t" PRIreg ", " PRIreg,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 21/27] disas/riscv: Use GString in format_inst
2024-04-12 7:33 ` [PATCH v3 21/27] disas/riscv: Use GString in format_inst Richard Henderson
@ 2024-06-03 19:24 ` Pierrick Bouvier
2024-06-04 0:15 ` Alistair Francis
1 sibling, 0 replies; 63+ messages in thread
From: Pierrick Bouvier @ 2024-06-03 19:24 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 4/12/24 00:33, Richard Henderson wrote:
> Allocate and fill a GString instead of snprintf and
> appending to a fixed sized buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/riscv.c | 209 ++++++++++++++++++++++----------------------------
> 1 file changed, 92 insertions(+), 117 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index e236c8b5b7..f60f9bda43 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -4757,272 +4757,249 @@ static size_t inst_length(rv_inst inst)
>
> /* format instruction */
>
> -static void append(char *s1, const char *s2, size_t n)
> -{
> - size_t l1 = strlen(s1);
> - if (n - l1 - 1 > 0) {
> - strncat(s1, s2, n - l1);
> - }
> -}
> -
> -static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
> +static GString *format_inst(size_t tab, rv_decode *dec)
> {
> const rv_opcode_data *opcode_data = dec->opcode_data;
> - char tmp[64];
> + GString *buf = g_string_sized_new(64);
> const char *fmt;
>
> fmt = opcode_data[dec->op].format;
> while (*fmt) {
> switch (*fmt) {
> case 'O':
> - append(buf, opcode_data[dec->op].name, buflen);
> + g_string_append(buf, opcode_data[dec->op].name);
> break;
> case '(':
> - append(buf, "(", buflen);
> - break;
> case ',':
> - append(buf, ",", buflen);
> - break;
> case ')':
> - append(buf, ")", buflen);
> - break;
> case '-':
> - append(buf, "-", buflen);
> + g_string_append_c(buf, *fmt);
> break;
> case 'b':
> - snprintf(tmp, sizeof(tmp), "%d", dec->bs);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->bs);
> break;
> case 'n':
> - snprintf(tmp, sizeof(tmp), "%d", dec->rnum);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->rnum);
> break;
> case '0':
> - append(buf, rv_ireg_name_sym[dec->rd], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rd]);
> break;
> case '1':
> - append(buf, rv_ireg_name_sym[dec->rs1], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
> break;
> case '2':
> - append(buf, rv_ireg_name_sym[dec->rs2], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
> break;
> case '3':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rd] :
> - rv_freg_name_sym[dec->rd],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rd]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rd]);
> + }
> break;
> case '4':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs1] :
> - rv_freg_name_sym[dec->rs1],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs1]);
> + }
> break;
> case '5':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs2] :
> - rv_freg_name_sym[dec->rs2],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs2]);
> + }
> break;
> case '6':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs3] :
> - rv_freg_name_sym[dec->rs3],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs3]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs3]);
> + }
> break;
> case '7':
> - snprintf(tmp, sizeof(tmp), "%d", dec->rs1);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->rs1);
> break;
> case 'i':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm);
> break;
> case 'u':
> - snprintf(tmp, sizeof(tmp), "%u", ((uint32_t)dec->imm & 0b111111));
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%u", ((uint32_t)dec->imm & 0b111111));
> break;
> case 'j':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm1);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm1);
> break;
> case 'o':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm);
> - append(buf, tmp, buflen);
> - while (strlen(buf) < tab * 2) {
> - append(buf, " ", buflen);
> + g_string_append_printf(buf, "%d", dec->imm);
> + while (buf->len < tab * 2) {
> + g_string_append_c(buf, ' ');
> }
> - snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
> - dec->pc + dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
> break;
> case 'U':
> fmt++;
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm >> 12);
> if (*fmt == 'o') {
> - while (strlen(buf) < tab * 2) {
> - append(buf, " ", buflen);
> + while (buf->len < tab * 2) {
> + g_string_append_c(buf, ' ');
> }
> - snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
> - dec->pc + dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
> }
> break;
> case 'c': {
> const char *name = csr_name(dec->imm & 0xfff);
> if (name) {
> - append(buf, name, buflen);
> + g_string_append(buf, name);
> } else {
> - snprintf(tmp, sizeof(tmp), "0x%03x", dec->imm & 0xfff);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "0x%03x", dec->imm & 0xfff);
> }
> break;
> }
> case 'r':
> switch (dec->rm) {
> case rv_rm_rne:
> - append(buf, "rne", buflen);
> + g_string_append(buf, "rne");
> break;
> case rv_rm_rtz:
> - append(buf, "rtz", buflen);
> + g_string_append(buf, "rtz");
> break;
> case rv_rm_rdn:
> - append(buf, "rdn", buflen);
> + g_string_append(buf, "rdn");
> break;
> case rv_rm_rup:
> - append(buf, "rup", buflen);
> + g_string_append(buf, "rup");
> break;
> case rv_rm_rmm:
> - append(buf, "rmm", buflen);
> + g_string_append(buf, "rmm");
> break;
> case rv_rm_dyn:
> - append(buf, "dyn", buflen);
> + g_string_append(buf, "dyn");
> break;
> default:
> - append(buf, "inv", buflen);
> + g_string_append(buf, "inv");
> break;
> }
> break;
> case 'p':
> if (dec->pred & rv_fence_i) {
> - append(buf, "i", buflen);
> + g_string_append_c(buf, 'i');
> }
> if (dec->pred & rv_fence_o) {
> - append(buf, "o", buflen);
> + g_string_append_c(buf, 'o');
> }
> if (dec->pred & rv_fence_r) {
> - append(buf, "r", buflen);
> + g_string_append_c(buf, 'r');
> }
> if (dec->pred & rv_fence_w) {
> - append(buf, "w", buflen);
> + g_string_append_c(buf, 'w');
> }
> break;
> case 's':
> if (dec->succ & rv_fence_i) {
> - append(buf, "i", buflen);
> + g_string_append_c(buf, 'i');
> }
> if (dec->succ & rv_fence_o) {
> - append(buf, "o", buflen);
> + g_string_append_c(buf, 'o');
> }
> if (dec->succ & rv_fence_r) {
> - append(buf, "r", buflen);
> + g_string_append_c(buf, 'r');
> }
> if (dec->succ & rv_fence_w) {
> - append(buf, "w", buflen);
> + g_string_append_c(buf, 'w');
> }
> break;
> case '\t':
> - while (strlen(buf) < tab) {
> - append(buf, " ", buflen);
> + while (buf->len < tab) {
> + g_string_append_c(buf, ' ');
> }
> break;
> case 'A':
> if (dec->aq) {
> - append(buf, ".aq", buflen);
> + g_string_append(buf, ".aq");
> }
> break;
> case 'R':
> if (dec->rl) {
> - append(buf, ".rl", buflen);
> + g_string_append(buf, ".rl");
> }
> break;
> case 'l':
> - append(buf, ",v0", buflen);
> + g_string_append(buf, ",v0");
> break;
> case 'm':
> if (dec->vm == 0) {
> - append(buf, ",v0.t", buflen);
> + g_string_append(buf, ",v0.t");
> }
> break;
> case 'D':
> - append(buf, rv_vreg_name_sym[dec->rd], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rd]);
> break;
> case 'E':
> - append(buf, rv_vreg_name_sym[dec->rs1], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs1]);
> break;
> case 'F':
> - append(buf, rv_vreg_name_sym[dec->rs2], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs2]);
> break;
> case 'G':
> - append(buf, rv_vreg_name_sym[dec->rs3], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs3]);
> break;
> case 'v': {
> - char nbuf[32] = {0};
> const int sew = 1 << (((dec->vzimm >> 3) & 0b111) + 3);
> - sprintf(nbuf, "%d", sew);
> const int lmul = dec->vzimm & 0b11;
> const int flmul = (dec->vzimm >> 2) & 1;
> const char *vta = (dec->vzimm >> 6) & 1 ? "ta" : "tu";
> const char *vma = (dec->vzimm >> 7) & 1 ? "ma" : "mu";
> - append(buf, "e", buflen);
> - append(buf, nbuf, buflen);
> - append(buf, ",m", buflen);
> +
> + g_string_append_printf(buf, "e%d,m", sew);
> if (flmul) {
> switch (lmul) {
> case 3:
> - sprintf(nbuf, "f2");
> + g_string_append(buf, "f2");
> break;
> case 2:
> - sprintf(nbuf, "f4");
> + g_string_append(buf, "f4");
> break;
> case 1:
> - sprintf(nbuf, "f8");
> - break;
> + g_string_append(buf, "f8");
> + break;
> }
> - append(buf, nbuf, buflen);
> } else {
> - sprintf(nbuf, "%d", 1 << lmul);
> - append(buf, nbuf, buflen);
> + g_string_append_printf(buf, "%d", 1 << lmul);
> }
> - append(buf, ",", buflen);
> - append(buf, vta, buflen);
> - append(buf, ",", buflen);
> - append(buf, vma, buflen);
> + g_string_append_c(buf, ',');
> + g_string_append(buf, vta);
> + g_string_append_c(buf, ',');
> + g_string_append(buf, vma);
> break;
> }
> case 'x': {
> switch (dec->rlist) {
> case 4:
> - snprintf(tmp, sizeof(tmp), "{ra}");
> + g_string_append(buf, "{ra}");
> break;
> case 5:
> - snprintf(tmp, sizeof(tmp), "{ra, s0}");
> + g_string_append(buf, "{ra, s0}");
> break;
> case 15:
> - snprintf(tmp, sizeof(tmp), "{ra, s0-s11}");
> + g_string_append(buf, "{ra, s0-s11}");
> break;
> default:
> - snprintf(tmp, sizeof(tmp), "{ra, s0-s%d}", dec->rlist - 5);
> + g_string_append_printf(buf, "{ra, s0-s%d}", dec->rlist - 5);
> break;
> }
> - append(buf, tmp, buflen);
> break;
> }
> case 'h':
> - append(buf, rv_fli_name_const[dec->imm], buflen);
> + g_string_append(buf, rv_fli_name_const[dec->imm]);
> break;
> default:
> break;
> }
> fmt++;
> }
> +
> + return buf;
> }
>
> /* lift instruction to pseudo-instruction */
> @@ -5108,9 +5085,8 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
>
> /* disassemble instruction */
>
> -static void
> -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> - RISCVCPUConfig *cfg)
> +static GString *disasm_inst(rv_isa isa, uint64_t pc, rv_inst inst,
> + RISCVCPUConfig *cfg)
> {
> rv_decode dec = { 0 };
> dec.pc = pc;
> @@ -5157,7 +5133,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> decode_inst_operands(&dec, isa);
> decode_inst_decompress(&dec, isa);
> decode_inst_lift_pseudo(&dec);
> - format_inst(buf, buflen, 24, &dec);
> + return format_inst(24, &dec);
> }
>
> #define INST_FMT_2 "%04" PRIx64 " "
> @@ -5168,7 +5144,6 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> static int
> print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
> {
> - char buf[128] = { 0 };
> bfd_byte packet[2];
> rv_inst inst = 0;
> size_t len = 2;
> @@ -5209,9 +5184,9 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
> }
> }
>
> - disasm_inst(buf, sizeof(buf), isa, memaddr, inst,
> - (RISCVCPUConfig *)info->target_info);
> - (*info->fprintf_func)(info->stream, "%s", buf);
> + g_autoptr(GString) str =
> + disasm_inst(isa, memaddr, inst, (RISCVCPUConfig *)info->target_info);
> + (*info->fprintf_func)(info->stream, "%s", str->str);
>
> return len;
> }
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
2024-04-12 10:00 ` Philippe Mathieu-Daudé
@ 2024-06-03 19:40 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-03 19:40 UTC (permalink / raw
To: Richard Henderson, qemu-devel
On 12/4/24 09:33, Richard Henderson wrote:
> Require that the caller output the offset and increment bufptr.
> Use QEMU_HEXDUMP_LINE_BYTES in vhost_vdpa_dump_config instead
> of raw integer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/cutils.h | 2 +-
> hw/virtio/vhost-vdpa.c | 4 ++--
> util/hexdump.c | 13 ++++++-------
> hw/virtio/trace-events | 2 +-
> 4 files changed, 10 insertions(+), 11 deletions(-)
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 96632fd026..d01bc85037 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -50,7 +50,7 @@ vhost_vdpa_get_device_id(void *dev, uint32_t device_id) "dev: %p device_id %"PRI
> vhost_vdpa_reset_device(void *dev) "dev: %p"
> vhost_vdpa_get_vq_index(void *dev, int idx, int vq_idx) "dev: %p idx: %d vq idx: %d"
> vhost_vdpa_set_vring_enable_one(void *dev, unsigned i, int enable, int r) "dev: %p, idx: %u, enable: %u, r: %d"
> -vhost_vdpa_dump_config(void *dev, const char *line) "dev: %p %s"
> +vhost_vdpa_dump_config(void *dev, unsigned ofs, const char *line) "dev: %p %04x: %s"
Queued squashing "0x" due to:
ERROR: Hex numbers must be prefixed with '0x'
#108: FILE: hw/virtio/trace-events:53:
+vhost_vdpa_dump_config(void *dev, unsigned ofs, const char *line)
"dev: %p %04x: %s"
total: 1 errors, 0 warnings, 67 lines checked
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 21/27] disas/riscv: Use GString in format_inst
2024-04-12 7:33 ` [PATCH v3 21/27] disas/riscv: Use GString in format_inst Richard Henderson
2024-06-03 19:24 ` Pierrick Bouvier
@ 2024-06-04 0:15 ` Alistair Francis
1 sibling, 0 replies; 63+ messages in thread
From: Alistair Francis @ 2024-06-04 0:15 UTC (permalink / raw
To: Richard Henderson; +Cc: qemu-devel
On Fri, Apr 12, 2024 at 5:37 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Allocate and fill a GString instead of snprintf and
> appending to a fixed sized buffer.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> disas/riscv.c | 209 ++++++++++++++++++++++----------------------------
> 1 file changed, 92 insertions(+), 117 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index e236c8b5b7..f60f9bda43 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -4757,272 +4757,249 @@ static size_t inst_length(rv_inst inst)
>
> /* format instruction */
>
> -static void append(char *s1, const char *s2, size_t n)
> -{
> - size_t l1 = strlen(s1);
> - if (n - l1 - 1 > 0) {
> - strncat(s1, s2, n - l1);
> - }
> -}
> -
> -static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
> +static GString *format_inst(size_t tab, rv_decode *dec)
> {
> const rv_opcode_data *opcode_data = dec->opcode_data;
> - char tmp[64];
> + GString *buf = g_string_sized_new(64);
> const char *fmt;
>
> fmt = opcode_data[dec->op].format;
> while (*fmt) {
> switch (*fmt) {
> case 'O':
> - append(buf, opcode_data[dec->op].name, buflen);
> + g_string_append(buf, opcode_data[dec->op].name);
> break;
> case '(':
> - append(buf, "(", buflen);
> - break;
> case ',':
> - append(buf, ",", buflen);
> - break;
> case ')':
> - append(buf, ")", buflen);
> - break;
> case '-':
> - append(buf, "-", buflen);
> + g_string_append_c(buf, *fmt);
> break;
> case 'b':
> - snprintf(tmp, sizeof(tmp), "%d", dec->bs);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->bs);
> break;
> case 'n':
> - snprintf(tmp, sizeof(tmp), "%d", dec->rnum);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->rnum);
> break;
> case '0':
> - append(buf, rv_ireg_name_sym[dec->rd], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rd]);
> break;
> case '1':
> - append(buf, rv_ireg_name_sym[dec->rs1], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
> break;
> case '2':
> - append(buf, rv_ireg_name_sym[dec->rs2], buflen);
> + g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
> break;
> case '3':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rd] :
> - rv_freg_name_sym[dec->rd],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rd]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rd]);
> + }
> break;
> case '4':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs1] :
> - rv_freg_name_sym[dec->rs1],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs1]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs1]);
> + }
> break;
> case '5':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs2] :
> - rv_freg_name_sym[dec->rs2],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs2]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs2]);
> + }
> break;
> case '6':
> - append(buf, dec->cfg->ext_zfinx ? rv_ireg_name_sym[dec->rs3] :
> - rv_freg_name_sym[dec->rs3],
> - buflen);
> + if (dec->cfg->ext_zfinx) {
> + g_string_append(buf, rv_ireg_name_sym[dec->rs3]);
> + } else {
> + g_string_append(buf, rv_freg_name_sym[dec->rs3]);
> + }
> break;
> case '7':
> - snprintf(tmp, sizeof(tmp), "%d", dec->rs1);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->rs1);
> break;
> case 'i':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm);
> break;
> case 'u':
> - snprintf(tmp, sizeof(tmp), "%u", ((uint32_t)dec->imm & 0b111111));
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%u", ((uint32_t)dec->imm & 0b111111));
> break;
> case 'j':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm1);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm1);
> break;
> case 'o':
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm);
> - append(buf, tmp, buflen);
> - while (strlen(buf) < tab * 2) {
> - append(buf, " ", buflen);
> + g_string_append_printf(buf, "%d", dec->imm);
> + while (buf->len < tab * 2) {
> + g_string_append_c(buf, ' ');
> }
> - snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
> - dec->pc + dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
> break;
> case 'U':
> fmt++;
> - snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "%d", dec->imm >> 12);
> if (*fmt == 'o') {
> - while (strlen(buf) < tab * 2) {
> - append(buf, " ", buflen);
> + while (buf->len < tab * 2) {
> + g_string_append_c(buf, ' ');
> }
> - snprintf(tmp, sizeof(tmp), "# 0x%" PRIx64,
> - dec->pc + dec->imm);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "# 0x%" PRIx64, dec->pc + dec->imm);
> }
> break;
> case 'c': {
> const char *name = csr_name(dec->imm & 0xfff);
> if (name) {
> - append(buf, name, buflen);
> + g_string_append(buf, name);
> } else {
> - snprintf(tmp, sizeof(tmp), "0x%03x", dec->imm & 0xfff);
> - append(buf, tmp, buflen);
> + g_string_append_printf(buf, "0x%03x", dec->imm & 0xfff);
> }
> break;
> }
> case 'r':
> switch (dec->rm) {
> case rv_rm_rne:
> - append(buf, "rne", buflen);
> + g_string_append(buf, "rne");
> break;
> case rv_rm_rtz:
> - append(buf, "rtz", buflen);
> + g_string_append(buf, "rtz");
> break;
> case rv_rm_rdn:
> - append(buf, "rdn", buflen);
> + g_string_append(buf, "rdn");
> break;
> case rv_rm_rup:
> - append(buf, "rup", buflen);
> + g_string_append(buf, "rup");
> break;
> case rv_rm_rmm:
> - append(buf, "rmm", buflen);
> + g_string_append(buf, "rmm");
> break;
> case rv_rm_dyn:
> - append(buf, "dyn", buflen);
> + g_string_append(buf, "dyn");
> break;
> default:
> - append(buf, "inv", buflen);
> + g_string_append(buf, "inv");
> break;
> }
> break;
> case 'p':
> if (dec->pred & rv_fence_i) {
> - append(buf, "i", buflen);
> + g_string_append_c(buf, 'i');
> }
> if (dec->pred & rv_fence_o) {
> - append(buf, "o", buflen);
> + g_string_append_c(buf, 'o');
> }
> if (dec->pred & rv_fence_r) {
> - append(buf, "r", buflen);
> + g_string_append_c(buf, 'r');
> }
> if (dec->pred & rv_fence_w) {
> - append(buf, "w", buflen);
> + g_string_append_c(buf, 'w');
> }
> break;
> case 's':
> if (dec->succ & rv_fence_i) {
> - append(buf, "i", buflen);
> + g_string_append_c(buf, 'i');
> }
> if (dec->succ & rv_fence_o) {
> - append(buf, "o", buflen);
> + g_string_append_c(buf, 'o');
> }
> if (dec->succ & rv_fence_r) {
> - append(buf, "r", buflen);
> + g_string_append_c(buf, 'r');
> }
> if (dec->succ & rv_fence_w) {
> - append(buf, "w", buflen);
> + g_string_append_c(buf, 'w');
> }
> break;
> case '\t':
> - while (strlen(buf) < tab) {
> - append(buf, " ", buflen);
> + while (buf->len < tab) {
> + g_string_append_c(buf, ' ');
> }
> break;
> case 'A':
> if (dec->aq) {
> - append(buf, ".aq", buflen);
> + g_string_append(buf, ".aq");
> }
> break;
> case 'R':
> if (dec->rl) {
> - append(buf, ".rl", buflen);
> + g_string_append(buf, ".rl");
> }
> break;
> case 'l':
> - append(buf, ",v0", buflen);
> + g_string_append(buf, ",v0");
> break;
> case 'm':
> if (dec->vm == 0) {
> - append(buf, ",v0.t", buflen);
> + g_string_append(buf, ",v0.t");
> }
> break;
> case 'D':
> - append(buf, rv_vreg_name_sym[dec->rd], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rd]);
> break;
> case 'E':
> - append(buf, rv_vreg_name_sym[dec->rs1], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs1]);
> break;
> case 'F':
> - append(buf, rv_vreg_name_sym[dec->rs2], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs2]);
> break;
> case 'G':
> - append(buf, rv_vreg_name_sym[dec->rs3], buflen);
> + g_string_append(buf, rv_vreg_name_sym[dec->rs3]);
> break;
> case 'v': {
> - char nbuf[32] = {0};
> const int sew = 1 << (((dec->vzimm >> 3) & 0b111) + 3);
> - sprintf(nbuf, "%d", sew);
> const int lmul = dec->vzimm & 0b11;
> const int flmul = (dec->vzimm >> 2) & 1;
> const char *vta = (dec->vzimm >> 6) & 1 ? "ta" : "tu";
> const char *vma = (dec->vzimm >> 7) & 1 ? "ma" : "mu";
> - append(buf, "e", buflen);
> - append(buf, nbuf, buflen);
> - append(buf, ",m", buflen);
> +
> + g_string_append_printf(buf, "e%d,m", sew);
> if (flmul) {
> switch (lmul) {
> case 3:
> - sprintf(nbuf, "f2");
> + g_string_append(buf, "f2");
> break;
> case 2:
> - sprintf(nbuf, "f4");
> + g_string_append(buf, "f4");
> break;
> case 1:
> - sprintf(nbuf, "f8");
> - break;
> + g_string_append(buf, "f8");
> + break;
> }
> - append(buf, nbuf, buflen);
> } else {
> - sprintf(nbuf, "%d", 1 << lmul);
> - append(buf, nbuf, buflen);
> + g_string_append_printf(buf, "%d", 1 << lmul);
> }
> - append(buf, ",", buflen);
> - append(buf, vta, buflen);
> - append(buf, ",", buflen);
> - append(buf, vma, buflen);
> + g_string_append_c(buf, ',');
> + g_string_append(buf, vta);
> + g_string_append_c(buf, ',');
> + g_string_append(buf, vma);
> break;
> }
> case 'x': {
> switch (dec->rlist) {
> case 4:
> - snprintf(tmp, sizeof(tmp), "{ra}");
> + g_string_append(buf, "{ra}");
> break;
> case 5:
> - snprintf(tmp, sizeof(tmp), "{ra, s0}");
> + g_string_append(buf, "{ra, s0}");
> break;
> case 15:
> - snprintf(tmp, sizeof(tmp), "{ra, s0-s11}");
> + g_string_append(buf, "{ra, s0-s11}");
> break;
> default:
> - snprintf(tmp, sizeof(tmp), "{ra, s0-s%d}", dec->rlist - 5);
> + g_string_append_printf(buf, "{ra, s0-s%d}", dec->rlist - 5);
> break;
> }
> - append(buf, tmp, buflen);
> break;
> }
> case 'h':
> - append(buf, rv_fli_name_const[dec->imm], buflen);
> + g_string_append(buf, rv_fli_name_const[dec->imm]);
> break;
> default:
> break;
> }
> fmt++;
> }
> +
> + return buf;
> }
>
> /* lift instruction to pseudo-instruction */
> @@ -5108,9 +5085,8 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
>
> /* disassemble instruction */
>
> -static void
> -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> - RISCVCPUConfig *cfg)
> +static GString *disasm_inst(rv_isa isa, uint64_t pc, rv_inst inst,
> + RISCVCPUConfig *cfg)
> {
> rv_decode dec = { 0 };
> dec.pc = pc;
> @@ -5157,7 +5133,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> decode_inst_operands(&dec, isa);
> decode_inst_decompress(&dec, isa);
> decode_inst_lift_pseudo(&dec);
> - format_inst(buf, buflen, 24, &dec);
> + return format_inst(24, &dec);
> }
>
> #define INST_FMT_2 "%04" PRIx64 " "
> @@ -5168,7 +5144,6 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> static int
> print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
> {
> - char buf[128] = { 0 };
> bfd_byte packet[2];
> rv_inst inst = 0;
> size_t len = 2;
> @@ -5209,9 +5184,9 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
> }
> }
>
> - disasm_inst(buf, sizeof(buf), isa, memaddr, inst,
> - (RISCVCPUConfig *)info->target_info);
> - (*info->fprintf_func)(info->stream, "%s", buf);
> + g_autoptr(GString) str =
> + disasm_inst(isa, memaddr, inst, (RISCVCPUConfig *)info->target_info);
> + (*info->fprintf_func)(info->stream, "%s", str->str);
>
> return len;
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf
2024-04-12 7:33 ` [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf Richard Henderson
2024-06-03 19:06 ` Pierrick Bouvier
@ 2024-06-04 0:16 ` Alistair Francis
1 sibling, 0 replies; 63+ messages in thread
From: Alistair Francis @ 2024-06-04 0:16 UTC (permalink / raw
To: Richard Henderson; +Cc: qemu-devel, Philippe Mathieu-Daudé
On Fri, Apr 12, 2024 at 5:36 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
> Use g_strdup_printf instead.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20240411104340.6617-8-philmd@linaro.org>
> [rth: Use g_strdup_printf]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d171e74f7b..4fdb660525 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1617,10 +1617,8 @@ static void virt_machine_instance_init(Object *obj)
> static char *virt_get_aia_guests(Object *obj, Error **errp)
> {
> RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
> - char val[32];
>
> - sprintf(val, "%d", s->aia_guests);
> - return g_strdup(val);
> + return g_strdup_printf("%d", s->aia_guests);
> }
>
> static void virt_set_aia_guests(Object *obj, const char *val, Error **errp)
> @@ -1741,7 +1739,6 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>
> static void virt_machine_class_init(ObjectClass *oc, void *data)
> {
> - char str[128];
> MachineClass *mc = MACHINE_CLASS(oc);
> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
> @@ -1767,7 +1764,6 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> #endif
>
> -
> object_class_property_add_bool(oc, "aclint", virt_get_aclint,
> virt_set_aclint);
> object_class_property_set_description(oc, "aclint",
> @@ -1785,9 +1781,14 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> object_class_property_add_str(oc, "aia-guests",
> virt_get_aia_guests,
> virt_set_aia_guests);
> - sprintf(str, "Set number of guest MMIO pages for AIA IMSIC. Valid value "
> - "should be between 0 and %d.", VIRT_IRQCHIP_MAX_GUESTS);
> - object_class_property_set_description(oc, "aia-guests", str);
> + {
> + g_autofree char *str =
> + g_strdup_printf("Set number of guest MMIO pages for AIA IMSIC. "
> + "Valid value should be between 0 and %d.",
> + VIRT_IRQCHIP_MAX_GUESTS);
> + object_class_property_set_description(oc, "aia-guests", str);
> + }
> +
> object_class_property_add(oc, "acpi", "OnOffAuto",
> virt_get_acpi, virt_set_acpi,
> NULL, NULL);
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 63+ messages in thread
end of thread, other threads:[~2024-06-04 0:17 UTC | newest]
Thread overview: 63+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-12 7:33 [PATCH v3 00/27] misc: Replace sprintf Richard Henderson
2024-04-12 7:33 ` [PATCH v3 01/27] util/hexdump: Remove b parameter from qemu_hexdump_line Richard Henderson
2024-04-12 10:00 ` Philippe Mathieu-Daudé
2024-06-03 19:40 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 02/27] util/hexdump: Remove ascii " Richard Henderson
2024-04-12 10:03 ` Philippe Mathieu-Daudé
2024-06-03 17:42 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line Richard Henderson
2024-04-12 17:41 ` Philippe Mathieu-Daudé
2024-04-12 18:59 ` Richard Henderson
2024-04-13 9:45 ` Philippe Mathieu-Daudé
2024-04-13 12:02 ` Stefan Hajnoczi
2024-04-12 7:33 ` [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line Richard Henderson
2024-04-12 17:42 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x" Richard Henderson
2024-04-12 17:43 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 06/27] hw/mips/malta: Add re-usable rng_seed_hex_new() method Richard Henderson
2024-06-03 18:56 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line Richard Henderson
2024-04-12 17:45 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf Richard Henderson
2024-04-12 17:45 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 09/27] hw/ide/atapi: " Richard Henderson
2024-06-03 18:57 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 10/27] hw/dma/pl330: " Richard Henderson
2024-06-03 18:59 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 11/27] backends/tpm: " Richard Henderson
2024-04-12 17:47 ` Philippe Mathieu-Daudé
2024-04-12 7:33 ` [PATCH v3 12/27] disas/m68k: Replace sprintf() by snprintf() Richard Henderson
2024-04-12 7:33 ` [PATCH v3 13/27] disas/microblaze: " Richard Henderson
2024-04-12 7:33 ` [PATCH v3 14/27] disas/microblaze: Split out print_immval_addr Richard Henderson
2024-06-03 19:14 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 15/27] target/microblaze: Re-indent print_insn_microblaze Richard Henderson
2024-06-03 19:14 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 16/27] disas/microblaze: Merge op->name output into each fprintf Richard Henderson
2024-06-03 19:15 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 17/27] disas/microblaze: Print registers directly with PRIreg Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 18/27] disas/microblaze: Print immediates directly with PRIimm Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 19/27] disas/microblaze: Print registers directly with PRIrfsl Richard Henderson
2024-06-03 19:17 ` Pierrick Bouvier
2024-06-03 19:18 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 20/27] disas/microblaze: Split get_field_special Richard Henderson
2024-06-03 19:20 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 21/27] disas/riscv: Use GString in format_inst Richard Henderson
2024-06-03 19:24 ` Pierrick Bouvier
2024-06-04 0:15 ` Alistair Francis
2024-04-12 7:33 ` [PATCH v3 22/27] linux-user/flatload: Replace sprintf() by snprintf() Richard Henderson
2024-04-12 9:48 ` Peter Maydell
2024-06-03 19:02 ` Pierrick Bouvier
2024-04-12 7:33 ` [PATCH v3 23/27] hw/misc/imx: " Richard Henderson
2024-04-12 7:33 ` [PATCH v3 24/27] hw/net/rocker: " Richard Henderson
2024-04-12 10:58 ` Philippe Mathieu-Daudé
2024-04-12 14:51 ` Richard Henderson
2024-04-12 7:33 ` [PATCH v3 25/27] hw/riscv/virt: Replace sprintf by g_strdup_printf Richard Henderson
2024-06-03 19:06 ` Pierrick Bouvier
2024-06-04 0:16 ` Alistair Francis
2024-04-12 7:33 ` [PATCH v3 26/27] target/arm: Replace sprintf() by snprintf() Richard Henderson
2024-04-12 7:33 ` [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings Richard Henderson
2024-04-12 10:59 ` Philippe Mathieu-Daudé
2024-04-16 15:32 ` Peter Maydell
2024-04-16 15:32 ` Zhao Liu
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).