[PATCH 0/4] checkpatch: skip more kernel-only checks for userspace code

[PATCH 0/4] checkpatch: skip more kernel-only checks for userspace code

[Kir Kolyshkin]

Commit 99b70ece33d8 ("checkpatch: suppress strscpy warnings for
userspace tools") added is_userspace() to skip checks which only make
sense for the kernel. This series extends that to a few more checks
which produce false positives for userspace code under tools/:

 - ENOSYS: the "ENOSYS means invalid syscall nr" rule is about the
   kernel ABI; in userspace, errno == ENOSYS is the standard way to
   detect a missing syscall (e.g. to skip a selftest on older kernels);
 - AVOID_BUG: "Do not crash the kernel" does not apply to userspace,
   where BUG()/BUG_ON() are assert-like shims;
 - SSCANF_TO_KSTRTO: kstrto<type> is not available in userspace.

Running "checkpatch.pl -f" (with --strict for AVOID_BUG, which is
a CHECK in file mode) on the files under tools/ that use ENOSYS, BUG()
or sscanf() gives 699 such warnings before this series (ENOSYS: 188,
AVOID_BUG: 491, SSCANF_TO_KSTRTO: 20), and none after.

Since tools/ also contains some kernel code (test modules, Kbuild-built
mocks, rvgen templates), patch 1 makes is_userspace() return false for
those locations, so they are still subject to all the kernel checks
(this also applies to the existing strcpy/strlcpy/strncpy checks).

Kir Kolyshkin (4):
  checkpatch: is_userspace: exclude kernel code under tools/
  checkpatch: don't warn about ENOSYS in userspace code
  checkpatch: don't warn about BUG() in userspace code
  checkpatch: don't suggest kstrto<type> in userspace code

 scripts/checkpatch.pl | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)


base-commit: f47d223627f80cb7a1eb14c585fe6cb35de5e984
-- 
2.56.0

[PATCH 1/4] checkpatch: is_userspace: exclude kernel code under tools/

[Kir Kolyshkin]

is_userspace() treats everything under tools/ as userspace code, and is
used to skip checks that only make sense for the kernel. However, tools/
also contains some kernel code: test modules, Kbuild-built mocks, and
templates and golden outputs of the runtime verification monitor
generator. Such code should be subject to the same checks as the rest
of the kernel.

Explicitly exclude the known locations of kernel code under tools/.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8a7787d228a6..8f9a46d809ae 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2665,6 +2665,12 @@ sub exclude_global_initialisers {
 
 sub is_userspace {
     my ($realfile) = @_;
+
+    # Kernel code under tools/: test modules, mocks, code generator templates.
+    return 0 if ($realfile =~ m@^tools/testing/(?:cxl|nvdimm|crypto)/@ ||
+             $realfile =~ m@^tools/testing/selftests/(?:bpf/test_kmods|livepatch/test_modules|mm/page_frag|net/bench/page_pool)/@ ||
+             $realfile =~ m@^tools/(?:power/cpupower/debug/kernel|virtio/vhost_test|verification/rvgen)/@);
+
     return ($realfile =~ m@^tools/@ || $realfile =~ m@^scripts/@);
 }
 
-- 
2.56.0

[PATCH 2/4] checkpatch: don't warn about ENOSYS in userspace code

[Kir Kolyshkin]

The ENOSYS check enforces a kernel ABI rule: the kernel must only return
-ENOSYS for an invalid syscall number. It makes no sense for userspace
code under tools/, where ENOSYS is a regular "Function not implemented"
error, and checking for errno == ENOSYS is the standard way to detect
that a syscall is not available (e.g. to skip a selftest on an older
kernel). There, the warning is always a false positive.

Use the existing is_userspace() helper to skip the check for such files.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8f9a46d809ae..a97d5748878b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4985,7 +4985,7 @@ sub process {
 # ENOSYS means "bad syscall nr" and nothing else.  This will have a small
 # number of false positives, but assembly files are not checked, so at
 # least the arch entry code will not trigger this warning.
-		if ($line =~ /\bENOSYS\b/) {
+		if ($line =~ /\bENOSYS\b/ && !is_userspace($realfile)) {
 			WARN("ENOSYS",
 			     "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr);
 		}
-- 
2.56.0

[PATCH 3/4] checkpatch: don't warn about BUG() in userspace code

[Kir Kolyshkin]

The AVOID_BUG check says "Do not crash the kernel unless it is absolutely
unavoidable". This makes no sense for userspace code under tools/, where
BUG() and BUG_ON() are assert-like helpers provided by the kernel API
shims (e.g. in tools/testing/radix-tree), and crashing a test program on
an unexpected condition is fine.

Use is_userspace() to skip the check for such files.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 scripts/checkpatch.pl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a97d5748878b..be0e0493d4a2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4924,8 +4924,9 @@ sub process {
 			}
 		}
 
-# do not use BUG() or variants
-		if ($line =~ /\b(?!AA_|BUILD_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) {
+# do not use BUG() or variants (does not apply to userspace)
+		if (!is_userspace($realfile) &&
+		    $line =~ /\b(?!AA_|BUILD_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) {
 			my $msg_level = \&WARN;
 			$msg_level = \&CHK if ($file);
 			&{$msg_level}("AVOID_BUG",
-- 
2.56.0

[PATCH 4/4] checkpatch: don't suggest kstrto<type> in userspace code

[Kir Kolyshkin]

The SSCANF_TO_KSTRTO check suggests using kstrto<type> instead of a
single-variable sscanf(). These helpers are kernel-only and are not
available to userspace code under tools/, so the suggestion can not be
followed there.

Use is_userspace() to skip the check for such files.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 scripts/checkpatch.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index be0e0493d4a2..7a520faec34e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7204,8 +7204,8 @@ sub process {
 			     "unchecked sscanf return value\n" . "$here\n$stat_real\n");
 		}
 
-# check for simple sscanf that should be kstrto<foo>
-		if ($perl_version_ok &&
+# check for simple sscanf that should be kstrto<foo> (not available in userspace)
+		if ($perl_version_ok && !is_userspace($realfile) &&
 		    defined $stat &&
 		    $line =~ /\bsscanf\b/) {
 			my $lc = $stat =~ tr@\n@@;
-- 
2.56.0