The qemu_plugin_{read,write} register API previously was inconsistent
with regard to its docstring (where a return value of both -1 and 0
would indicate an error) and to the memory read/write APIs, which
already return a boolean value to indicate success or failure.
Returning the number of bytes read or written is superfluous, as the
GByteArray* passed to the API functions already encodes the length.
See the linked thread for more details.This patch moves from returning an int (number of bytes read/written) to returning a bool from the register read/write API, bumps the plugin API version, and adjusts plugins and tests accordingly. Link: https://lore.kernel.org/qemu-devel/[email protected]/ Signed-off-by: Florian Hofhammer <[email protected]> --- contrib/plugins/execlog.c | 14 ++++++++------ contrib/plugins/uftrace.c | 8 ++++---- include/qemu/qemu-plugin.h | 19 +++++++++++-------- plugins/api.c | 15 ++++++++------- tests/tcg/plugins/insn.c | 4 ++-- 5 files changed, 33 insertions(+), 27 deletions(-) diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c index 811f320319..d00d9c4ff3 100644 --- a/contrib/plugins/execlog.c +++ b/contrib/plugins/execlog.c @@ -91,11 +91,13 @@ static void insn_check_regs(CPU *cpu) { for (int n = 0; n < cpu->registers->len; n++) { Register *reg = cpu->registers->pdata[n]; - int sz; + bool success = false; + int sz = 0; g_byte_array_set_size(reg->new, 0); - sz = qemu_plugin_read_register(reg->handle, reg->new); - g_assert(sz > 0); + success = qemu_plugin_read_register(reg->handle, reg->new); + g_assert(success); + sz = reg->new->len; g_assert(sz == reg->last->len); if (memcmp(reg->last->data, reg->new->data, sz)) { @@ -303,7 +305,7 @@ static Register *init_vcpu_register(qemu_plugin_reg_descriptor *desc) { Register *reg = g_new0(Register, 1); g_autofree gchar *lower = g_utf8_strdown(desc->name, -1); - int r; + bool success = false; reg->handle = desc->handle; reg->name = g_intern_string(lower); @@ -311,8 +313,8 @@ static Register *init_vcpu_register(qemu_plugin_reg_descriptor *desc) reg->new = g_byte_array_new(); /* read the initial value */ - r = qemu_plugin_read_register(reg->handle, reg->last); - g_assert(r > 0); + success = qemu_plugin_read_register(reg->handle, reg->last); + g_assert(success); return reg; } diff --git a/contrib/plugins/uftrace.c b/contrib/plugins/uftrace.c index b7d6124d2f..a7e21b5b87 100644 --- a/contrib/plugins/uftrace.c +++ b/contrib/plugins/uftrace.c @@ -403,8 +403,8 @@ static uint64_t cpu_read_register64(Cpu *cpu, struct qemu_plugin_register *reg) { GByteArray *buf = cpu->buf; g_byte_array_set_size(buf, 0); - size_t sz = qemu_plugin_read_register(reg, buf); - g_assert(sz == 8); + bool success = qemu_plugin_read_register(reg, buf); + g_assert(success); g_assert(buf->len == 8); return *((uint64_t *) buf->data); } @@ -413,8 +413,8 @@ static uint32_t cpu_read_register32(Cpu *cpu, struct qemu_plugin_register *reg) { GByteArray *buf = cpu->buf; g_byte_array_set_size(buf, 0); - size_t sz = qemu_plugin_read_register(reg, buf); - g_assert(sz == 4); + bool success = qemu_plugin_read_register(reg, buf); + g_assert(success); g_assert(buf->len == 4); return *((uint32_t *) buf->data); } diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index 60de4fdd3f..c3b0532619 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -72,11 +72,14 @@ typedef uint64_t qemu_plugin_id_t; * - added qemu_plugin_write_memory_hwaddr * - added qemu_plugin_write_register * - added qemu_plugin_translate_vaddr + * + * version 6: + * - changed return value of qemu_plugin_{read,write}_register from int to bool */ extern QEMU_PLUGIN_EXPORT int qemu_plugin_version; -#define QEMU_PLUGIN_VERSION 5 +#define QEMU_PLUGIN_VERSION 6 /** * struct qemu_info_t - system information for plugins @@ -972,12 +975,12 @@ GArray *qemu_plugin_get_registers(void); * qemu_plugin_register_vcpu_init_cb(), except for callbacks registered with * qemu_plugin_register_atexit_cb() and qemu_plugin_register_flush_cb(). * - * Returns the size of the read register. The content of @buf is in target byte - * order. On failure returns -1. + * Returns true on success, false on failure. The content of @buf is in target + * byte order. */ QEMU_PLUGIN_API -int qemu_plugin_read_register(struct qemu_plugin_register *handle, - GByteArray *buf); +bool qemu_plugin_read_register(struct qemu_plugin_register *handle, + GByteArray *buf); /** * qemu_plugin_write_register() - write register for current vCPU @@ -997,11 +1000,11 @@ int qemu_plugin_read_register(struct qemu_plugin_register *handle, * Attempting to write a register with @buf smaller than the register size * will result in a crash or other undesired behavior. * - * Returns the number of bytes written. On failure returns 0. + * Returns true on sucess, false on failure. */ QEMU_PLUGIN_API -int qemu_plugin_write_register(struct qemu_plugin_register *handle, - GByteArray *buf); +bool qemu_plugin_write_register(struct qemu_plugin_register *handle, + GByteArray *buf); /** * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address diff --git a/plugins/api.c b/plugins/api.c index eac04cc1f6..d8846b4773 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -434,27 +434,28 @@ GArray *qemu_plugin_get_registers(void) return create_register_handles(regs); } -int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf) +bool qemu_plugin_read_register(struct qemu_plugin_register *reg, + GByteArray *buf) { g_assert(current_cpu); if (qemu_plugin_get_cb_flags() == QEMU_PLUGIN_CB_NO_REGS) { - return -1; + return false; } - return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1); + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0); } -int qemu_plugin_write_register(struct qemu_plugin_register *reg, - GByteArray *buf) +bool qemu_plugin_write_register(struct qemu_plugin_register *reg, + GByteArray *buf) { g_assert(current_cpu); if (buf->len == 0 || qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS) { - return -1; + return false; } - return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1); + return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0); } bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len) diff --git a/tests/tcg/plugins/insn.c b/tests/tcg/plugins/insn.c index 0c723cb9ed..e6c5207cd6 100644 --- a/tests/tcg/plugins/insn.c +++ b/tests/tcg/plugins/insn.c @@ -93,8 +93,8 @@ static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index) for (int i = 0; i < reg_list->len; i++) { qemu_plugin_reg_descriptor *rd = &g_array_index( reg_list, qemu_plugin_reg_descriptor, i); - int count = qemu_plugin_read_register(rd->handle, reg_value); - g_assert(count > 0); + bool success = qemu_plugin_read_register(rd->handle, reg_value); + g_assert(success); } } } base-commit: c1c58cee16380f81f88fbde6b12f247b376839e2 -- 2.51.0
