Re: [PATCH v3 1/1] system: optimizing info mtree printing for monitors

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 30/4/25 23:38, David Hildenbrand wrote:
> On 30.04.25 11:58, Philippe Mathieu-Daudé wrote:
>> Hi Chao,
>>
>> On 30/4/25 11:02, Chao Liu wrote:
>>> Make the hierarchical relationship between nodes clearer by adding 
>>> characters
>>
>> W.r.t. $Subject, it isn't really about "optimization" but making
>> output clearer, as you describe.
>>
>>>
>>> e.g.
>>>
>>> qemu-system-riscv64 -M virt -monitor stdio -display none
>>>
>>> ```
>>> (qemu) info mtree
>>> ...
>>> memory-region: system
>>> │  ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>> │  │   ├── 0000000003000000-000000000300ffff (prio 0, i/o): 
>>> gpex_ioport_window
>>> │  │   │   └── 0000000003000000-000000000300ffff (prio 0, i/o): 
>>> gpex_ioport
>>> ...
>>> │  │   └── 0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
>>> ```
>>>
>>> Signed-off-by: Chao Liu <lc00631@tecorigin.com>
>>> Reviewed-by: Qingze Zhao <zqz00548@tecorigin.com>
>>> Reviewed-by: Tingjian Zhang <zhangtj@tecorigin.com>
>>> ---
>>>    system/memory.c | 36 ++++++++++++++++++++++++++++++------
>>>    1 file changed, 30 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/system/memory.c b/system/memory.c
>>> index 71434e7ad0..cf91718bcc 100644
>>> --- a/system/memory.c
>>> +++ b/system/memory.c
>>> @@ -3296,6 +3296,21 @@ typedef QTAILQ_HEAD(, MemoryRegionList) 
>>> MemoryRegionListHead;
>>>                               int128_sub((size), int128_one())) : 0)
>>>    #define MTREE_INDENT "  "
>>> +enum mtree_node_type {
>>> +    MTREE_NODE_T_INNER,
>>> +    MTREE_NODE_T_TAIL,
>>> +};
>>> +
>>> +static void mtree_print_node(enum mtree_node_type is_tail)
>>
>> Either rename 'is_tail' as 'node_type', or really use a boolean
>> (maybe '[is_]last_node' is clearer?), since the mtree_node_type enum
>> doesn't bring much (see [*] below).
>>
> 
> Note that I suggested that, because the passing or true/false in the 
> code is horrible for readability.

Oh good point. Sorry I missed previous version reviews.