Re: [RFC PATCH] Documentation: Document macro coding style

[Jonathan Corbet]

Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/lkml/CAHk-=wjzpHjqhybyEhkTzGgTdBP3LZ1FmOw8=1MMXr=-j5OPxQ@xxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/lkml/CAHk-=wh-x1PL=UUGD__Dv6kd+kyCHjNF-TCHGG9ayLnysf-PdQ@xxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/lkml/CAHk-=wg27iiFZWYmjKmULxwkXisOHuAXq=vbiazBabgh9M1rqg@xxxxxxxxxxxxxx/
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Jonathan Corbet <corbet@xxxxxxx>
> Cc: linux-doc@xxxxxxxxxxxxxxx
> ---
>  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