From 6813216bbdba18e182759d949589be95ebef290f Mon Sep 17 00:00:00 2001 From: Barry Song <v-songbaohua@oppo.com> Date: Tue, 7 May 2024 15:27:56 +1200 Subject: [PATCH] Documentation: coding-style: ask function-like macros to evaluate parameters Patch series "codingstyle: avoid unused parameters for a function-like macro", v7. 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. This patch (of 2): 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. Link: https://lkml.kernel.org/r/20240507032757.146386-1-21cnbao@gmail.com Link: https://lkml.kernel.org/r/20240507032757.146386-2-21cnbao@gmail.com Signed-off-by: Barry Song <v-songbaohua@oppo.com> Suggested-by: Max Filippov <jcmvbkbc@gmail.com> Reviewed-by: Mark Brown <broonie@kernel.org> Acked-by: Joe Perches <joe@perches.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: 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> Cc: Charlemagne Lasse <charlemagnelasse@gmail.com> Cc: Jeff Johnson <quic_jjohnson@quicinc.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- Documentation/process/coding-style.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst index 9c7cf73473943..7e768c65aa926 100644 --- a/Documentation/process/coding-style.rst +++ b/Documentation/process/coding-style.rst @@ -827,6 +827,29 @@ 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) + { + } + +Due to historical practices, many files still employ the "cast to (void)" +approach to evaluate parameters. However, this method is not advisable. +Inline functions address the issue of "expression with side effects +evaluated more than once", circumvent unused-variable problems, and +are generally better documented than macros for some reason. + +.. code-block:: c + + /* + * Avoid doing this whenever possible and instead opt for static + * inline functions + */ + #define macrofun(foo) do { (void) (foo); } while (0) + Things to avoid when using macros: 1) macros that affect control flow: -- GitLab