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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Cheers,

David / dhildenb