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

From: Meiyong Yu
Date: Thu Mar 21 2024 - 00:40:00 EST



在 2024/3/21 8:11, Barry Song 写道:
On Thu, Mar 21, 2024 at 12:39 PM Meiyong Yu <meiyong.yu@xxxxxxx> wrote:

On Mar 20, 2024, at 08:17, Barry Song <21cnbao@xxxxxxxxx> wrote:

From: Barry Song <v-songbaohua@xxxxxxxx>

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 have a guidance in codingstyle to ask for the evaluation
of parameters.

Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Chris Zankel <chris@xxxxxxxxxx>
Cc: Huacai Chen <chenhuacai@xxxxxxxxxxx>
Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
Suggested-by: Max Filippov <jcmvbkbc@xxxxxxxxx>
Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
---
Documentation/process/coding-style.rst | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index 9c7cf7347394..8065747fddff 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -827,6 +827,13 @@ Macros with multiple statements should be enclosed in a do - while block:
do_this(b, c); \
} while (0)


+Function-like macros should evaluate their parameters, for unused parameters,
I do not support this point, if the parameter is unused, why not to remove it.

Linux boasts support for numerous architectures, striving for
independence in its
drivers and core code implementation across these architectures. Consequently,
certain architectures may utilize parameters for the same APIs, while others may
not.

So the probem is  designed api is not reasonable,  it use not essential paramter,

you can change the api, but not avoid it.

Anthor question, why you do not use the parameter, if not use it,  will trigger function/feature dismiss problem ?

about the warning, is tool misreport, the tool must make better

no. This is not the case.

+cast them to void:
+
+.. code-block:: c
+
+ #define macrofun(a) do { (void) (a); } while (0)
+
Things to avoid when using macros:

1) macros that affect control flow:
--
2.34.1