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

[Edwin Torok <edwin.torok@xxxxxxxxx>]

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.

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?

> In addition this pretty extensive use of Unicode chars then raises a concern
> that was brought up for the binutils project a little while ago [1]. As I
> have indicated in replies there, isolated uses may be appropriate in certain
> cases. A larger comment like the one here is different though, as the
> slipping in of problematic characters (now or in the future) might go
> unnoticed.

https://github.com/rurban/libu8ident might help (there is also a
configuration there that'd match what is proposed for C26)
GCC 12+ also has some protection against it
https://developers.redhat.com/articles/2022/01/12/prevent-trojan-source-attacks-gcc-12
Safely using Unicode in source code is a rather complex (and perhaps
not entirely solved) topic, so I understand if you'd rather avoid
extensive Unicode
use.

Best regards,
--Edwin

>
> Jan
>
> [1] 
> https://inbox.sourceware.org/binutils/72219ed1-147d-4dd3-b503-363d981528a2@xxxxxxx/