Re: [PATCH trivial] plugins: add missing docstrings to qemu-plugin.h

On 3/9/26 9:35 AM, Peter Maydell wrote:
> On Mon, 9 Mar 2026 at 16:22, Florian Hofhammer
> <[email protected]> wrote:
>>
>> This patch adds docstrings for typedefs and function declarations in
>> include/plugins/qemu-plugin.h that were previously missing. This
>> resolves inconsistencies in the docs, e.g., the description for
>> qemu_plugin_read_register() referring to qemu_plugin_register_flush_cb()
>> but code cache flush callbacks not being documented themselves.
>>
>> Signed-off-by: Florian Hofhammer <[email protected]>
> 
> I don't know this API either, so a couple of questions below
> for those who do:
> 
>> +/**
>> + * enum qemu_plugin_mem_rw - type of memory access
>> + *
>> + * @QEMU_PLUGIN_MEM_R: memory read access only
>> + * @QEMU_PLUGIN_MEM_W: memory write access only
>> + * @QEMU_PLUGIN_MEM_RW: memory read and write access
>> + */
>>   enum qemu_plugin_mem_rw {
>>       QEMU_PLUGIN_MEM_R = 1,
>>       QEMU_PLUGIN_MEM_W,
>>       QEMU_PLUGIN_MEM_RW,
>>   };
> 
> This define sets up the values such that you can use
> the _R and _W values as bit checks (i.e. _RW == _R | _W).
> Is that intentional and can code using the API rely on it,
> or must it strictly check against _R and _RW separately?
> 
>> +/**
>> + * enum qemu_plugin_mem_value_type - size of memory value
>> + *
>> + * @QEMU_PLUGIN_MEM_VALUE_U8: unsigned 8-bit value
>> + * @QEMU_PLUGIN_MEM_VALUE_U16: unsigned 16-bit value
>> + * @QEMU_PLUGIN_MEM_VALUE_U32: unsigned 32-bit value
>> + * @QEMU_PLUGIN_MEM_VALUE_U64: unsigned 64-bit value
>> + * @QEMU_PLUGIN_MEM_VALUE_U128: unsigned 128-bit value
>> + */
>>   enum qemu_plugin_mem_value_type {
>>       QEMU_PLUGIN_MEM_VALUE_U8,
>>       QEMU_PLUGIN_MEM_VALUE_U16,
>> @@ -353,7 +369,13 @@ enum qemu_plugin_mem_value_type {
>>       QEMU_PLUGIN_MEM_VALUE_U128,
>>   };
> 
> In this enum, the values are set up such that if you have
> a qemu_plugin_mem_value_type vt, then the size of the data
> is (1 << vt) (i.e., a byte is 0, a short is 1, 32 bits are 2,
> and so on up). Is that intentional and can code using the
> API rely on it, or must it strictly use the enum only as
> something to switch() on to select a length.
>

The latter, see it as something similar to Rust enums, or what theory 
calls an Algebraic data type:
https://doc.rust-lang.org/std/keyword.enum.html

> I actually ran into that last one in tests/tcg/plugins/mem.c:
> update_region_info() has a switch() on this enum which (among
> other things) sets val_size in each case, because I didn't
> know if it was guaranteed to work to set val_size = 1 << value.type.
> 
> thanks
> -- PMM