* [PATCH 0/3] 2 checkpatch fixes, one pr_info_once
@ 2025-03-25 23:51 Jim Cromie
2025-03-25 23:51 ` [PATCH 1/3] checkpatch: dont warn about unused macro arg on empty body Jim Cromie
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Jim Cromie @ 2025-03-25 23:51 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, Jim Cromie
2 small tweaks to checkpatch,
1 reducing several pages of powernow "not-relevant-here" log-msgs to a few lines
Jim Cromie (3):
checkpatch: dont warn about unused macro arg on empty body
checkpatch: qualify do-while-0 advice
powernow: use pr_info_once
drivers/cpufreq/powernow-k8.c | 2 +-
scripts/checkpatch.pl | 35 ++++++++++++++++++++++++++++-------
2 files changed, 29 insertions(+), 8 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] checkpatch: dont warn about unused macro arg on empty body
2025-03-25 23:51 [PATCH 0/3] 2 checkpatch fixes, one pr_info_once Jim Cromie
@ 2025-03-25 23:51 ` Jim Cromie
2025-03-25 23:51 ` [PATCH 2/3] checkpatch: qualify do-while-0 advice Jim Cromie
2025-03-25 23:51 ` [PATCH 3/3] powernow: use pr_info_once Jim Cromie
2 siblings, 0 replies; 4+ messages in thread
From: Jim Cromie @ 2025-03-25 23:51 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, Jim Cromie, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
Lukas Bulwahn, Louis Chauvet
we currently get:
WARNING: Argument 'name' is not used in function-like macro
on:
#define DRM_CLASSMAP_USE(name) /* nothing here */
Following this advice is wrong here, and shouldn't be fixed by
ignoring args altogether; the macro should properly fail if invoked
with 0 or 2+ args.
cc: Andy Whitcroft <apw@canonical.com>
cc: Joe Perches <joe@perches.com>
cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Acked-by: Joe Perches <joe@perches.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331742..0c4f578ea6e7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6037,7 +6037,7 @@ sub process {
}
# check if this is an unused argument
- if ($define_stmt !~ /\b$arg\b/) {
+ if ($define_stmt !~ /\b$arg\b/ && $define_stmt) {
WARN("MACRO_ARG_UNUSED",
"Argument '$arg' is not used in function-like macro\n" . "$herectx");
}
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] checkpatch: qualify do-while-0 advice
2025-03-25 23:51 [PATCH 0/3] 2 checkpatch fixes, one pr_info_once Jim Cromie
2025-03-25 23:51 ` [PATCH 1/3] checkpatch: dont warn about unused macro arg on empty body Jim Cromie
@ 2025-03-25 23:51 ` Jim Cromie
2025-03-25 23:51 ` [PATCH 3/3] powernow: use pr_info_once Jim Cromie
2 siblings, 0 replies; 4+ messages in thread
From: Jim Cromie @ 2025-03-25 23:51 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, Jim Cromie, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
Lukas Bulwahn
Add a paragraph of advice qualifying the general do-while-0 advice,
noting 3 possible misguidings. reduce one ERROR to WARN, for the case
I actually encountered.
And add 'static_assert' to named exceptions, along with some
additional comments about named exceptions vs (detection of)
declarative construction primitives (union, struct, [], etc).
cc: Andy Whitcroft <apw@canonical.com>
cc: Joe Perches <joe@perches.com>
cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
scripts/checkpatch.pl | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0c4f578ea6e7..044157ba5b47 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -150,6 +150,24 @@ EOM
exit($exitcode);
}
+my $DO_WHILE_0_ADVICE = q{
+ do {} while (0) advice is over-stated in a few situations:
+
+ The more obvious case is macros, like MODULE_PARM_DESC, invoked at
+ file-scope, where C disallows code (it must be in functions). See
+ $exceptions if you have one to add by name.
+
+ More troublesome is declarative macros used at top of new scope,
+ like DECLARE_PER_CPU. These might just compile with a do-while-0
+ wrapper, but would be incorrect. Most of these are handled by
+ detecting struct,union,etc declaration primitives in $exceptions.
+
+ Theres also macros called inside an if (block), which "return" an
+ expression. These cannot do-while, and need a ({}) wrapper.
+
+ Enjoy this qualification while we work to improve our heuristics.
+};
+
sub uniq {
my %seen;
return grep { !$seen{$_}++ } @_;
@@ -5896,9 +5914,9 @@ sub process {
}
}
-# multi-statement macros should be enclosed in a do while loop, grab the
-# first statement and ensure its the whole macro if its not enclosed
-# in a known good container
+# Usually multi-statement macros should be enclosed in a do {} while
+# (0) loop. Grab the first statement and ensure its the whole macro
+# if its not enclosed in a known good container
if ($realfile !~ m@/vmlinux.lds.h$@ &&
$line =~ /^.\s*\#\s*define\s*$Ident(\()?/) {
my $ln = $linenr;
@@ -5951,10 +5969,13 @@ sub process {
my $exceptions = qr{
$Declare|
+ # named exceptions
module_param_named|
MODULE_PARM_DESC|
DECLARE_PER_CPU|
DEFINE_PER_CPU|
+ static_assert|
+ # declaration primitives
__typeof__\(|
union|
struct|
@@ -5989,11 +6010,11 @@ sub process {
ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE",
"Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects\n" . "$herectx");
} elsif ($dstat =~ /;/) {
- ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE",
- "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx");
+ WARN("MULTISTATEMENT_MACRO_USE_DO_WHILE",
+ "Non-declarative macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx\nBUT SEE:\n$DO_WHILE_0_ADVICE");
} else {
ERROR("COMPLEX_MACRO",
- "Macros with complex values should be enclosed in parentheses\n" . "$herectx");
+ "Macros with complex values should be enclosed in parentheses\n" . "$herectx\nBUT SEE:\n$DO_WHILE_0_ADVICE");
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] powernow: use pr_info_once
2025-03-25 23:51 [PATCH 0/3] 2 checkpatch fixes, one pr_info_once Jim Cromie
2025-03-25 23:51 ` [PATCH 1/3] checkpatch: dont warn about unused macro arg on empty body Jim Cromie
2025-03-25 23:51 ` [PATCH 2/3] checkpatch: qualify do-while-0 advice Jim Cromie
@ 2025-03-25 23:51 ` Jim Cromie
2 siblings, 0 replies; 4+ messages in thread
From: Jim Cromie @ 2025-03-25 23:51 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, Jim Cromie
This reduces log-msgs during boot from many pages to ~10 occurrences.
I didn't investigate why it wasn't just 1, maybe its a low-level
service to other modules, re-probed by each of them ?
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
drivers/cpufreq/powernow-k8.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index a01170f7d01c..0559be0d87a5 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -482,7 +482,7 @@ static void check_supported_cpu(void *_rc)
cpuid(CPUID_FREQ_VOLT_CAPABILITIES, &eax, &ebx, &ecx, &edx);
if ((edx & P_STATE_TRANSITION_CAPABLE)
!= P_STATE_TRANSITION_CAPABLE) {
- pr_info("Power state transitions not supported\n");
+ pr_info_once("Power state transitions not supported\n");
return;
}
*rc = 0;
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-25 23:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-25 23:51 [PATCH 0/3] 2 checkpatch fixes, one pr_info_once Jim Cromie
2025-03-25 23:51 ` [PATCH 1/3] checkpatch: dont warn about unused macro arg on empty body Jim Cromie
2025-03-25 23:51 ` [PATCH 2/3] checkpatch: qualify do-while-0 advice Jim Cromie
2025-03-25 23:51 ` [PATCH 3/3] powernow: use pr_info_once Jim Cromie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox