From 704c44dabe900df1e988f15ae5a3013f5c5d04d0 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 19 Oct 2026 21:58:56 +0300 Subject: [PATCH] scripts: Add whole-file Checkpatch analysis Expose make checkpatch through the common C source/header policy, including userspace and KUnit sources. DIFFBR selects whole changed files. Use the kernel's Checkpatch and companion data with --strict, without adding diagnostic exceptions or loading local configuration. Retain raw output and require native totals and a completion record for every selected file. Distinguish source findings from unreadable inputs, inconsistent reports and tool failures, with isolated fixtures. Keep native caches in a temporary directory and serialize only this analyzer's reports so it can run alongside the Kbuild analyzers. Make checkpatch_scan use this target's implementation and exit status instead of suppressing errors. Preserve the separate existing policy of patch/commit helpers and the kernel regression runner. --- Makefile | 6 ++ scripts/README.md | 32 ++++-- scripts/analyzer_policy.py | 4 +- scripts/checkpatch_scan | 22 +---- scripts/policy/checkpatch.json | 9 ++ scripts/style/checkpatch.py | 134 ++++++++++++++++++++++++++ scripts/tests/test_analyzer_policy.py | 4 +- scripts/tests/test_checkpatch.py | 58 +++++++++++ 8 files changed, 239 insertions(+), 30 deletions(-) create mode 100644 scripts/policy/checkpatch.json create mode 100644 scripts/style/checkpatch.py create mode 100644 scripts/tests/test_checkpatch.py diff --git a/Makefile b/Makefile index 8bcd4b2f6..6df930724 100644 --- a/Makefile +++ b/Makefile @@ -171,6 +171,7 @@ help: ' sparse, smatch analyze configured modules through Kbuild' \ ' coccicheck run kernel Coccinelle report rules' \ ' cppcheck analyze fresh kernel and userspace build contexts' \ + ' checkpatch check C source/header style, including KUnit' \ ' DIFFBR=commit compare with one commit; select relevant inputs' \ ' lint-test test shared analyzer tooling (no modules)' \ ' all make all' \ @@ -280,6 +281,11 @@ coccicheck: export SCST_LINT_KDIR := $(KDIR) coccicheck: +@$(PYTHON) -B scripts/coccinelle/coccicheck.py +.PHONY: checkpatch +checkpatch: export SCST_LINT_KDIR := $(KDIR) +checkpatch: + @$(PYTHON) -B scripts/style/checkpatch.py + .PHONY: cppcheck lint-userspace cppcheck: export SCST_LINT_KDIR := $(KDIR) cppcheck: export SCST_LINT_CC := $(CC) diff --git a/scripts/README.md b/scripts/README.md index b9084c013..7feb7946a 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -15,13 +15,13 @@ direct callers of a relocated script must use its new directory below. | `sparse_smatch/` | Native Kbuild analyzers (`make sparse`, `make smatch`) and CHECK records. | | `coccinelle/` | Kernel semantic-patch report rules (`make coccicheck`). | | `cppcheck/` | Native analysis using captured compiler commands (`make cppcheck`). | +| `style/` | Whole-file Checkpatch analysis (`make checkpatch`, `scripts/checkpatch_scan`). | | `tests/` | Isolated regression fixtures for the shared analyzer helpers (`make lint-test`). | -The existing `checkpatch`, `checkpatch_commits`, `checkpatch_diff`, and -`checkpatch_scan` entry points remain directly under `scripts/`. The wrapper's -diagnostic ignore list is in `checkpatch`; whole-tree scan exclusions are in -`checkpatch_scan`. The kernel regression runner retains its separate checkpatch -command and filtering; Sparse and Smatch use the dedicated Make targets below. +The `checkpatch`, `checkpatch_commits` and `checkpatch_diff` helpers retain +their existing patch/commit policy, including the ignored types in `checkpatch`. +The kernel regression runner also retains its separate checkpatch filtering. +`checkpatch_scan` now delegates to the unfiltered whole-file analyzer below. Use Make targets from the repository root unless a component workflow says otherwise. Direct scripts retain their working-directory conventions: @@ -80,7 +80,8 @@ python3 -B scripts/analyzer_policy.py validate ``` The Python commands also accept `--format=json` and a policy name: `common`, -`sparse`, `smatch`, `coccicheck` or `cppcheck`. Use `make analyzer-policy ANALYZER=smatch` to see the +`sparse`, `smatch`, `coccicheck`, `cppcheck` or `checkpatch`. +Use `make analyzer-policy ANALYZER=smatch` to see the effective input scope and all diagnostic exceptions. A file inventory does not establish analysis or build coverage. @@ -232,6 +233,25 @@ analysis. Source findings return 1 and incomplete analysis returns 2; Make maps both to 2. Full logs and the summary are in `.cache/lint/cppcheck.{log,json}`; the summary links to captured commands, native XML and checker coverage. +## Checkpatch + +```sh +make checkpatch KDIR=/path/to/kernel/build +make checkpatch DIFFBR=master CHECKPATCH_PATH=/path/to/kernel/scripts +``` + +The target checks whole C files and headers selected by the common policy, +including userspace and KUnit. Diff mode selects whole changed files. It needs +the kernel's `checkpatch.pl`, `spelling.txt` and `const_structs.checkpatch`, +but no compilation or KUnit-enabled kernel. `CHECKPATCH_PATH` overrides `KDIR`. + +Native `--strict` checks run without ignored types or project exceptions; +local `.checkpatch.conf` files are not loaded. Source files are never rewritten. +Findings return 1; missing per-file completion, inconsistent totals, stderr +or tool failures return 2. Make maps both failures to 2. Complete output and +coverage are in `.cache/lint/checkpatch.{log,json}`. `scripts/checkpatch_scan` +uses the same policy and exit codes; legacy patch/commit helpers remain separate. + ## Analyzer CI The [workflow](../.github/workflows/analyzers.yml) runs on pushes to `master` diff --git a/scripts/analyzer_policy.py b/scripts/analyzer_policy.py index 159cb2ce1..b94eda292 100644 --- a/scripts/analyzer_policy.py +++ b/scripts/analyzer_policy.py @@ -24,8 +24,8 @@ KBUILD_SCHEMA = { "kernel_exceptions": [{"header": str, "message": str, "reason": str}], } BASIC_SCHEMA = {"version": int, "notes": [str]} -POLICIES = ("common", "sparse", "smatch", "coccicheck", "cppcheck") -BASIC_POLICIES = ("coccicheck", "cppcheck") +POLICIES = ("common", "sparse", "smatch", "coccicheck", "cppcheck", "checkpatch") +BASIC_POLICIES = ("coccicheck", "cppcheck", "checkpatch") def relative_path(value: str, location: str): diff --git a/scripts/checkpatch_scan b/scripts/checkpatch_scan index b153040db..33f4327d7 100755 --- a/scripts/checkpatch_scan +++ b/scripts/checkpatch_scan @@ -2,23 +2,5 @@ set -euo pipefail -rootdir="$(readlink -f "$(dirname "$0")/..")" -scriptsdir="${rootdir}/scripts" -outputfile="checkpatch.out" - -mapfile -t src_files < <( - "${scriptsdir}/source/list-source-files" "${rootdir}" | \ - grep -vE '^debian/|^fcst/linux-patches|patch$|pdf$|png$|^iscsi-scst/usr|^qla|^scripts/|^scstadmin/|^usr/|^www/' | \ - while read -r filename; do echo "${rootdir}/$filename"; done) - -"${scriptsdir}/checkpatch" -f "${src_files[@]}" 1> "${outputfile}" || true - -errors=$(grep -c '^ERROR' "${outputfile}" || true) -warnings=$(grep -c '^WARNING' "${outputfile}" || true) -checks=$(grep -c '^CHECK' "${outputfile}" || true) - -echo "${errors} errors / ${warnings} warnings / ${checks} checks." -grep -E '^WARNING|^ERROR|^CHECK' "${outputfile}" | - sort | - sed 's/^CHECK:CAMELCASE: Avoid CamelCase:.*/CHECK:CAMELCASE Avoid CamelCase/' | - uniq -c +scriptsdir="$(cd "$(dirname "$0")" && pwd)" +exec "${PYTHON:-python3}" -B "$scriptsdir/style/checkpatch.py" "$@" diff --git a/scripts/policy/checkpatch.json b/scripts/policy/checkpatch.json new file mode 100644 index 000000000..54b768b96 --- /dev/null +++ b/scripts/policy/checkpatch.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "notes": [ + "Check whole C files and headers with native Checkpatch --strict.", + "Include userspace and KUnit sources through the common input policy.", + "No diagnostic exceptions, ignored types or local Checkpatch configuration.", + "Legacy patch and commit helpers retain their separate existing policy." + ] +} diff --git a/scripts/style/checkpatch.py b/scripts/style/checkpatch.py new file mode 100644 index 000000000..4b9757845 --- /dev/null +++ b/scripts/style/checkpatch.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +"""Check whole SCST C files with native Checkpatch and no diagnostic filters.""" + +import argparse +from collections import Counter +import fcntl +import json +import os +from pathlib import Path +import re +import subprocess +import sys +import tempfile + +sys.dont_write_bytecode = True +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "scripts")) +from lint_common import copy_output, entrypoint, log_path, run, show_log # noqa: E402 +from lint_scope import select_sources # noqa: E402 + +TOTAL = re.compile(r"total: (\d+) errors, (\d+) warnings, (\d+) checks, \d+ lines checked") +COMPLETE = re.compile(r"(.+) has (?:style problems, please review|" + r"no obvious style problems and is ready for submission)\.") +DIAGNOSTIC = re.compile(r"^(ERROR|WARNING|CHECK):([^:\n]+): ?(.*)$", re.MULTILINE) + + +def checker_path(): + kernel = Path(os.environ.get("SCST_LINT_KDIR") or os.environ.get("KDIR") or + f"/lib/modules/{os.environ.get('KVER') or os.uname().release}/build") + directory = Path(os.environ.get("CHECKPATCH_PATH") or kernel / "scripts").resolve() + for name in ("checkpatch.pl", "spelling.txt", "const_structs.checkpatch"): + if not (directory / name).is_file(): + raise ValueError(f"missing Checkpatch tool/data: {directory / name}") + return directory / "checkpatch.pl" + + +def native_report(text, stderr, status, selected): + """Require matching native totals and completion for every requested file.""" + diagnostics = Counter(match[1] for match in DIAGNOSTIC.finditer(text)) + totals = [tuple(map(int, match.groups())) for line in text.splitlines() + if (match := TOTAL.fullmatch(line))] + completed = Counter(str(Path(match[1]).resolve()) for line in text.splitlines() + if (match := COMPLETE.fullmatch(line))) + expected = Counter(str(path.resolve()) for path in selected) + failures = [] + if stderr: + failures.append("native stderr: " + stderr.rstrip()) + if status not in (0, 1): + failures.append(f"native exit status {status}") + if len(totals) != len(selected): + failures.append(f"expected {len(selected)} native totals, got {len(totals)}") + for index, severity in enumerate(("ERROR", "WARNING", "CHECK")): + if sum(row[index] for row in totals) != diagnostics[severity]: + failures.append(f"native {severity} total does not match retained diagnostics") + for name, count in (expected - completed).items(): + failures.append(f"missing {count} native completion record(s) for {name}") + for name, count in (completed - expected).items(): + failures.append(f"unexpected {count} native completion record(s) for {name}") + remaining = sum(diagnostics.values()) + if status in (0, 1) and status != int(bool(remaining)): + failures.append("native exit status disagrees with diagnostic totals") + return {"native_status": status, "counts": dict(diagnostics), "remaining": remaining, + "failures": failures, "complete": not failures, + "completed_files": sorted(completed)} + + +def check(tool, selected, log): + git_dir = subprocess.check_output(["git", "-C", str(ROOT), "rev-parse", + "--absolute-git-dir"], text=True).strip() + # Native Checkpatch writes/deletes CamelCase caches in cwd. Its own + # configuration search must not import a user's --ignore or --fix flags. + with tempfile.TemporaryDirectory(prefix="checkpatch-", dir=log.parent) as temporary: + directory = Path(temporary) + (directory / ".checkpatch.conf").write_text("") + stdout = directory / "stdout" + stderr = directory / "stderr" + env = dict(os.environ, CHECKPATCH_CONFIG_DIR=str(directory), + GIT_DIR=git_dir, GIT_WORK_TREE=str(ROOT)) + command = [str(tool), "--no-tree", "--root=" + str(ROOT), "--strict", "--show-types", + "--color=never", "--no-fix", "--no-fix-inplace", "--file", *map(str, selected)] + try: + with stdout.open("wb") as output, stderr.open("wb") as errors: + result = run(command, cwd=directory, env=env, stdout=output, stderr=errors, + progress=f"Checkpatch ({len(selected)} files)") + finally: + with log.open("ab") as stream: + for path in (stdout, stderr): + if path.exists(): + copy_output(path, stream) + return native_report(stdout.read_text(errors="replace"), stderr.read_text(errors="replace"), + result.returncode, selected) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", default=os.environ.get("DIFFBR") or None) + parser.add_argument("files", nargs="*", type=Path) + args = parser.parse_args() + with log_path(ROOT, "checkpatch.lock", truncate=False).open("a") as lock: + # Serialize this report without blocking independent Kbuild analyzers. + fcntl.flock(lock, fcntl.LOCK_EX) + log = log_path(ROOT, "checkpatch.log") + report = log.with_suffix(".json") + summary = {"tool": "checkpatch", "run_id": os.environ.get("SCST_LINT_RUN_ID"), + "outcome": "incomplete", "exit_code": 2, "remaining": 0, + "checked_files": [], "selected_files": []} + report.write_text(json.dumps(summary, indent=2) + "\n") + show_log(log) + try: + selected = select_sources(ROOT, args.files, args.base, "checkpatch") + summary["selected_files"] = [path.relative_to(ROOT).as_posix() for path in selected] + if not selected: + print("No eligible Checkpatch inputs; analysis skipped.") + summary.update(outcome="skipped", exit_code=0) + return 0 + summary.update(check(checker_path(), selected, log)) + if not summary["complete"]: + print("Checkpatch analysis is incomplete:\n" + "\n".join(summary["failures"]), + file=sys.stderr) + return 2 + summary["checked_files"] = summary["selected_files"] + status = int(bool(summary["remaining"])) + summary.update(outcome="findings" if status else "clean", exit_code=status) + print(f"Checkpatch: {len(selected)} files, {summary['remaining']} diagnostic(s).") + return status + finally: + report.write_text(json.dumps(summary, indent=2) + "\n") + if os.environ.get("LINT_DEFER_REPORTS") != "1": + copy_output(log) + show_log(log) + + +if __name__ == "__main__": + sys.exit(entrypoint(main)) diff --git a/scripts/tests/test_analyzer_policy.py b/scripts/tests/test_analyzer_policy.py index c5d76bc14..fddc82725 100644 --- a/scripts/tests/test_analyzer_policy.py +++ b/scripts/tests/test_analyzer_policy.py @@ -157,7 +157,7 @@ class PolicyCliTests(unittest.TestCase): source = analyzer_policy.POLICY_PATH.parent.parent for name in ("analyzer_policy.py", "lint_scope.py", "policy/common.json", "policy/sparse.json", "policy/smatch.json", "policy/coccicheck.json", - "policy/cppcheck.json"): + "policy/cppcheck.json", "policy/checkpatch.json"): shutil.copyfile(source / name, self.scripts / name) def run_cli(self, *args): @@ -233,7 +233,7 @@ class PolicyCliTests(unittest.TestCase): def test_basic_analyzers_inherit_common_scope_without_exceptions(self): common = json.loads((self.scripts / "policy/common.json").read_text()) - for tool in ("coccicheck", "cppcheck"): + for tool in ("coccicheck", "cppcheck", "checkpatch"): with self.subTest(tool=tool): result = self.run_cli("show", tool, "--format=json") self.assertEqual(result.returncode, 0, result.stderr) diff --git a/scripts/tests/test_checkpatch.py b/scripts/tests/test_checkpatch.py new file mode 100644 index 000000000..5da5fca6f --- /dev/null +++ b/scripts/tests/test_checkpatch.py @@ -0,0 +1,58 @@ +"""Checkpatch protocol fixtures without a kernel build or source rewrites.""" + +from pathlib import Path +import sys +import unittest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from style.checkpatch import native_report + + +class CheckpatchTests(unittest.TestCase): + def setUp(self): + self.first = Path("/tmp/scst/first.c") + self.second = Path("/tmp/scst/second.c") + self.clean = ("total: 0 errors, 0 warnings, 0 checks, 1 lines checked\n" + f"{self.first} has no obvious style problems and is ready for submission.\n") + self.warning = ("WARNING:SPDX_LICENSE_TAG: Missing SPDX tag\n" + "total: 0 errors, 1 warnings, 0 checks, 1 lines checked\n" + f"{self.first} has style problems, please review.\n") + + def test_completed_clean_file(self): + report = native_report(self.clean, "", 0, [self.first]) + self.assertTrue(report["complete"]) + self.assertEqual(report["remaining"], 0) + + def test_source_warning_is_retained(self): + report = native_report(self.warning, "", 1, [self.first]) + self.assertTrue(report["complete"]) + self.assertEqual(report["remaining"], 1) + self.assertEqual(report["counts"], {"WARNING": 1}) + + def test_unreadable_second_file_cannot_hide_behind_first_file(self): + for status, text in ((0, self.clean), (1, self.warning)): + with self.subTest(status=status): + report = native_report(text, "diff: second.c: Permission denied", status, + [self.first, self.second]) + self.assertFalse(report["complete"]) + self.assertTrue(any("native stderr" in item for item in report["failures"])) + self.assertTrue(any("completion" in item for item in report["failures"])) + + def test_missing_duplicate_and_unknown_completions_fail(self): + for text, files in ((self.clean, [self.first, self.second]), + (self.clean + self.clean, [self.first, self.second]), + (self.clean, [self.second]), ("", [self.first])): + with self.subTest(text=text, files=files): + self.assertFalse(native_report(text, "", 0, files)["complete"]) + + def test_native_failure_and_unaccounted_diagnostics_fail(self): + for text, stderr, status in ((self.clean, "Perl failed", 0), (self.clean, "", 2), + (self.warning, "", 0), (self.clean, "", 1), + (self.warning.replace("1 warnings", "0 warnings"), "", 1), + (self.warning.split("\n", 1)[1], "", 1)): + with self.subTest(text=text, stderr=stderr, status=status): + self.assertFalse(native_report(text, stderr, status, [self.first])["complete"]) + + +if __name__ == "__main__": + unittest.main()