[2/4] coding-style: show how reusing macros prevents naming collisions

  In section "18) Don't re-invent the kernel macros" in "Linux kernel
coding style":

Show how reusing macros from shared headers prevents naming collisions
using "stringify", the one of the most widely reinvented macro, as an
example.

This patch aims to provide a stronger reason to reuse shared macros,
by showing the risk of improvised macro variants.

Signed-off-by: Yueh-Shun Li <shamrocklee@posteo.net>
---
 Documentation/process/coding-style.rst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
  

Yueh-Shun Li <shamrocklee@posteo.net> writes:

> In section "18) Don't re-invent the kernel macros" in "Linux kernel
> coding style":
>
> Show how reusing macros from shared headers prevents naming collisions
> using "stringify", the one of the most widely reinvented macro, as an
> example.
>
> This patch aims to provide a stronger reason to reuse shared macros,
> by showing the risk of improvised macro variants.
>
> Signed-off-by: Yueh-Shun Li <shamrocklee@posteo.net>
> ---
>  Documentation/process/coding-style.rst | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 2504cb00a961..1e79aba4b346 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -1070,6 +1070,28 @@ Similarly, if you need to calculate the size of some structure member, use
>  There are also ``min()`` and ``max()`` macros in ``include/linux/minmax.h``
>  that do strict type checking if you need them.
>  
> +Using existing macros provided by the shared headers also prevents naming
> +collisions. For example, if one developer define in ``foo.h``
> +
> +.. code-block:: c
> +
> +	#define __stringify(x) __stringify_1(x)
> +	#define __stringify_1(x) #x
> +
> +and another define in ``bar.h``
> +
> +.. code-block:: c
> +
> +	#define stringify(x) __stringify(x)
> +	#define __stringify(x) #x
> +
> +When both headers are ``#include``-d into the same file, the facilities provided
> +by ``foo.h`` might be broken by ``bar.h``.
> +
> +If both ``foo.h`` and ``bar.h``  use the macro ``__stringify()`` provided by
> +``include/linux/stringify.h``, they wouldn't have stepped onto each other's
> +toes.
> +

So everything we add to our documentation has a cost in terms of reader
attention.  We ask people to read through a lot of material now, and
should only increase that ask for good reason.

With that context, I have to wonder whether we really need to tell our
readers, who are supposed to be capable developers, that reuse can help
to avoid name collisions?

Thanks,

jon
  

Dear Mr. Corbet,

Thank you very much for your feed back.

On 2024-01-09 00:28, Jonathan Corbet wrote:
> Yueh-Shun Li <shamrocklee@posteo.net> writes:
> 
>> In section "18) Don't re-invent the kernel macros" in "Linux kernel
>> coding style":
>> 
>> Show how reusing macros from shared headers prevents naming collisions
>> using "stringify", the one of the most widely reinvented macro, as an
>> example.
>> 
>> This patch aims to provide a stronger reason to reuse shared macros,
>> by showing the risk of improvised macro variants.
>> 
>> Signed-off-by: Yueh-Shun Li <shamrocklee@posteo.net>
>> ---
>>  Documentation/process/coding-style.rst | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>> 
>> diff --git a/Documentation/process/coding-style.rst 
>> b/Documentation/process/coding-style.rst
>> index 2504cb00a961..1e79aba4b346 100644
>> --- a/Documentation/process/coding-style.rst
>> +++ b/Documentation/process/coding-style.rst
>> @@ -1070,6 +1070,28 @@ Similarly, if you need to calculate the size of 
>> some structure member, use
>>  There are also ``min()`` and ``max()`` macros in 
>> ``include/linux/minmax.h``
>>  that do strict type checking if you need them.
>> 
>> +Using existing macros provided by the shared headers also prevents 
>> naming
>> +collisions. For example, if one developer define in ``foo.h``
>> +
>> +.. code-block:: c
>> +
>> +	#define __stringify(x) __stringify_1(x)
>> +	#define __stringify_1(x) #x
>> +
>> +and another define in ``bar.h``
>> +
>> +.. code-block:: c
>> +
>> +	#define stringify(x) __stringify(x)
>> +	#define __stringify(x) #x
>> +
>> +When both headers are ``#include``-d into the same file, the 
>> facilities provided
>> +by ``foo.h`` might be broken by ``bar.h``.
>> +
>> +If both ``foo.h`` and ``bar.h``  use the macro ``__stringify()`` 
>> provided by
>> +``include/linux/stringify.h``, they wouldn't have stepped onto each 
>> other's
>> +toes.
>> +
> 
> So everything we add to our documentation has a cost in terms of reader
> attention.  We ask people to read through a lot of material now, and
> should only increase that ask for good reason.
> 
> With that context, I have to wonder whether we really need to tell our
> readers, who are supposed to be capable developers, that reuse can help
> to avoid name collisions?
> 

The motivation comes from existing inconsistency of the "__stringify()" 
macro
definition between e.g. "samples/bpf/tracex5.bpf.c" and other files.

I agree that increasing the length of the documentation without 
substantial
benefits would not be helpful for the readers, and doubling the length 
of a
section is too much for its purpose.

Should I shorten it into one sentence, like

```
On the other hand, locally-defined variants, such as ``#define 
__stringify(x) #x``,
could lead to naming collisions that break otherwise functioning 
facilities.
```

or just omit it in the next version of patches?

> Thanks,
> 
> jon

Thank you for your time and guidance.

Shamrock
  

Yueh-Shun Li <shamrocklee@posteo.net> writes:

>> So everything we add to our documentation has a cost in terms of reader
>> attention.  We ask people to read through a lot of material now, and
>> should only increase that ask for good reason.
>> 
>> With that context, I have to wonder whether we really need to tell our
>> readers, who are supposed to be capable developers, that reuse can help
>> to avoid name collisions?
>> 
>
> The motivation comes from existing inconsistency of the "__stringify()" 
> macro
> definition between e.g. "samples/bpf/tracex5.bpf.c" and other files.
>
> I agree that increasing the length of the documentation without
> substantial benefits would not be helpful for the readers, and
> doubling the length of a section is too much for its purpose.
>
> Should I shorten it into one sentence, like
>
> ```
> On the other hand, locally-defined variants, such as ``#define 
> __stringify(x) #x``,
> could lead to naming collisions that break otherwise functioning 
> facilities.
> ```
>
> or just omit it in the next version of patches?

My own feeling (others may well disagree) is that this isn't worth
mentioning in the coding-style document.  What you *could* do is to fix
the redefinitions (if that hasn't happened yet) and make sure that the
macros in question are covered in our kernel documentation.

Thanks,

jon