[PATCH v5 0/2] codingstyle: avoid unused parameters for a function-like macro

[PATCH v5 0/2] codingstyle: avoid unused parameters for a function-like macro

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

-v5:
 * Simplify the code for Patch 2 according to Joe's suggestions.
 * add s-o-b of Barry according to Jeff Johnson

-v4:
 * fix Xining's email address, s/ma.xxn@outlook.com/mac.xxn@outlook.com/g
 * fix some false positives of checkpatch.pl
 * downgrade from ERROR to WARN in checkpatch.pl

 Thanks for Joe's comments!

 v4 link: https://lore.kernel.org/all/20240328022136.5789-1-21cnbao@gmail.com/

-v3:
 https://lore.kernel.org/all/20240322084937.66018-1-21cnbao@gmail.com/

A function-like macro could result in build warnings such as
"unused variable." This patchset updates the guidance to
recommend always using a static inline function instead
and also provides checkpatch support for this new rule.

Barry Song (1):
  Documentation: coding-style: ask function-like macros to evaluate
    parameters

Xining Xu (1):
  scripts: checkpatch: check unused parameters for function-like macro

 Documentation/process/coding-style.rst | 16 ++++++++++++++++
 scripts/checkpatch.pl                  |  6 ++++++
 2 files changed, 22 insertions(+)

-- 
2.34.1

[PATCH v5 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

Recent commit 77292bb8ca69c80 ("crypto: scomp - remove memcpy if
sg_nents is 1 and pages are lowmem") leads to warnings on xtensa
and loongarch,
   In file included from crypto/scompress.c:12:
   include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
   include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
      76 |                 struct page *page;
         |                              ^~~~
   crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
     174 |                         struct page *dst_page = sg_page(req->dst);
         |

The reason is that flush_dcache_page() is implemented as a noop
macro on these platforms as below,

 #define flush_dcache_page(page) do { } while (0)

The driver code, for itself, seems be quite innocent and placing
maybe_unused seems pointless,

 struct page *dst_page = sg_page(req->dst);

 for (i = 0; i < nr_pages; i++)
 	flush_dcache_page(dst_page + i);

And it should be independent of architectural implementation
differences.

Let's provide guidance on coding style for requesting parameter
evaluation or proposing the migration to a static inline
function.

Signed-off-by: Barry Song <v-songbaohua@oppo.com>
Suggested-by: Max Filippov <jcmvbkbc@gmail.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Cc: Chris Zankel <chris@zankel.net>
Cc: Huacai Chen <chenhuacai@loongson.cn>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Xining Xu <mac.xxn@outlook.com>
---
 Documentation/process/coding-style.rst | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index 9c7cf7347394..791d333a57fd 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -827,6 +827,22 @@ Macros with multiple statements should be enclosed in a do - while block:
 				do_this(b, c);		\
 		} while (0)
 
+Function-like macros with unused parameters should be replaced by static
+inline functions to avoid the issue of unused variables:
+
+.. code-block:: c
+
+	static inline void fun(struct foo *foo)
+	{
+	}
+
+For historical reasons, many files still use the cast to (void) to evaluate
+parameters, but this method is not recommended:
+
+.. code-block:: c
+
+	#define macrofun(foo) do { (void) (foo); } while (0)
+
 Things to avoid when using macros:
 
 1) macros that affect control flow:
-- 
2.34.1

[PATCH v5 2/2] scripts: checkpatch: check unused parameters for function-like macro

[Barry Song]

From: Xining Xu <mac.xxn@outlook.com>

If function-like macros do not utilize a parameter, it might result in a
build warning.  In our coding style guidelines, we advocate for utilizing
static inline functions to replace such macros.  This patch verifies
compliance with the new rule.

For a macro such as the one below,

 #define test(a) do { } while (0)

The test result is as follows.

 ERROR: Parameter 'a' is not used in function-like macro, please use static
 inline instead
 #21: FILE: mm/init-mm.c:20:
 +#define test(a) do { } while (0)

 total: 1 errors, 0 warnings, 8 lines checked

Signed-off-by: Xining Xu <mac.xxn@outlook.com>
Tested-by: Barry Song <v-songbaohua@oppo.com>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Huacai Chen <chenhuacai@loongson.cn>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Mark Brown <broonie@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Charlemagne Lasse <charlemagnelasse@gmail.com>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..9895d7e38a9f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6040,6 +6040,12 @@ sub process {
 					CHK("MACRO_ARG_PRECEDENCE",
 					    "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx");
 				}
+
+# check if this is an unused argument
+				if ($define_stmt !~ /\b$arg\b/) {
+					WARN("MACRO_ARG_UNUSED",
+						"Argument '$arg' is not used in function-like macro\n" . "$herectx");
+				}
 			}
 
 # check for macros with flow control, but without ## concatenation
-- 
2.34.1

Re: [PATCH v5 2/2] scripts: checkpatch: check unused parameters for function-like macro

[Joe Perches]

On Mon, 2024-04-01 at 14:21 +1300, Barry Song wrote:
> From: Xining Xu <mac.xxn@outlook.com>
> 
> If function-like macros do not utilize a parameter, it might result in a
> build warning.  In our coding style guidelines, we advocate for utilizing
> static inline functions to replace such macros.  This patch verifies
> compliance with the new rule.
> 
> For a macro such as the one below,
> 
>  #define test(a) do { } while (0)
> 
> The test result is as follows.
> 
>  ERROR: Parameter 'a' is not used in function-like macro, please use static
>  inline instead
>  #21: FILE: mm/init-mm.c:20:
>  +#define test(a) do { } while (0)

This is no longer true.
Please update the ERROR->WARN and message as below

Ideally, this would have an update to
Documentation/dev-tools/checkpatch.rst

to describe the new --verbose message type

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6040,6 +6040,12 @@ sub process {
>  					CHK("MACRO_ARG_PRECEDENCE",
>  					    "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx");
>  				}
> +
> +# check if this is an unused argument
> +				if ($define_stmt !~ /\b$arg\b/) {
> +					WARN("MACRO_ARG_UNUSED",
> +						"Argument '$arg' is not used in function-like macro\n" . "$herectx");
> +				}

Re: [PATCH v5 2/2] scripts: checkpatch: check unused parameters for function-like macro

[Mac Xu]

> On Mon, 2024-04-01 at 14:21 +1300, Barry Song wrote:
> > From: Xining Xu <mac.xxn@outlook.com>
> >
> > If function-like macros do not utilize a parameter, it might result in a
> > build warning.  In our coding style guidelines, we advocate for utilizing
> > static inline functions to replace such macros.  This patch verifies
> > compliance with the new rule.
> >
> > For a macro such as the one below,
> >
> >  #define test(a) do { } while (0)
> >
> > The test result is as follows.
> >
> >  ERROR: Parameter 'a' is not used in function-like macro, please use static
> >  inline instead
> >  #21: FILE: mm/init-mm.c:20:
> >  +#define test(a) do { } while (0)
> 
> This is no longer true.
> Please update the ERROR->WARN and message as below
> 
> Ideally, this would have an update to
> Documentation/dev-tools/checkpatch.rst
> 
> to describe the new --verbose message type

Hi Joe,

Thank you for the comments, here's the code:

+# check if this is an unused argument
+if ($define_stmt !~ /\b$arg\b/) {
+	WARN("MACRO_ARG_UNUSED",
+		"Argument '$arg' is not used in function-like macro\n" . "$herectx");
+}

and here's the document for it which is inserted into the "Macros, Attributes and
Symbols" section of checkpatch.rst starting from line 909:
+
+  **MACRO_ARG_UNUSED**
+    If function-like macros do not utilize a parameter, it might result
+    in a build warning. We advocate for utilizing static inline functions
+    to replace such macros.
+    For example, for a macro as below::
+
+      #define test(a) do { } while (0)
+
+    there would be a warning as below::
+
+      WARNING: Parameter 'a' is not used in function-like macro, please use
+      static inline instead.

Please let me know if the document needs further re-wording to make it helpful enough
to the readers.

Thanks,
Xining

Re: [PATCH v5 2/2] scripts: checkpatch: check unused parameters for function-like macro

[Joe Perches]

On Tue, 2024-04-02 at 00:16 +0000, Mac Xu wrote:
> > On Mon, 2024-04-01 at 14:21 +1300, Barry Song wrote:
> > > From: Xining Xu <mac.xxn@outlook.com>
> > > 
> > > If function-like macros do not utilize a parameter, it might result in a
> > > build warning.  In our coding style guidelines, we advocate for utilizing
> > > static inline functions to replace such macros.  This patch verifies
> > > compliance with the new rule.
> > > 
> > > For a macro such as the one below,
> > > 
> > >  #define test(a) do { } while (0)
> > > 
> > > The test result is as follows.
> > > 
> > >  ERROR: Parameter 'a' is not used in function-like macro, please use static
> > >  inline instead
> > >  #21: FILE: mm/init-mm.c:20:
> > >  +#define test(a) do { } while (0)
> > 
> > This is no longer true.
> > Please update the ERROR->WARN and message as below
> > 
> > Ideally, this would have an update to
> > Documentation/dev-tools/checkpatch.rst
> > 
> > to describe the new --verbose message type
> 
> Hi Joe,
> 
> Thank you for the comments, here's the code:
> 
> +# check if this is an unused argument
> +if ($define_stmt !~ /\b$arg\b/) {
> +	WARN("MACRO_ARG_UNUSED",
> +		"Argument '$arg' is not used in function-like macro\n" . "$herectx");
> +}
> 
> and here's the document for it which is inserted into the "Macros, Attributes and
> Symbols" section of checkpatch.rst starting from line 909:
> +
> +  **MACRO_ARG_UNUSED**
> +    If function-like macros do not utilize a parameter, it might result
> +    in a build warning. We advocate for utilizing static inline functions
> +    to replace such macros.
> +    For example, for a macro as below::
> +
> +      #define test(a) do { } while (0)
> +
> +    there would be a warning as below::
> +
> +      WARNING: Parameter 'a' is not used in function-like macro, please use
> +      static inline instead.
> 
> Please let me know if the document needs further re-wording to make it helpful enough
> to the readers.

Hi again Xining.

Thanks.

That looks good but it doesn't match the script output
which doesn't use ", please use static inline instead."
(and I believe the script should not output that too)

Another good thing would be to add a line like:

	See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl

For example, from: checkpatch.rst

  **ALLOC_SIZEOF_STRUCT**
    The allocation style is bad.  In general for family of
    allocation functions using sizeof() to get memory size,
    constructs like::

      p = alloc(sizeof(struct foo), ...)

    should be::

      p = alloc(sizeof(*p), ...)

    See: https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating-memory

Re: [PATCH v5 2/2] scripts: checkpatch: check unused parameters for function-like macro

[Mac Xu]

> On Tue, 2024-04-02 at 00:16 +0000, Mac Xu wrote:
> > > On Mon, 2024-04-01 at 14:21 +1300, Barry Song wrote:
> > > > From: Xining Xu <mac.xxn@outlook.com>
> > > >
> > > > If function-like macros do not utilize a parameter, it might result in a
> > > > build warning.  In our coding style guidelines, we advocate for utilizing
> > > > static inline functions to replace such macros.  This patch verifies
> > > > compliance with the new rule.
> > > >
> > > > For a macro such as the one below,
> > > >
> > > >  #define test(a) do { } while (0)
> > > >
> > > > The test result is as follows.
> > > >
> > > >  ERROR: Parameter 'a' is not used in function-like macro, please use static
> > > >  inline instead
> > > >  #21: FILE: mm/init-mm.c:20:
> > > >  +#define test(a) do { } while (0)
> > >
> > > This is no longer true.
> > > Please update the ERROR->WARN and message as below
> > >
> > > Ideally, this would have an update to
> > > Documentation/dev-tools/checkpatch.rst
> > >
> > > to describe the new --verbose message type
> >
> > Hi Joe,
> >
> > Thank you for the comments, here's the code:
> >
> > +# check if this is an unused argument
> > +if ($define_stmt !~ /\b$arg\b/) {
> > +     WARN("MACRO_ARG_UNUSED",
> > +             "Argument '$arg' is not used in function-like macro\n" . "$herectx");
> > +}
> >
> > and here's the document for it which is inserted into the "Macros, Attributes and
> > Symbols" section of checkpatch.rst starting from line 909:
> > +
> > +  **MACRO_ARG_UNUSED**
> > +    If function-like macros do not utilize a parameter, it might result
> > +    in a build warning. We advocate for utilizing static inline functions
> > +    to replace such macros.
> > +    For example, for a macro as below::
> > +
> > +      #define test(a) do { } while (0)
> > +
> > +    there would be a warning as below::
> > +
> > +      WARNING: Parameter 'a' is not used in function-like macro, please use
> > +      static inline instead.
> >
> > Please let me know if the document needs further re-wording to make it helpful enough
> > to the readers.
> 
> Hi again Xining.
> 
> Thanks.
> 
> That looks good but it doesn't match the script output
> which doesn't use ", please use static inline instead."
> (and I believe the script should not output that too)
> 
> Another good thing would be to add a line like:
> 
>         See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
> 
> For example, from: checkpatch.rst
> 
>   **ALLOC_SIZEOF_STRUCT**
>     The allocation style is bad.  In general for family of
>     allocation functions using sizeof() to get memory size,
>     constructs like::
> 
>       p = alloc(sizeof(struct foo), ...)
> 
>     should be::
> 
>       p = alloc(sizeof(*p), ...)
> 
>     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating-memory
> 
> 

Hi again Joe,

Thanks again for the detailed comments.

Here's the code (which keeps unchanged):
+# check if this is an unused argument
+if ($define_stmt !~ /\b$arg\b/) {
+  WARN("MACRO_ARG_UNUSED",
+    "Argument '$arg' is not used in function-like macro\n" . "$herectx");
+}

And here's the updated document, which drops the ", please use static inline instead" from the
warning message, and adds a link at the end of this document block:
+
+  **MACRO_ARG_UNUSED**
+    If function-like macros do not utilize a parameter, it might result
+    in a build warning. We advocate for utilizing static inline functions
+    to replace such macros.
+    For example, for a macro such as the one below::
+
+      #define test(a) do { } while (0)
+
+    there would be a warning like below::
+
+      WARNING: Parameter 'a' is not used in function-like macro.
+
+    See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl


Regards,
Xining.

Re: [PATCH v5 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters

[Jonathan Corbet]

So I'm not sure what your desired path for getting this upstream is.  I
can take it, but I'm generally quite leery of taking coding-style
changes without some serious acks on them - nobody elected me as the
arbiter of proper coding style.

A nit below

Barry Song <21cnbao@gmail.com> writes:

> From: Barry Song <v-songbaohua@oppo.com>
>
> Recent commit 77292bb8ca69c80 ("crypto: scomp - remove memcpy if
> sg_nents is 1 and pages are lowmem") leads to warnings on xtensa
> and loongarch,
>    In file included from crypto/scompress.c:12:
>    include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
>    include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
>       76 |                 struct page *page;
>          |                              ^~~~
>    crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
>>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
>      174 |                         struct page *dst_page = sg_page(req->dst);
>          |
>
> The reason is that flush_dcache_page() is implemented as a noop
> macro on these platforms as below,
>
>  #define flush_dcache_page(page) do { } while (0)
>
> The driver code, for itself, seems be quite innocent and placing
> maybe_unused seems pointless,
>
>  struct page *dst_page = sg_page(req->dst);
>
>  for (i = 0; i < nr_pages; i++)
>  	flush_dcache_page(dst_page + i);
>
> And it should be independent of architectural implementation
> differences.
>
> Let's provide guidance on coding style for requesting parameter
> evaluation or proposing the migration to a static inline
> function.
>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> Suggested-by: Max Filippov <jcmvbkbc@gmail.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Huacai Chen <chenhuacai@loongson.cn>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Andy Whitcroft <apw@canonical.com>
> Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Cc: Xining Xu <mac.xxn@outlook.com>
> ---
>  Documentation/process/coding-style.rst | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 9c7cf7347394..791d333a57fd 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -827,6 +827,22 @@ Macros with multiple statements should be enclosed in a do - while block:
>  				do_this(b, c);		\
>  		} while (0)
>  
> +Function-like macros with unused parameters should be replaced by static
> +inline functions to avoid the issue of unused variables:
> +
> +.. code-block:: c

I would just use the "::" notation here; the ..code-block:: just adds
noise IMO.

> +	static inline void fun(struct foo *foo)
> +	{
> +	}
> +
> +For historical reasons, many files still use the cast to (void) to evaluate
> +parameters, but this method is not recommended:
> +
> +.. code-block:: c
> +
> +	#define macrofun(foo) do { (void) (foo); } while (0)
> +

1) If you're putting in examples of something *not* to do, it's probably
better to also put in something like:

   /* don't do this */

people don't always read closely.

2) Can we say *why* it's not recommended?

Thanks,

jon

