[RFC] Documentation: Document macro coding style
Commit Message
Document the kernel coding style for macros with parameters.
The purpose of this text is to be used as a reference to gradually
transition towards macros with a more consistent style, and eliminate
subtle bugs that can creep up due to missing parentheses, and generally
remove the need to think and argue about C operator precedence.
This is based on a mailing list discussion with Linus.
Link: https://lore.kernel.org/lkml/CAHk-=wjfgCa-u8h9z+8U7gaKK6PnRCpws1Md9wYSSXywUxoUSA@mail.gmail.com/
Link: https://lore.kernel.org/lkml/CAHk-=wjzpHjqhybyEhkTzGgTdBP3LZ1FmOw8=1MMXr=-j5OPxQ@mail.gmail.com/
Link: https://lore.kernel.org/lkml/CAHk-=wh-x1PL=UUGD__Dv6kd+kyCHjNF-TCHGG9ayLnysf-PdQ@mail.gmail.com/
Link: https://lore.kernel.org/lkml/CAHk-=wg27iiFZWYmjKmULxwkXisOHuAXq=vbiazBabgh9M1rqg@mail.gmail.com/
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
---
Documentation/process/coding-style.rst | 152 ++++++++++++++++++++++++-
1 file changed, 151 insertions(+), 1 deletion(-)
Comments
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
> Document the kernel coding style for macros with parameters.
>
> The purpose of this text is to be used as a reference to gradually
> transition towards macros with a more consistent style, and eliminate
> subtle bugs that can creep up due to missing parentheses, and generally
> remove the need to think and argue about C operator precedence.
>
> This is based on a mailing list discussion with Linus.
>
> Link: https://lore.kernel.org/lkml/CAHk-=wjfgCa-u8h9z+8U7gaKK6PnRCpws1Md9wYSSXywUxoUSA@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAHk-=wjzpHjqhybyEhkTzGgTdBP3LZ1FmOw8=1MMXr=-j5OPxQ@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAHk-=wh-x1PL=UUGD__Dv6kd+kyCHjNF-TCHGG9ayLnysf-PdQ@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAHk-=wg27iiFZWYmjKmULxwkXisOHuAXq=vbiazBabgh9M1rqg@mail.gmail.com/
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> ---
> Documentation/process/coding-style.rst | 152 ++++++++++++++++++++++++-
> 1 file changed, 151 insertions(+), 1 deletion(-)
So this looks generally OK to me. I really like to see some reviews /
acks on coding-style patches, though; I don't feel like I should be the
arbiter of kernel coding style.
One little comment below
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 6db37a46d305..3cf62c91d91c 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -819,10 +819,160 @@ Macros with multiple statements should be enclosed in a do - while block:
>
> #define macrofun(a, b, c) \
> do { \
> - if (a == 5) \
> + if ((a) == 5) \
> do_this(b, c); \
> } while (0)
>
> +Always use parentheses around macro arguments, except for the situations listed
> +below.
> +
> +Examples where parentheses are required around macro arguments:
> +
> +.. code-block:: c
> +
> + #define foo(a, b) \
> + do { \
> + (a) = (b); \
> + } while (0)
> +
> +.. code-block:: c
> +
> + #define foo(a) \
> + do { \
> + (a)++; \
> + } while (0)
> +
> +.. code-block:: c
> +
> + #define cmp_gt(a, b) ((a) > (b))
> +
> +.. code-block:: c
> +
> + #define foo(a) do_this(!(a))
> +
> +.. code-block:: c
> +
> + #define foo(a) do_this(*(a))
> +
> +.. code-block:: c
> +
> + #define foo(a) do_this(&(a))
> +
> +.. code-block:: c
> +
> + #define get_member(struct_var) do_this((struct_var).member)
> +
> +.. code-block:: c
> +
> + #define deref_member(struct_ptr) do_this((struct_ptr)->member)
I wonder if we really need to give all of these examples? We've already
said "always put parentheses except in a few cases" - I would think that
would be enough.
> +Situations where parentheses should not be added around arguments, when:
For these, it would be nice to say *why* parentheses shouldn't be added;
helping readers understand the reasoning might have more benefit than
imparting a set of rules.
Thanks,
jon
On 5/19/23 11:23, Jonathan Corbet wrote:
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
>
>> Document the kernel coding style for macros with parameters.
>>
>> The purpose of this text is to be used as a reference to gradually
>> transition towards macros with a more consistent style, and eliminate
>> subtle bugs that can creep up due to missing parentheses, and generally
>> remove the need to think and argue about C operator precedence.
>>
>> This is based on a mailing list discussion with Linus.
>>
>> Link: https://lore.kernel.org/lkml/CAHk-=wjfgCa-u8h9z+8U7gaKK6PnRCpws1Md9wYSSXywUxoUSA@mail.gmail.com/
>> Link: https://lore.kernel.org/lkml/CAHk-=wjzpHjqhybyEhkTzGgTdBP3LZ1FmOw8=1MMXr=-j5OPxQ@mail.gmail.com/
>> Link: https://lore.kernel.org/lkml/CAHk-=wh-x1PL=UUGD__Dv6kd+kyCHjNF-TCHGG9ayLnysf-PdQ@mail.gmail.com/
>> Link: https://lore.kernel.org/lkml/CAHk-=wg27iiFZWYmjKmULxwkXisOHuAXq=vbiazBabgh9M1rqg@mail.gmail.com/
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: linux-doc@vger.kernel.org
>> ---
>> Documentation/process/coding-style.rst | 152 ++++++++++++++++++++++++-
>> 1 file changed, 151 insertions(+), 1 deletion(-)
>
> So this looks generally OK to me. I really like to see some reviews /
> acks on coding-style patches, though; I don't feel like I should be the
> arbiter of kernel coding style.
Sure, I'll update the patch based on your comments and send without the RFC tag, we'll
see if we get reviews or ack.
Meanwhile there are two additional cases where I think we should mandate the parentheses
even though they are not strictly needed, because it eliminates corner-cases without
negatively impacting readability.
The first case is when an argument is used as declarator identifier, e.g.
#define DECLARE_WAITQUEUE(name, tsk) \
struct wait_queue_entry name = __WAITQUEUE_INITIALIZER(name, tsk)
^ here
Adding parentheses around "name" does not break anything, makes the code *more*
regular by following the general rule of adding parentheses around macro arguments
unless it is not possible for some reason.
I also think that we should mandate parentheses around initializers, even though
those are full expressions:
#define foo(a) \
do { \
int __m_var = a; \
^ here
} while (0)
Because requiring the (useless) parentheses here makes the code more consistent and
removes a corner-case to think about when writing code, without negatively impacting
readability.
This would basically leave only the following corner-cases. Please let me know if any
of them would be good candidates for requiring the extra parentheses nevertheless:
- For readability:
- "a;"
- "for (a; b; c)"
- "if (a)"
- "switch (a)"
- "while (a)"
- "do { ... } while (a)"
- "return a;"
- "array[a]",
- "f(a)",
- "f(a, b, c)",
Because it would not work otherwise:
- "(p)->a,
- "#a",
- "sym##a".
>
> One little comment below
>
>> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
>> index 6db37a46d305..3cf62c91d91c 100644
>> --- a/Documentation/process/coding-style.rst
>> +++ b/Documentation/process/coding-style.rst
>> @@ -819,10 +819,160 @@ Macros with multiple statements should be enclosed in a do - while block:
>>
>> #define macrofun(a, b, c) \
>> do { \
>> - if (a == 5) \
>> + if ((a) == 5) \
>> do_this(b, c); \
>> } while (0)
>>
>> +Always use parentheses around macro arguments, except for the situations listed
>> +below.
>> +
>> +Examples where parentheses are required around macro arguments:
>> +
>> +.. code-block:: c
>> +
>> + #define foo(a, b) \
>> + do { \
>> + (a) = (b); \
>> + } while (0)
>> +
>> +.. code-block:: c
>> +
>> + #define foo(a) \
>> + do { \
>> + (a)++; \
>> + } while (0)
>> +
>> +.. code-block:: c
>> +
>> + #define cmp_gt(a, b) ((a) > (b))
>> +
>> +.. code-block:: c
>> +
>> + #define foo(a) do_this(!(a))
>> +
>> +.. code-block:: c
>> +
>> + #define foo(a) do_this(*(a))
>> +
>> +.. code-block:: c
>> +
>> + #define foo(a) do_this(&(a))
>> +
>> +.. code-block:: c
>> +
>> + #define get_member(struct_var) do_this((struct_var).member)
>> +
>> +.. code-block:: c
>> +
>> + #define deref_member(struct_ptr) do_this((struct_ptr)->member)
>
> I wonder if we really need to give all of these examples? We've already
> said "always put parentheses except in a few cases" - I would think that
> would be enough.
OK, let's remove those examples for now. If it proves that it causes confusion
we can always add them back.
>
>> +Situations where parentheses should not be added around arguments, when:
>
> For these, it would be nice to say *why* parentheses shouldn't be added;
> helping readers understand the reasoning might have more benefit than
> imparting a set of rules.
OK, those would look like:
Always use parentheses around macro arguments, except when:
* they are used as a full expression and negatively impact code readability
(because the extra parentheses would not have any role in preserving operator
precedence, making them redundant):
[...]
* they are used as expression within an array subscript operator "[]" (because brackets
nest just like parentheses themselves do):
[...]
* they are used as arguments to functions or other macros (because the comma
separator between arguments is not even an operator, so there is no operator
precedence to preserve):
[...]
(note: the ones below include new additions)
* there is some syntax reason why adding the parentheses would not work, e.g.
when using the parameter as member name, for string expansion, or token
pasting:
* the ``member`` argument:
.. code-block:: c
#define foo(struct_p, member) do_this((struct_p)->member)
* string expansion:
.. code-block:: c
#define __stringify_1(x...) #x
#define __stringify(x...) __stringify_1(x)
* token pasting:
.. code-block:: c
#define COMPAT_SYSCALL_DEFINE1(name, ...) \
COMPAT_SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
Thanks!
Mathieu
>
> Thanks,
>
> jon
@@ -819,10 +819,160 @@ Macros with multiple statements should be enclosed in a do - while block:
#define macrofun(a, b, c) \
do { \
- if (a == 5) \
+ if ((a) == 5) \
do_this(b, c); \
} while (0)
+Always use parentheses around macro arguments, except for the situations listed
+below.
+
+Examples where parentheses are required around macro arguments:
+
+.. code-block:: c
+
+ #define foo(a, b) \
+ do { \
+ (a) = (b); \
+ } while (0)
+
+.. code-block:: c
+
+ #define foo(a) \
+ do { \
+ (a)++; \
+ } while (0)
+
+.. code-block:: c
+
+ #define cmp_gt(a, b) ((a) > (b))
+
+.. code-block:: c
+
+ #define foo(a) do_this(!(a))
+
+.. code-block:: c
+
+ #define foo(a) do_this(*(a))
+
+.. code-block:: c
+
+ #define foo(a) do_this(&(a))
+
+.. code-block:: c
+
+ #define get_member(struct_var) do_this((struct_var).member)
+
+.. code-block:: c
+
+ #define deref_member(struct_ptr) do_this((struct_ptr)->member)
+
+Situations where parentheses should not be added around arguments, when:
+
+* they are used as a full expression:
+
+ * as an initializer:
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ int __m_var = a; \
+ } while (0)
+
+ * as an expression statement:
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ a; \
+ } while (0)
+
+ * as the controlling expression of a selection statement (``if`` or ``switch``):
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ if (a) \
+ do_this(); \
+ } while (0)
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ switch (a) { \
+ case 1: do_this(); \
+ break; \
+ } \
+ } while (0)
+
+ * as the controlling expression of a ``while`` or ``do`` statement:
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ while (a) \
+ do_this(); \
+ } while (0)
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ do_this(); \
+ } while (a)
+
+ * as any of the expressions of a ``for`` statement:
+
+ .. code-block:: c
+
+ #define foo(a, b, c) \
+ do { \
+ for (a; b; c) \
+ do_this(); \
+ } while (0)
+
+ * as the expression in a return statement (note that use of return
+ statements in macros is strongly discouraged because it affects the control
+ flow),
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ return a; \
+ } while (0)
+
+* they are used as expression within an array subscript operator "[]":
+
+ .. code-block:: c
+
+ #define foo(a) \
+ do { \
+ if (array[a] == 1) \
+ do_this(); \
+ } while (0)
+
+* they are used as arguments to functions or other macros:
+
+ .. code-block:: c
+
+ #define foo(a) do_this(a)
+
+ .. code-block:: c
+
+ #define foo(a, b, c) do_this(a, b, c)
+
+* there is some syntax reason why adding the parentheses would not work, e.g.
+ the ``member`` argument:
+
+ .. code-block:: c
+
+ #define foo(struct_p, member) do_this((struct_p)->member)
+
Things to avoid when using macros:
1) macros that affect control flow: