* [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro
@ 2024-03-28 2:21 Barry Song
2024-03-28 2:21 ` [PATCH v4 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters Barry Song
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Barry Song @ 2024-03-28 2:21 UTC (permalink / raw)
To: akpm, linux-doc
Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert,
joe, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr,
v-songbaohua, workflows
From: Barry Song <v-songbaohua@oppo.com>
-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!
-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 | 30 ++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v4 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters 2024-03-28 2:21 [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro Barry Song @ 2024-03-28 2:21 ` Barry Song 2024-03-28 2:21 ` [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro Barry Song 2024-03-28 4:22 ` [PATCH v4 0/2] codingstyle: avoid unused parameters for a " Barry Song 2 siblings, 0 replies; 11+ messages in thread From: Barry Song @ 2024-03-28 2:21 UTC (permalink / raw) To: akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, joe, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr, v-songbaohua, workflows, Max Filippov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 2:21 [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro Barry Song 2024-03-28 2:21 ` [PATCH v4 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters Barry Song @ 2024-03-28 2:21 ` Barry Song 2024-03-28 9:57 ` Joe Perches 2024-03-28 16:01 ` Jeff Johnson 2024-03-28 4:22 ` [PATCH v4 0/2] codingstyle: avoid unused parameters for a " Barry Song 2 siblings, 2 replies; 11+ messages in thread From: Barry Song @ 2024-03-28 2:21 UTC (permalink / raw) To: akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, joe, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr, v-songbaohua, workflows, Max Filippov 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> 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> --- scripts/checkpatch.pl | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9c4c4a61bc83..bcb886014d60 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6109,6 +6109,36 @@ sub process { WARN("TRAILING_SEMICOLON", "macros should not use a trailing semicolon\n" . "$herectx"); } + + # match "\s*" rather than "\s+" after the balanced parens, as macro definition with arguments + # is not required to have whitespace after arguments + if ($dstat =~ /^\+\s*#\s*define\s+$Ident$balanced_parens\s*(\S+.*)(\/[\/*].*)?/) { + my $params = $1 || ""; + my $body = $2 || ""; + + # get the individual params + $params =~ tr/()//d; + # remove leading and trailing whitespace + $params =~ s/^\s+|\s+$//g; + + $ctx =~ s/\n*$//; + my $cnt = statement_rawlines($ctx); + my $herectx = get_stat_here($linenr, $cnt, $here); + + if ($params ne "") { + my @paramList = split /,\s*/, $params; + foreach my $param(@paramList) { + if ($param =~ /\.\.\.$/) { + # if the param name ends with "...", skip the check + next; + } + if ($body !~ /\b$param\b/) { + WARN("UNUSED_PARAM_IN_MACRO", + "Parameter '$param' is not used in function-like macro\n" . "$herectx"); + } + } + } + } } # check for redundant bracing round if etc -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 2:21 ` [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro Barry Song @ 2024-03-28 9:57 ` Joe Perches 2024-03-31 13:43 ` Mac Xu 2024-03-31 13:46 ` Mac Xu 2024-03-28 16:01 ` Jeff Johnson 1 sibling, 2 replies; 11+ messages in thread From: Joe Perches @ 2024-03-28 9:57 UTC (permalink / raw) To: Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr, v-songbaohua, workflows, Max Filippov On Thu, 2024-03-28 at 15: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. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -6109,6 +6109,36 @@ sub process { > WARN("TRAILING_SEMICOLON", > "macros should not use a trailing semicolon\n" . "$herectx"); > } > + > + # match "\s*" rather than "\s+" after the balanced parens, as macro definition with arguments > + # is not required to have whitespace after arguments > + if ($dstat =~ /^\+\s*#\s*define\s+$Ident$balanced_parens\s*(\S+.*)(\/[\/*].*)?/) { I think '(\/[\/*].*)?' doesn't do what you expect perhaps '(\/[\/\*].*)?' though I don't know why this should be capture group > + my $params = $1 || ""; > + my $body = $2 || ""; Should never get the || "" as the 2nd capture group is not optional > + > + # get the individual params > + $params =~ tr/()//d; > + # remove leading and trailing whitespace > + $params =~ s/^\s+|\s+$//g; > + > + $ctx =~ s/\n*$//; > + my $cnt = statement_rawlines($ctx); > + my $herectx = get_stat_here($linenr, $cnt, $here); > + > + if ($params ne "") { probably unnecessary > + my @paramList = split /,\s*/, $params; please use split() with parentheses > + foreach my $param(@paramList) { maybe foreach my $param (split(/,/, $params) { $param = trim($param); next if ($param =~ /\.\.\.$/); > + if ($param =~ /\.\.\.$/) { > + # if the param name ends with "...", skip the check > + next; > + } > + if ($body !~ /\b$param\b/) { > + WARN("UNUSED_PARAM_IN_MACRO", > + "Parameter '$param' is not used in function-like macro\n" . "$herectx"); > + } > + } It seems this logic is a bit redundant to existing code and might be better added in the block that starts (line 6026) # check if any macro arguments are reused (ignore '...' and 'type') as that already does each param in a #define and ignores ... and type ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 9:57 ` Joe Perches @ 2024-03-31 13:43 ` Mac Xu 2024-03-31 13:46 ` Mac Xu 1 sibling, 0 replies; 11+ messages in thread From: Mac Xu @ 2024-03-31 13:43 UTC (permalink / raw) To: Joe Perches, Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, linux-kernel, linux, lukas.bulwahn, sfr, v-songbaohua, workflows, Max Filippov > On Thu, 2024-03-28 at 15: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. > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -6109,6 +6109,36 @@ sub process { > > WARN("TRAILING_SEMICOLON", > > "macros should not use a trailing semicolon\n" . "$herectx"); > > } > > + > > + # match "\s*" rather than "\s+" after the balanced parens, as macro definition with arguments > > + # is not required to have whitespace after arguments > > + if ($dstat =~ /^\+\s*#\s*define\s+$Ident$balanced_parens\s*(\S+.*)(\/[\/*].*)?/) { > > I think '(\/[\/*].*)?' doesn't do what you expect > perhaps '(\/[\/\*].*)?' > though I don't know why this should be capture group I'd wanted to capture the comment to handle a case where a unused param happens to appears in a comment > > > + my $params = $1 || ""; > > > > + my $body = $2 || ""; > > Should never get the || "" as the 2nd capture group is not optional > > > + > > + # get the individual params > > + $params =~ tr/()//d; > > + # remove leading and trailing whitespace > > + $params =~ s/^\s+|\s+$//g; > > + > > + $ctx =~ s/\n*$//; > > + my $cnt = statement_rawlines($ctx); > > + my $herectx = get_stat_here($linenr, $cnt, $here); > > + > > + if ($params ne "") { > > probably unnecessary > > > + my @paramList = split /,\s*/, $params; > > please use split() with parentheses > > > + foreach my $param(@paramList) { > > maybe > foreach my $param (split(/,/, $params) { > $param = trim($param); > next if ($param =~ /\.\.\.$/); > > + if ($param =~ /\.\.\.$/) { > > + # if the param name ends with "...", skip the check > > + next; > > + } > > + if ($body !~ /\b$param\b/) { > > + WARN("UNUSED_PARAM_IN_MACRO", > > + "Parameter '$param' is not used in function-like macro\n" . "$herectx"); > > + } > > + } > It seems this logic is a bit redundant to existing > code and might be better added in the block that starts > > (line 6026) > # check if any macro arguments are reused (ignore '...' and 'type') > > as that already does each param in a #define and > ignores ... and type Hi Joe, Thank you for your comments with insights, as you said, code block of line 6026 is a better place to place this new logic, as it already handles the logic I'd wanted like extracting, splitting and trimming the arguments, excluding the trailing comments etc. By placing the logic in the new place, code duplicates are drastically reduced. Here's my new code (inserted from line 6044): +# check if this is an unused argument + if ($define_stmt !~ /\b$arg\b/) { + WARN("UNUSED_ARG_IN_MACRO", + "Argument '$arg' is not used in function-like macro\n" . "$herectx"); + } +} Please note that I use the wording of "arg/argument" instead of "param/parameter" for consistency, please let me know if if this is the correct wording to use here. Thanks, Mac. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 9:57 ` Joe Perches 2024-03-31 13:43 ` Mac Xu @ 2024-03-31 13:46 ` Mac Xu 2024-03-31 15:54 ` Joe Perches 1 sibling, 1 reply; 11+ messages in thread From: Mac Xu @ 2024-03-31 13:46 UTC (permalink / raw) To: Joe Perches, Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, linux-kernel, linux, lukas.bulwahn, sfr, v-songbaohua, workflows, Max Filippov > On Thu, 2024-03-28 at 15: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. > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -6109,6 +6109,36 @@ sub process { > > WARN("TRAILING_SEMICOLON", > > "macros should not use a trailing semicolon\n" . "$herectx"); > > } > > + > > + # match "\s*" rather than "\s+" after the balanced parens, as macro definition with arguments > > + # is not required to have whitespace after arguments > > + if ($dstat =~ /^\+\s*#\s*define\s+$Ident$balanced_parens\s*(\S+.*)(\/[\/*].*)?/) { > > I think '(\/[\/*].*)?' doesn't do what you expect > perhaps '(\/[\/\*].*)?' > though I don't know why this should be capture group I'd wanted to capture the comment to handle a case where a unused param happens to appears in a comment > > > + my $params = $1 || ""; > > > > + my $body = $2 || ""; > > Should never get the || "" as the 2nd capture group is not optional > > > + > > + # get the individual params > > + $params =~ tr/()//d; > > + # remove leading and trailing whitespace > > + $params =~ s/^\s+|\s+$//g; > > + > > + $ctx =~ s/\n*$//; > > + my $cnt = statement_rawlines($ctx); > > + my $herectx = get_stat_here($linenr, $cnt, $here); > > + > > + if ($params ne "") { > > probably unnecessary > > > + my @paramList = split /,\s*/, $params; > > please use split() with parentheses > > > + foreach my $param(@paramList) { > > maybe > foreach my $param (split(/,/, $params) { > $param = trim($param); > next if ($param =~ /\.\.\.$/); > > + if ($param =~ /\.\.\.$/) { > > + # if the param name ends with "...", skip the check > > + next; > > + } > > + if ($body !~ /\b$param\b/) { > > + WARN("UNUSED_PARAM_IN_MACRO", > > + "Parameter '$param' is not used in function-like macro\n" . "$herectx"); > > + } > > + } > It seems this logic is a bit redundant to existing > code and might be better added in the block that starts > > (line 6026) > # check if any macro arguments are reused (ignore '...' and 'type') > > as that already does each param in a #define and > ignores ... and type Hi Joe, Thank you for your comments with insights, as you said, code block of line 6026 is a better place to place this new logic, as it already handles the logic I'd wanted like extracting, splitting and trimming the arguments, excluding the trailing comments etc. By placing the logic in the new place, code duplicates are reduced. Here's my new code (inserted from line 6044): +# check if this is an unused argument + if ($define_stmt !~ /\b$arg\b/) { + WARN("UNUSED_ARG_IN_MACRO", + "Argument '$arg' is not used in function-like macro\n" . "$herectx"); + } +} Please note that I use the wording of "arg/argument" instead of "param/parameter" for consistency, please let me know if if this is the correct wording to use here. Thanks, Mac. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-31 13:46 ` Mac Xu @ 2024-03-31 15:54 ` Joe Perches 2024-03-31 23:21 ` Mac Xu 0 siblings, 1 reply; 11+ messages in thread From: Joe Perches @ 2024-03-31 15:54 UTC (permalink / raw) To: Mac Xu, Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, linux-kernel, linux, lukas.bulwahn, sfr, v-songbaohua, workflows, Max Filippov On Sun, 2024-03-31 at 13:46 +0000, Mac Xu wrote: > > On Thu, 2024-03-28 at 15: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. > > [] > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > It seems this logic is a bit redundant to existing > > code and might be better added in the block that starts > > > > (line 6026) > > # check if any macro arguments are reused (ignore '...' and 'type') > > > > as that already does each param in a #define and > > ignores ... and type > > Hi Joe, > > Thank you for your comments with insights, as you said, code block of line 6026 is a better place to > place this new logic, as it already handles the logic I'd wanted like extracting, splitting and trimming > the arguments, excluding the trailing comments etc. > > By placing the logic in the new place, code duplicates are reduced. > > Here's my new code (inserted from line 6044): > +# check if this is an unused argument > + if ($define_stmt !~ /\b$arg\b/) { > + WARN("UNUSED_ARG_IN_MACRO", Perhaps WARN("MACRO_ARG_UNUSED", ... to better match the others above it in the block: CHK("MACRO_ARG_REUSE", and CHK("MACRO_ARG_PRECEDENCE", Other than that trivial bit, seems ok. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-31 15:54 ` Joe Perches @ 2024-03-31 23:21 ` Mac Xu 0 siblings, 0 replies; 11+ messages in thread From: Mac Xu @ 2024-03-31 23:21 UTC (permalink / raw) To: Joe Perches, Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, linux-kernel, linux, lukas.bulwahn, sfr, v-songbaohua, workflows, Max Filippov > On Sun, 2024-03-31 at 13:46 +0000, Mac Xu wrote: > > > On Thu, 2024-03-28 at 15: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. > > > [] > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > > [] > > > It seems this logic is a bit redundant to existing > > > code and might be better added in the block that starts > > > > > > (line 6026) > > > # check if any macro arguments are reused (ignore '...' and 'type') > > > > > > as that already does each param in a #define and > > > ignores ... and type > > > > Hi Joe, > > > > Thank you for your comments with insights, as you said, code block of line 6026 is a better place to > > place this new logic, as it already handles the logic I'd wanted like extracting, splitting and trimming > > the arguments, excluding the trailing comments etc. > > > > By placing the logic in the new place, code duplicates are reduced. > > > > Here's my new code (inserted from line 6044): > > +# check if this is an unused argument > > + if ($define_stmt !~ /\b$arg\b/) { > > + WARN("UNUSED_ARG_IN_MACRO", > Perhaps > WARN("MACRO_ARG_UNUSED", > ... > > to better match the others above it in the block: > > CHK("MACRO_ARG_REUSE", > and > CHK("MACRO_ARG_PRECEDENCE", > > Other than that trivial bit, seems ok. Sure, updated, thank you! +# 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"); +} Regards, Xining ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 2:21 ` [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro Barry Song 2024-03-28 9:57 ` Joe Perches @ 2024-03-28 16:01 ` Jeff Johnson 2024-03-28 21:24 ` Barry Song 1 sibling, 1 reply; 11+ messages in thread From: Jeff Johnson @ 2024-03-28 16:01 UTC (permalink / raw) To: Barry Song, akpm, linux-doc Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, joe, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr, v-songbaohua, workflows, Max Filippov On 3/27/2024 7:21 PM, 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) > > total: 1 errors, 0 warnings, 8 lines checked > > Signed-off-by: Xining Xu <mac.xxn@outlook.com> if you are re-posting somebody else's work you need to add your own Signed-off-by > Tested-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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro 2024-03-28 16:01 ` Jeff Johnson @ 2024-03-28 21:24 ` Barry Song 0 siblings, 0 replies; 11+ messages in thread From: Barry Song @ 2024-03-28 21:24 UTC (permalink / raw) To: Jeff Johnson Cc: akpm, linux-doc, apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, joe, linux-kernel, linux, lukas.bulwahn, mac.xxn, sfr, v-songbaohua, workflows, Max Filippov On Fri, Mar 29, 2024 at 5:01 AM Jeff Johnson <quic_jjohnson@quicinc.com> wrote: > > On 3/27/2024 7:21 PM, 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) > > > > total: 1 errors, 0 warnings, 8 lines checked > > > > Signed-off-by: Xining Xu <mac.xxn@outlook.com> > > if you are re-posting somebody else's work you need to add your own Signed-off-by Ok. Jeff, I will do it in the new version and obviously Joe still has some remaining comments to be addressed by Xining. > > > Tested-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> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro 2024-03-28 2:21 [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro Barry Song 2024-03-28 2:21 ` [PATCH v4 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters Barry Song 2024-03-28 2:21 ` [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro Barry Song @ 2024-03-28 4:22 ` Barry Song 2 siblings, 0 replies; 11+ messages in thread From: Barry Song @ 2024-03-28 4:22 UTC (permalink / raw) To: akpm, mac.xxn Cc: apw, broonie, chenhuacai, chris, corbet, dwaipayanray1, herbert, joe, linux-kernel, linux, lukas.bulwahn, sfr, v-songbaohua, workflows, linux-doc On Thu, Mar 28, 2024 at 3:21 PM Barry Song <21cnbao@gmail.com> wrote: > > From: Barry Song <v-songbaohua@oppo.com> > > -v4: > * fix Xining's email address, s/ma.xxn@outlook.com/mac.xxn@outlook.com/g Hi Andrew, Apologies for the oversight. Could you please apply these two patches to replace the ones in the mm-nonmm-unstable branch? We need to correct Xining's email address regardless. > * fix some false positives of checkpatch.pl > * downgrade from ERROR to WARN in checkpatch.pl > > Thanks for Joe's comments! > > -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 | 30 ++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > -- > 2.34.1 > Thanks Barry ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-03-31 23:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-28 2:21 [PATCH v4 0/2] codingstyle: avoid unused parameters for a function-like macro Barry Song 2024-03-28 2:21 ` [PATCH v4 1/2] Documentation: coding-style: ask function-like macros to evaluate parameters Barry Song 2024-03-28 2:21 ` [PATCH v4 2/2] scripts: checkpatch: check unused parameters for function-like macro Barry Song 2024-03-28 9:57 ` Joe Perches 2024-03-31 13:43 ` Mac Xu 2024-03-31 13:46 ` Mac Xu 2024-03-31 15:54 ` Joe Perches 2024-03-31 23:21 ` Mac Xu 2024-03-28 16:01 ` Jeff Johnson 2024-03-28 21:24 ` Barry Song 2024-03-28 4:22 ` [PATCH v4 0/2] codingstyle: avoid unused parameters for a " Barry Song
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox