[PATCH 2/3] checkpatch: recognize __chkp_no_side_effects(_var) hint macro

Jim Cromie posted 3 patches 3 months, 2 weeks ago
[PATCH 2/3] checkpatch: recognize __chkp_no_side_effects(_var) hint macro
Posted by Jim Cromie 3 months, 2 weeks ago
Teach checkpatch --strict to allow an author to suppress specific
var-reuse complaints:

  CHECK: Macro argument reuse '_var' - possible side-effects?

Consider:

   #define module_param_named(name, value, type, perm)  \
  +     __chkp_no_side_effects(name, value)		\
        param_check_##type(name, &(value));		\
        module_param_cb(name, &param_ops_##type, &value, perm); \
        __MODULE_PARM_TYPE(name, #type)

Without the +line, this kind of multi-statement macro-defn would cause
checkpatch to carp about name and value being reused.

Note that it wouldn't complain about type, even though type is
expanded 3 times, because checkpatch knew to strip # and ## constructs
on an arg before counting its uses.

module_param_named is actually a named $exception, amongst a ~dozen
others, so it wouldn't draw the complaint (I said kind of above), but
this approach doesn't require further checkpatch tweaks, is specific
to the _vars named, and is author initiated, not a carte-blanche
exception on the macro-name.

IIRC, trybot kicks of a CI pipeline that calls checkpatch --strict,
this hint could help reduce CI-noise going forward.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/checkpatch.pl | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2b0275dcc5a4..bc807c20fcde 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6070,11 +6070,24 @@ sub process {
 			        next if ($arg =~ /\.\.\./);
 			        next if ($arg =~ /^type$/i);
 				my $tmp_stmt = $define_stmt;
-				$tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
-				$tmp_stmt =~ s/\#+\s*$arg\b//g;
-				$tmp_stmt =~ s/\b$arg\s*\#\#//g;
+
+				$tmp_stmt =~ s/\#+\s*$arg\b/drx_print("strip '#|## arg catenations")/ge;
+				$tmp_stmt =~ s/\b$arg\s*\#\#/drx_print("strip 'arg ##' catenations");/ge;
+
+				$tmp_stmt =~ s{
+					\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|
+					   typeof|__typeof__|__builtin\w+|typecheck
+					   \s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b }
+				{
+					drx_print("-builtins-");
+				}xge;
+
+				my $no_side_effect_vars = "";
+				if ($tmp_stmt =~ s/__chkp_no_side_effects\((.+)\)//) {
+					$no_side_effect_vars = $1;
+				}
 				my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g;
-				if ($use_cnt > 1) {
+				if ($use_cnt > 1 and $no_side_effect_vars !~ m/\b$arg\b/) {
 					CHK("MACRO_ARG_REUSE",
 					    "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx");
 				    }
-- 
2.51.0
Re: [PATCH 2/3] checkpatch: recognize __chkp_no_side_effects(_var) hint macro
Posted by Joe Perches 3 months, 2 weeks ago
On Sat, 2025-10-25 at 15:15 -0600, Jim Cromie wrote:
> Teach checkpatch --strict to allow an author to suppress specific
> var-reuse complaints:

It seems to me that annotating source code to silence checkpatch
isn't a great idea.

If it is a great idea, maybe a general CHECKPATCH_ON/OFF(type)
mechanism might be better than specific controls.