On 16/01/2026 22:39, Pierrick Bouvier wrote:
> For sake of consistency, we should make this use the same interface than
> {read,write}_memory_vaddr, minus the len param.
> bool qemu_plugin_read_memory_vaddr(uint64_t addr,
> GByteArray *data, size_t len);
> bool qemu_plugin_write_memory_vaddr(uint64_t addr,
> GByteArray *data);
>
> So it would be:
> bool qemu_plugin_read_register(uint64_t addr, GByteArray *data);
> bool qemu_plugin_write_register(uint64_t addr, GByteArray *data);
>
> This is better and unambiguous, as no one needs a documentation to know what
> a bool return is, and data already holds the size information.
Sounds like a good idea. This would break existing plugins though, is
this acceptable? (Asking purely out of curiosity, as I'm not familiar
yet with the processes and policies around breaking changes in QEMU
development :))
> As well, writing this, I realized that existing write_register is broken by
> design: we never check the size of data array (except > 1) and blindly an
> arbitrary amount of memory from it, which is wrong.
> Even though the doc mentions it, we should just fix it, detect when size <
> reg_size, and return false.
>
> This comes from the fact gdb_write_register itself has no notion of size and
> trust the pointer. so it would need another refactor also. And while at it,
> change gdb_{read,write}_register definition to return bool also.
gdb_read_register already takes a GByteArray, it would make sense to
also use GByteArray for gdb_write_register instead of a uint8_t* and use
the size of the array for sanity checks.
However, I'm not sure changing the return value to bool for those
internal functions makes sense, as other parts of the codebase rely on
the size information. E.g., handle_write_all_regs() in gdbstub/gdbstub.c
relies on the size returned by gdb_write_register() to know how far to
offset into the packet data it received from GDB, as GDB does not send
register size information when writing multiple registers via a 'G'
packet (just a flat byte stream with the register values concatenated).
> I think it pushes a lot of changes just for a simple comment change, so I
> would understand if you don't want to do this whole refactoring beyond
> plugins interface. I can pick it up, or let you work on it if you have
> time/interest. Feel free to let us know.
I'd be happy to handle this!
Best regards,
Florian
