From:   =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= <bjorn@kernel.org>
To:     Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com>,
        Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        linux-kernel@vger.kernel.org
Cc:     =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= <bjorn@rivosinc.com>,
        Andy Shevchenko <andy@kernel.org>
Subject: [RFC PATCH] checkpatch: Support __initconst combined with struct definition
Date:   Wed,  1 Mar 2023 10:43:20 +0100
Message-Id: <20230301094320.15954-1-bjorn@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Björn Töpel <bjorn@rivosinc.com>

Checkpatch sometimes report a false positive for __initconst. E.g., for the
following snippet:

 | static const struct strspn_test {
 | 	const char str[16];
 | 	const char accept[16];
 | 	const char reject[16];
 | 	unsigned a;
 | 	unsigned r;
 | } tests[] __initconst = {
 | 	{ "foobar", "", "", 0, 6 },
 | 	{ "abba", "abc", "ABBA", 4, 4 },
 | 	{ "abba", "a", "b", 1, 1 },
 | 	{ "", "abc", "abc", 0, 0},
 | };

checkpatch would report:

 | ERROR: Use of __initconst requires a separate use of const
 | #190: FILE: ./test_string.c:190:
 | +	} tests[] __initconst = {

Improve the reporting by trying harder to find the 'const'.

Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---

In [1], Andy asked if it was possible to fix the __initconst false
positive in checkpatch, rather than the code.

I did a crude hack that searches backwards for the 'const' in the
struct definition, but I'm sure the Perl hackers out there hate it,
hence the RFC. ;-)

Björn

[1] https://lore.kernel.org/linux-riscv/CAHp75VfK3RM+SP90d3nOXEobY81Xd_94tLL=Qt86mmdNwXaQpg@mail.gmail.com/

---
 scripts/checkpatch.pl | 54 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..d2370233e2c1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1854,6 +1854,48 @@ sub ctx_statement_full {
 	return ($level, $linenr, @chunks);
 }
 
+sub ctx_block_outer_rev {
+	my ($linenr, $open, $close) = @_;
+	my $line;
+	my $start = $linenr;
+	my $blk = '';
+	my @res = ();
+
+	my $level = 0;
+	my @stack = ($level);
+	for ($line = $start; $line >= 0; $line--) {
+		next if ($rawlines[$line] =~ /^-/);
+
+		$blk .= $rawlines[$line];
+
+		# Handle nested #if/#else.
+		if ($lines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) {
+			$level = pop(@stack);
+		} elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) {
+			$level = $stack[$#stack - 1];
+		} elsif ($lines[$line] =~ /^.\s*#\s*endif\b/) {
+			push(@stack, $level);
+		}
+
+		foreach my $c (split(//, $lines[$line])) {
+			if ($c eq $close && $level > 0) {
+				$level--;
+				last if ($level == 0);
+			} elsif ($c eq $open) {
+				$level++;
+			}
+		}
+
+		if ($level <= 1) {
+			push(@res, $rawlines[$line]);
+		}
+
+		last if ($level == 0);
+	}
+
+	return @res;
+}
+
 sub ctx_block_get {
 	my ($linenr, $remain, $outer, $open, $close, $off) = @_;
 	my $line;
@@ -6502,7 +6544,17 @@ sub process {
 # check for $InitAttributeConst (ie: __initconst) without const
 		if ($line !~ /\bconst\b/ && $line =~ /($InitAttributeConst)/) {
 			my $attr = $1;
-			if (ERROR("INIT_ATTRIBUTE",
+			my $error = 1;
+			if ($line =~ /}/) {
+			# The const might be part of a struct definition. Try to find that...
+				my @ctx = ctx_block_outer_rev($linenr, '}', '{');
+				if (@ctx) {
+					if ($ctx[$#ctx] =~ /\bconst\b/) {
+						$error = 0;
+					}
+				}
+			}
+			if ($error && ERROR("INIT_ATTRIBUTE",
 				  "Use of $attr requires a separate use of const\n" . $herecurr) &&
 			    $fix) {
 				my $lead = $fixed[$fixlinenr] =~

base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
-- 
2.37.2