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/60089475-3891-4448-bfe0-8dd698cd2435@epfl.ch/
Signed-off-by: Florian Hofhammer <florian.hofhammer@fhofhammer.de>
---
 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

On 1/19/26 5:18 AM, Florian Hofhammer wrote:
> 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/60089475-3891-4448-bfe0-8dd698cd2435@epfl.ch/
> Signed-off-by: Florian Hofhammer <florian.hofhammer@fhofhammer.de>
> ---
>   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(-)
> 

This was merged into master (a8e6997ef).
Thank you for your contribution!

Regards,
Pierrick

On 1/19/26 5:18 AM, Florian Hofhammer wrote:
> 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/60089475-3891-4448-bfe0-8dd698cd2435@epfl.ch/
> Signed-off-by: Florian Hofhammer <florian.hofhammer@fhofhammer.de>
> ---
>   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(-)
> 

Thank you for the very quick patch Florian, looks great!

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>