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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan