Re: [RFC PATCH v1 03/10] arch-x86/pmu.h: convert ascii art drawing to Unicode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.07.2025 18:07, Edwin Torok wrote:
> On Mon, Jul 28, 2025 at 11:23 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 25.07.2025 17:06, Edwin Török wrote:
>>> Use `aa2u` (ascii-art-to-unicode from Hackage) to convert to
>>> Unicode box drawing characters.
>>>
>>> The full list of supported drawing characters can be seen in the
>>> examples at:
>>> https://github.com/fmthoma/ascii-art-to-unicode/blob/master/src/Text/AsciiArt.hs
>>>
>>> For future maintenance: conversion can be done incrementally
>>> (mixing ascii art with already converted Unicode,
>>>  and running the conversion tool again), or by hand.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Edwin Török <edwin.torok@xxxxxxxxx>
>>> ---
>>>  xen/include/public/arch-x86/pmu.h | 120 +++++++++++++++---------------
>>>  1 file changed, 60 insertions(+), 60 deletions(-)
>>
>> I'm already unconvinced of the earlier patch: The whole construct isn't self-
>> explanatory, and it lacks a legend. There's also the question of legibility.
>> The change here has the main problem of making readability dependent upon the
>> capabilities of the editor / viewer / etc one is using. For example, the '┆'
>> character as well as the arrow ones I can't get Win10's console subsystem to
>> display properly.
>>
> 
> The original ASCII diagram could also be moved to another file that
> contains only documentation and is not used during compilation.
> There is https://ivanceras.github.io/svgbob-editor/ which can then
> create an SVG out of it if needed.
> The SVG (or its ASCII source) wouldn't be restricted to 80 chars, and
> could contain more details.
> 
> Although if it is a separate file it is more likely to go stale when
> the .h is updated.
> 
> Here is a solution that works with ASCII instead then (the diagram
> itself is not very readable in pure ASCII).
> I think my main goal was to understand what the flexible array member
> would contain, and that could actually be explained in a sort of
> pseudo-C.
> Something like:
> 
> ```
> struct ... {
>  uint32_t fixed_counters;
>  uint32_t arch_counters;
> ....
>   union {
>       uint64_t regs[];
>       struct {
>            uint64_t fixed[fixed_counters];
>            struct xen_pmu_cntr_pair arch[arch_counters];
>       }
>   }
> }
> ```
> 
> This isn't (yet?) valid C, although it follows the trend the C
> standard is evolving to, e.g. you can already refer to array
> dimensions in function arguments, where the array dimension is another
> function argument, in fact the manpages already started to get updated
> to follow this new style, and newer versions of GCC support it, e.g.
> memcpy: https://man7.org/linux/man-pages/man3/memcpy.3.html
> I don't know whether future C  standards would ever add support for
> flexible array members where the size depends on another struct field,
> but it should be fine as a comment, and perhaps easier to maintain
> than a diagram. If it ever becomes valid C it can be promoted from a
> comment to actual code.

Somewhat related (but afaict not really usable here, leaving aside that
this is a public header and hence needs to remain free of extension uses)
is gcc's relatively new counted_by attribute.

> It'd retain the main benefit: being able to see the memory layout,
> without having to read through the source code and all the
> sizeof/offset pointer calculation to figure out what is actually
> stored in regs[] and how big it could be.
> 
> What do you think?

Why not.

Jan