[PATCH v2 1/2] checkpatch.pl: Make common.h check boarder
From: Tom Rini trini@konsulko.com
At this point in time we should not add common.h to any new files, so make checkpatch.pl complain.
Signed-off-by: Tom Rini trini@konsulko.com Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Only check one header at a time in checkpatch.pl
scripts/checkpatch.pl | 10 ++++++++-- tools/patman/test_checkpatch.py | 9 +++++++-- 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 488d73a0ed77..b8eb57f38c74 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2636,12 +2636,18 @@ sub u_boot_line { "All CONFIG symbols are managed by Kconfig\n" . $herecurr); }
- # Don't put common.h and dm.h in header files - if ($realfile =~ /.h$/ && $rawline =~ /^+#include\s*<(common|dm).h>*/) { + # Don't put dm.h in header files + if ($realfile =~ /.h$/ && $rawline =~ /^+#include\s*<dm.h>*/) { ERROR("BARRED_INCLUDE_IN_HDR", "Avoid including common.h and dm.h in header files\n" . $herecurr); }
+ # Don't add common.h to files + if ($rawline =~ /^+#include\s*<common.h>*/) { + ERROR("BARRED_INCLUDE_COMMON_H", + "Do not add common.h to files\n" . $herecurr); + } + # Do not disable fdt / initrd relocation if ($rawline =~ /^+.*(fdt|initrd)_high=0xffffffff/) { ERROR("DISABLE_FDT_OR_INITRD_RELOC", diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py index a8bb364e42b2..f8117460ced2 100644 --- a/tools/patman/test_checkpatch.py +++ b/tools/patman/test_checkpatch.py @@ -238,7 +238,7 @@ index 0000000..2234c87 + * passed to kernel in the ATAGs + */ + -+#include <common.h> ++#include <config.h> + +struct bootstage_record { + u32 time_us; @@ -401,10 +401,15 @@ index 0000000..2234c87 def test_barred_include_in_hdr(self): """Test for using a barred include in a header file""" pm = PatchMaker() - #pm.add_line('include/myfile.h', '#include <common.h>') pm.add_line('include/myfile.h', '#include <dm.h>') self.check_single_message(pm, 'BARRED_INCLUDE_IN_HDR', 'error')
+ def test_barred_include_common_h(self): + """Test for adding common.h to a file""" + pm = PatchMaker() + pm.add_line('include/myfile.h', '#include <common.h>') + self.check_single_message(pm, 'BARRED_INCLUDE_COMMON_H', 'error') + def test_config_is_enabled_config(self): """Test for accidental CONFIG_IS_ENABLED(CONFIG_*) calls""" pm = PatchMaker()
These texts lack comments. Add some so that it is clearer what is going on.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/test_checkpatch.py | 49 +++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py index f8117460ced2..0a8f7408f146 100644 --- a/tools/patman/test_checkpatch.py +++ b/tools/patman/test_checkpatch.py @@ -18,19 +18,47 @@ from patman import commit
class Line: + """Single changed line in one file in a patch + + Args: + fname (str): Filename containing the added line + text (str): Text of the added line + """ def __init__(self, fname, text): self.fname = fname self.text = text
class PatchMaker: + """Makes a patch for checking with checkpatch.pl + + The idea here is to create a patch which adds one line in one file, + intended to provoke a checkpatch error or warning. The base patch is empty + (i.e. invalid), so you should call add_line() to add at least one line. + """ def __init__(self): + """Set up the PatchMaker object + + Properties: + lines (list of Line): List of lines to add to the patch. Note that + each line has both a file and some text associated with it, + since for simplicity we just add a single line for each file + """ self.lines = []
def add_line(self, fname, text): + """Add to the list of filename/line pairs""" self.lines.append(Line(fname, text))
def get_patch_text(self): + """Build the patch text + + Takes a base patch and adds a diffstat and patch for each filename/line + pair in the list. + + Returns: + str: Patch text ready for submission to checkpatch + """ base = '''From 125b77450f4c66b8fd9654319520bbe795c9ef31 Mon Sep 17 00:00:00 2001 From: Simon Glass sjg@chromium.org Date: Sun, 14 Jun 2020 09:45:14 -0600 @@ -75,6 +103,11 @@ Signed-off-by: Simon Glass sjg@chromium.org return '\n'.join(lines)
def get_patch(self): + """Get the patch text and write it into a temporary file + + Returns: + str: Filename containing the patch + """ inhandle, inname = tempfile.mkstemp() infd = os.fdopen(inhandle, 'w') infd.write(self.get_patch_text()) @@ -82,6 +115,22 @@ Signed-off-by: Simon Glass sjg@chromium.org return inname
def run_checkpatch(self): + """Run checkpatch on the patch file + + Returns: + namedtuple containing: + ok: False=failure, True=ok + problems: List of problems, each a dict: + 'type'; error or warning + 'msg': text message + 'file' : filename + 'line': line number + errors: Number of errors + warnings: Number of warnings + checks: Number of checks + lines: Number of lines + stdout: Full output of checkpatch + """ return checkpatch.check_patch(self.get_patch(), show_types=True)
On Fri, Oct 13, 2023 at 09:28:33AM -0700, Simon Glass wrote:
These texts lack comments. Add some so that it is clearer what is going on.
Signed-off-by: Simon Glass sjg@chromium.org
Ah, OK, I see now why my changes failed, thanks.
On Fri, Oct 13, 2023 at 09:28:33AM -0700, Simon Glass wrote:
These texts lack comments. Add some so that it is clearer what is going on.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
On Fri, Oct 13, 2023 at 09:28:32AM -0700, Simon Glass wrote:
From: Tom Rini trini@konsulko.com
At this point in time we should not add common.h to any new files, so make checkpatch.pl complain.
Signed-off-by: Tom Rini trini@konsulko.com Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (2)
-
Simon Glass -
Tom Rini