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

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

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.
> > To me, the main takeaway is that there
> > are many unavoidable but unnecessary changes.
> Interesting. I'd say it slightly differently: The main takeaway is that
> there are many avoidable / unnecessary changes.
But at the end of the day it will allow using automatic formatting...
> Jan
Thank you,
Oleksandr
[1] 
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#disabling-formatting-on-a-piece-of-code