Re: [PATCH v5 08/10] acpi/nfit: Improve terminator line in acpi_nfit_ids

[Wilczynski, Michal]

On 6/30/2023 1:13 PM, Rafael J. Wysocki wrote:
> On Fri, Jun 30, 2023 at 1:04 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>> On Fri, Jun 30, 2023 at 11:52 AM Wilczynski, Michal
>> <michal.wilczynski@xxxxxxxxx> wrote:
>>>
>>>
>>> On 6/29/2023 6:14 PM, Rafael J. Wysocki wrote:
>>>> On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski
>>>> <michal.wilczynski@xxxxxxxxx> wrote:
>>>>> Currently terminator line contains redunant characters.
>>>> Well, they are terminating the list properly AFAICS, so they aren't
>>>> redundant and the size of it before and after the change is actually
>>>> the same, isn't it?
>>> This syntax is correct of course, but we have an internal guidelines specifically
>>> saying that terminator line should NOT contain a comma at the end. Justification:
>>>
>>> "Terminator line is established for the data structure arrays which may have unknown,
>>> to the caller, sizes. The purpose of it is to stop iteration over an array and avoid
>>> out-of-boundary access. Nevertheless, we may apply a bit more stricter rule to avoid
>>> potential, but unlike, event of adding the entry after terminator, already at compile time.
>>> This will be achieved by not putting comma at the end of terminator line"
>> This certainly applies to any new code.
>>
>> The existing code, however, is what it is and the question is how much
>> of an improvement the given change makes.
>>
>> So yes, it may not follow the current rules for new code, but then it
>> may not be worth changing to follow these rules anyway.
> This is a bit like housing in a city.
>
> Usually, there are strict requirements that must be followed while
> constructing a new building, but existing buildings are not
> reconstructed to follow them in the majority of cases.  It may not
> even be a good idea to do that.

Thanks, great explanation ! I think it's a shared sentiment among maintainers.
I've been watching upstreaming effort of intel new idpf driver, and it got rejected
basically because new drivers are held to a higher standard (they didn't modernize
their code to use new page pool API).

https://lore.kernel.org/netdev/20230621122106.56cb5bf1@xxxxxxxxxx/#t