Re: [PATCH 0/2] code style exercise: Drivers folder samples

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

Hello, Jan!

On 19.02.25 14:49, Jan Beulich wrote:
> On 19.02.2025 13:43, Oleksandr Andrushchenko wrote:
> > Hello, Jan, Stefano!
> >
> > On 18.02.25 13:34, Jan Beulich wrote:
> > > On 18.02.2025 03:36, Stefano Stabellini wrote:
> > > > On Mon, 17 Feb 2025, Jan Beulich wrote:
> > > > > On 16.02.2025 11:21, Oleksandr Andrushchenko wrote:
> > > > > > 1. Const string arrays reformatting
> > > > > > In case the length of items change we might need to introduce a bigger
> > > > > > change wrt new formatting of unaffected lines
> > > > > > ==============================================================================
> > > > > >
> > > > > > --- a/xen/drivers/acpi/tables.c
> > > > > > +++ b/xen/drivers/acpi/tables.c
> > > > > > @@ -38,10 +38,10 @@
> > > > > > -static const char *__initdata
> > > > > > -mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
> > > > > > -static const char *__initdata
> > > > > > -mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level" };
> > > > > > +static const char *__initdata mps_inti_flags_polarity[] = { "dfl", "high",
> > > > > > +                                                            "res", "low" };
> > > > > > +static const char *__initdata mps_inti_flags_trigger[] = { "dfl", "edge", 
> > > > > > "res",
> > > > > >
> > > > > > --- a/xen/drivers/acpi/utilities/utglobal.c
> > > > > > +++ b/xen/drivers/acpi/utilities/utglobal.c
> > > > > >    static const char *const acpi_gbl_region_types[ACPI_NUM_PREDEFINED_REGIONS] 
> > > > > > = {
> > > > > > -       "SystemMemory",
> > > > > > -       "SystemIO",
> > > > > > -       "PCI_Config",
> > > > > > -       "EmbeddedControl",
> > > > > > -       "SMBus",
> > > > > > -       "CMOS",
> > > > > > -       "PCIBARTarget",
> > > > > > -       "DataTable"
> > > > > > +    "SystemMemory", "SystemIO", "PCI_Config",   "EmbeddedControl",
> > > > > > +    "SMBus",        "CMOS",     "PCIBARTarget", "DataTable"
> > > > > >    };
> > > > > Why in the world would a tool need to touch anything like the two examples
> > > > > above? My take is that the code is worse readability-wise afterwards.
> > > > I think the output is acceptable: not necessarily better than before,
> > > > but also not significantly worse.
> > > Hmm, for the change to xen/drivers/acpi/tables.c I wouldn't agree with this
> > > statement. And for xen/drivers/acpi/utilities/utglobal.c remember that this
> > > is code taken from ACPI CA, which we may better not re-format.
> > We can use /* clang-format off */ constructs to protect those lines we
> > do not want to be touched by clang-format [1]. This is what Grygprii
> > mentioned in some other e-mail.
> We have fall-through comments. We have SAF comments. Yet another flavor to
> keep some external tool happy. If everyone else thinks this is a good idea,
> I'm not intending to stand in the way. Yet I don't like this as a workaround.
> Instead I think the tool's going too far.
Yes, I do agree. But only if we talk about having an automated
code style check now (which is definitely the goal at some time).
Before that we could still use the tool to take all that good that
it does and manually prepare a set of patches to fix those
code style issues which we like.
> Jan