Re: [PATCH v5 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters

[Barry Song]

On Wed, Apr 3, 2024 at 5:13 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> So I'm not sure what your desired path for getting this upstream is.  I
> can take it, but I'm generally quite leery of taking coding-style
> changes without some serious acks on them - nobody elected me as the
> arbiter of proper coding style.

Hi Jonathan,
Thanks!
Andrew previously integrated it into mm-nomm and tagged it as [TO-BE-UPDATED]
before removing it a few days back. Here's the link:
https://lore.kernel.org/mm-commits/20240330025857.CD609C433F1@smtp.kernel.org/
So if feasible, I'd prefer to stick with Andrew's channel.

>
> A nit below
>
> Barry Song <21cnbao@gmail.com> writes:
>
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > Recent commit 77292bb8ca69c80 ("crypto: scomp - remove memcpy if
> > sg_nents is 1 and pages are lowmem") leads to warnings on xtensa
> > and loongarch,
> >    In file included from crypto/scompress.c:12:
> >    include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
> >    include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
> >       76 |                 struct page *page;
> >          |                              ^~~~
> >    crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
> >>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
> >      174 |                         struct page *dst_page = sg_page(req->dst);
> >          |
> >
> > The reason is that flush_dcache_page() is implemented as a noop
> > macro on these platforms as below,
> >
> >  #define flush_dcache_page(page) do { } while (0)
> >
> > The driver code, for itself, seems be quite innocent and placing
> > maybe_unused seems pointless,
> >
> >  struct page *dst_page = sg_page(req->dst);
> >
> >  for (i = 0; i < nr_pages; i++)
> >       flush_dcache_page(dst_page + i);
> >
> > And it should be independent of architectural implementation
> > differences.
> >
> > Let's provide guidance on coding style for requesting parameter
> > evaluation or proposing the migration to a static inline
> > function.
> >
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > Suggested-by: Max Filippov <jcmvbkbc@gmail.com>
> > Reviewed-by: Mark Brown <broonie@kernel.org>
> > Cc: Chris Zankel <chris@zankel.net>
> > Cc: Huacai Chen <chenhuacai@loongson.cn>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Andy Whitcroft <apw@canonical.com>
> > Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > Cc: Joe Perches <joe@perches.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> > Cc: Xining Xu <mac.xxn@outlook.com>
> > ---
> >  Documentation/process/coding-style.rst | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> > index 9c7cf7347394..791d333a57fd 100644
> > --- a/Documentation/process/coding-style.rst
> > +++ b/Documentation/process/coding-style.rst
> > @@ -827,6 +827,22 @@ Macros with multiple statements should be enclosed in a do - while block:
> >                               do_this(b, c);          \
> >               } while (0)
> >
> > +Function-like macros with unused parameters should be replaced by static
> > +inline functions to avoid the issue of unused variables:
> > +
> > +.. code-block:: c
>
> I would just use the "::" notation here; the ..code-block:: just adds
> noise IMO.

I am not quite sure we want this. as reading the whole coding-style.rst,
.. code-block:: c is everywhere :-)   Should I do something different or
just follow the tradition?

>
> > +     static inline void fun(struct foo *foo)
> > +     {
> > +     }
> > +
> > +For historical reasons, many files still use the cast to (void) to evaluate
> > +parameters, but this method is not recommended:
> > +
> > +.. code-block:: c
> > +
> > +     #define macrofun(foo) do { (void) (foo); } while (0)
> > +
>
> 1) If you're putting in examples of something *not* to do, it's probably
> better to also put in something like:
>
>    /* don't do this */
>
> people don't always read closely.

ok.

>
> 2) Can we say *why* it's not recommended?
>

Andrew makes a valid point.

https://lore.kernel.org/all/20240321104427.730b859087afecf5973d1c58@linux-foundation.org/

"I think so.  My overall view is that we should write things in C.  Only
use macros if the thing we're trying to do simply cannot be done in a C
function.

- inline functions don't have the "expression with side effects
  evaluated more than once" problem.

- inline functions avoid the unused-variable issue which started this thread

- inline functions look better

- for some reason, people are more inclined to document inline
  functions than macros."

Andrew's point seems too lengthy for inclusion in the coding-style.rst document?
I'll attempt to condense it.

> Thanks,
>
> jon

Thanks
Barry

Re: [PATCH v5 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters

[Joe Perches]

On Wed, 2024-04-03 at 10:21 +1300, Barry Song wrote:
> On Wed, Apr 3, 2024 at 5:13 AM Jonathan Corbet <corbet@lwn.net> wrote:
> > 
> > So I'm not sure what your desired path for getting this upstream is.  I
> > can take it, but I'm generally quite leery of taking coding-style
> > changes without some serious acks on them - nobody elected me as the
> > arbiter of proper coding style.

I believe it is generally appropriate for macros that take
arguments to use static inlines where feasible, so:

Acked-by: Joe Perches <joe@perches.com>

And yes, mm is the usual path for upstreaming at least this
sort of checkpatch change.