Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jonathan Corbet <corbet@lwn.net>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
Cc:     linux-kernel@vger.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Steven Rostedt <rostedt@goodmis.org>, linux-doc@vger.kernel.org
Subject: Re: [RFC PATCH] Documentation: Document macro coding style
In-Reply-To: <20230511152951.1970870-1-mathieu.desnoyers@efficios.com>
References: <20230511152951.1970870-1-mathieu.desnoyers@efficios.com>
Date:   Fri, 19 May 2023 09:23:48 -0600
Message-ID: <87y1lkmjjv.fsf@meer.lwn.net>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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