On 2024/02/27 19:29, Alex Bennée wrote:
Akihiko Odaki <[email protected]> writes:

On 2024/02/27 19:08, Alex Bennée wrote:
Akihiko Odaki <[email protected]> writes:

On 2024/02/27 1:56, Alex Bennée wrote:
We can only request a list of registers once the vCPU has been
initialised so the user needs to use either call the get function on
vCPU initialisation or during the translation phase.
We don't expose the reg number to the plugin instead hiding it
behind
an opaque handle. For now this is just the gdb_regnum encapsulated in
an anonymous GPOINTER but in future as we add more state for plugins
to track we can expand it.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
Cc: Akihiko Odaki <[email protected]>
Message-Id: <[email protected]>
Based-on: <[email protected]>
Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Pierrick Bouvier <[email protected]>

Hi,

Mostly looks good. I have a few trivial comments so please have a look
at them.
Done
<snip>
+        g_array_append_val(find_data, desc);
+    }
+
+    return find_data;
+}
+
+GArray *qemu_plugin_get_registers(void)
+{
+    g_assert(current_cpu);
+
+    g_autoptr(GArray) regs = gdb_get_register_list(current_cpu);
+    return regs->len ? create_register_handles(current_cpu, regs) : NULL;

Why do you need regs->len check?
Not all guests expose register to gdb so we need to catch that:
    TEST    catch-syscalls-with-libinsn.so on alpha
**
ERROR:../../plugins/api.c:459:qemu_plugin_get_registers: assertion failed: 
(regs->len)
Aborted
Certainly regs->len can be 0, but why do you need to return NULL in
that case? Can't qemu_plugin_get_registers() return an empty array
just as gdb_get_register_list() does?

That seems more troublesome to handle the other end. NULL is nothing is
a fairly common pattern here which makes short-circuiting the array
iteration simple.

I'm not sure why you would want to short-circuiting array iteration. Iterating an empty array is often simpler.

In any case, the same logic also applies to gdb_get_register_list(), so you should change gdb_get_register_list() to return NULL instead of an empty array if you think that's more reasonable.

Reply via email to