[PATCH 0/2] checkpatch: fix handling of acpi expected files

[PATCH 0/2] checkpatch: fix handling of acpi expected files

[Michael S. Tsirkin]

Fix a couple of bugs that surfaced.

Michael S. Tsirkin (2):
  checkpatch: fix acpi check with multiple file name
  checkpatch: ignore allowed diff list

 scripts/checkpatch.pl | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

-- 
MST

[PATCH 1/2] checkpatch: fix acpi check with multiple file name

[Michael S. Tsirkin]

Using global expected/nonexpected values causes
false positives when testing multiple patches in one
checkpatch run: one patch can change expected,
another one non-expected.

Use local variables within process() to fix that.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 scripts/checkpatch.pl | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e658e6546f..c3d08aa99f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -35,8 +35,6 @@ my $summary_file = 0;
 my $root;
 my %debug;
 my $help = 0;
-my $acpi_testexpected;
-my $acpi_nontestexpected;
 
 sub help {
 	my ($exitcode) = @_;
@@ -1261,21 +1259,22 @@ sub WARN {
 # According to tests/qtest/bios-tables-test.c: do not
 # change expected file in the same commit with adding test
 sub checkfilename {
-	my ($name) = @_;
+	my ($name, $acpi_testexpected, $acpi_nontestexpected) = @_;
+
 	if ($name =~ m#^tests/data/acpi/# and
 		# make exception for a shell script that rebuilds the files
 		not $name =~ m#^\.sh$# or
 		$name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
-		$acpi_testexpected = $name;
+		$$acpi_testexpected = $name;
 	} else {
-		$acpi_nontestexpected = $name;
+		$$acpi_nontestexpected = $name;
 	}
-	if (defined $acpi_testexpected and defined $acpi_nontestexpected) {
+	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
 		ERROR("Do not add expected files together with tests, " .
 		      "follow instructions in " .
 		      "tests/qtest/bios-tables-test.c: both " .
-		      $acpi_testexpected . " and " .
-		      $acpi_nontestexpected . " found\n");
+		      $$acpi_testexpected . " and " .
+		      $$acpi_nontestexpected . " found\n");
 	}
 }
 
@@ -1325,6 +1324,9 @@ sub process {
 	my %suppress_whiletrailers;
 	my %suppress_export;
 
+        my $acpi_testexpected;
+        my $acpi_nontestexpected;
+
 	# Pre-scan the patch sanitizing the lines.
 
 	sanitise_line_reset();
@@ -1454,11 +1456,11 @@ sub process {
 		if ($line =~ /^diff --git.*?(\S+)$/) {
 			$realfile = $1;
 			$realfile =~ s@^([^/]*)/@@ if (!$file);
-	                checkfilename($realfile);
+	                checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
 		} elsif ($line =~ /^\+\+\+\s+(\S+)/) {
 			$realfile = $1;
 			$realfile =~ s@^([^/]*)/@@ if (!$file);
-	                checkfilename($realfile);
+	                checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
 
 			$p1_prefix = $1;
 			if (!$file && $tree && $p1_prefix ne '' &&
-- 
MST

[PATCH 2/2] checkpatch: ignore allowed diff list

[Michael S. Tsirkin]

Allow changing allowed diff list at any point:
- when changing code under test
- when adding expected files

It's just a list of files so easy to review and merge anyway.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 scripts/checkpatch.pl | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c3d08aa99f..0ba213e9f2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1261,12 +1261,13 @@ sub WARN {
 sub checkfilename {
 	my ($name, $acpi_testexpected, $acpi_nontestexpected) = @_;
 
-	if ($name =~ m#^tests/data/acpi/# and
-		# make exception for a shell script that rebuilds the files
-		not $name =~ m#^\.sh$# or
-		$name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
+        # Note: shell script that rebuilds the expected files is in the same
+        # directory as files themselves.
+        # Note: allowed diff list can be changed both when changing expected
+        # files and when changing tests.
+	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
 		$$acpi_testexpected = $name;
-	} else {
+	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
 		$$acpi_nontestexpected = $name;
 	}
 	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
-- 
MST

Re: [PATCH 0/2] checkpatch: fix handling of acpi expected files

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200504115848.34410-1-mst@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200504115848.34410-1-mst@redhat.com
Subject: [PATCH 0/2] checkpatch: fix handling of acpi expected files
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
0f382d2 checkpatch: ignore allowed diff list
ba77a3e checkpatch: fix acpi check with multiple file name

=== OUTPUT BEGIN ===
1/2 Checking commit ba77a3ef0c54 (checkpatch: fix acpi check with multiple file name)
ERROR: line over 90 characters
#74: FILE: scripts/checkpatch.pl:1459:
+                       checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);

ERROR: line over 90 characters
#79: FILE: scripts/checkpatch.pl:1463:
+                       checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);

total: 2 errors, 0 warnings, 58 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit 0f382d20ef00 (checkpatch: ignore allowed diff list)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200504115848.34410-1-mst@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com