Re: [PATCH v6 0/1] Optimizing the print format of the QEMU monitor 'info mtree'

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 3023 bytes --]

On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
> Hi Chao,
>
> On 1/5/25 04:24, Chao Liu wrote:
>> Hi, all:
>> 
>> Thanks to BALATON, and David for their reviews.
>> 
>> In PATCH v6:
>> 1. Replaced the bool type with enum mtree_node_type to improve code 
>> readability.
>> 2. Modified the output to use only one horizontal dash instead of two, and
>>     aligned character printing for a cleaner look.
>> 
>> like this:
>> 
>> ```
>> $ ./qemu-system-aarch64 -S -monitor stdio -M raspi4b
>> (qemu) info mtree
>> address-space: memory
>> `- 0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>     |- 0000000000000000-000000007fffffff (prio 0, ram): ram
>> ...
>>     |- 00000000fe000000-00000000ff7fffff (prio 1, i/o): bcm2835-peripherals
>>     |  |- 00000000fe900000-00000000fe907fff (prio -1000, i/o): bcm2835-dbus
>>     |  |- 00000000fe910000-00000000fe917fff (prio -1000, i/o): bcm2835-ave0
>>     |  |- 00000000fe980000-00000000fe990fff (prio 0, i/o): dwc2
>>     |  |  |- 00000000fe980000-00000000fe980fff (prio 0, i/o): dwc2-io
>>     |  |  `- 00000000fe981000-00000000fe990fff (prio 0, i/o): dwc2-fifo
>>     |  |- 00000000fec00000-00000000fec00fff (prio -1000, i/o): bcm2835-v3d
>>     |  |- 00000000fec11000-00000000fec110ff (prio -1000, i/o): 
>> bcm2835-clkisp
>>     |  |- 00000000fee00000-00000000fee000ff (prio -1000, i/o): 
>> bcm2835-sdramc
>>     |  `- 00000000fee05000-00000000fee050ff (prio 0, i/o): 
>> bcm2835-dma-chan15
>>     |- 00000000ff800000-00000000ff8000ff (prio 0, i/o): bcm2836-control
>> ...
>>     |- 00000000ff845600-00000000ff8456ff (prio 0, i/o): gic_cpu
>>     `- 00000000ff846000-00000000ff847fff (prio 0, i/o): gic_vcpu
>> ```
>
> Could we keep the address ranges aligned? I.e.:
>
>>   |--+     00000000fe000000-00000000ff7fffff (prio 1, i/o): 
> bcm2835-peripherals
>>   |  |---- 00000000fe900000-00000000fe907fff (prio -1000, i/o): 
> bcm2835-dbus
>>   |  |---- 00000000fe910000-00000000fe917fff (prio -1000, i/o): 
> bcm2835-ave0
>>   |  |--+  00000000fe980000-00000000fe990fff (prio 0, i/o): dwc2
>>   |  |  |- 00000000fe980000-00000000fe980fff (prio 0, i/o): dwc2-io
>>   |  |  `- 00000000fe981000-00000000fe990fff (prio 0, i/o): dwc2-fifo
>>   |  |---- 00000000fec00000-00000000fec00fff (prio -1000, i/o): bcm2835-v3d
>>   |  |---- 00000000fec11000-00000000fec110ff (prio -1000, i/o): 
> bcm2835-clkisp
>>   |  |---- 00000000fee00000-00000000fee000ff (prio -1000, i/o): 
> bcm2835-sdramc
>>   |  `---- 00000000fee05000-00000000fee050ff (prio 0, i/o): 
> bcm2835-dma-chan15
>>   |------- 00000000ff800000-00000000ff8000ff (prio 0, i/o): bcm2836-control

I don't think that helps. I was OK with just indents and adding graphics 
does not change that but this would actually make it harder to see what is 
below what as you'd have to trace back to the beginning of the line and 
not just look at the right end where it would not be clear where a sub 
region starts so it't less readable even if it looks more organised. So 
I'd keep the indent.

Regards,
BALATON Zoltan