| Message ID | 20231012230359.2005249-1-trini@konsulko.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/7] checkpatch.pl: Make common.h check boarder | expand |
On Thu, 12 Oct 2023 at 16:04, Tom Rini <trini@konsulko.com> wrote: > > 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> > --- > Cc: Simon Glass <sjg@chromium.org> > > This causes a bunch of patman tests, for checkpatch, to now fail and I > don't really understand why, at all. And that was before I added a test > for the new error, which I had hoped would clear up the problem. > --- > scripts/checkpatch.pl | 8 +++++++- > tools/patman/test_checkpatch.py | 7 ++++++- > 2 files changed, 13 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On Fri, Oct 13, 2023 at 08:15:00AM -0700, Simon Glass wrote: > On Thu, 12 Oct 2023 at 16:04, Tom Rini <trini@konsulko.com> wrote: > > > > 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> > > --- > > Cc: Simon Glass <sjg@chromium.org> > > > > This causes a bunch of patman tests, for checkpatch, to now fail and I > > don't really understand why, at all. And that was before I added a test > > for the new error, which I had hoped would clear up the problem. > > --- > > scripts/checkpatch.pl | 8 +++++++- > > tools/patman/test_checkpatch.py | 7 ++++++- > > 2 files changed, 13 insertions(+), 2 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> It can't be reviewed, as I can't actually apply it, as it causes CI to fail as patman tests now fail, and I don't know why.
Hi Tom, On Fri, 13 Oct 2023 at 08:16, Tom Rini <trini@konsulko.com> wrote: > > On Fri, Oct 13, 2023 at 08:15:00AM -0700, Simon Glass wrote: > > On Thu, 12 Oct 2023 at 16:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > 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> > > > --- > > > Cc: Simon Glass <sjg@chromium.org> > > > > > > This causes a bunch of patman tests, for checkpatch, to now fail and I > > > don't really understand why, at all. And that was before I added a test > > > for the new error, which I had hoped would clear up the problem. > > > --- > > > scripts/checkpatch.pl | 8 +++++++- > > > tools/patman/test_checkpatch.py | 7 ++++++- > > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > It can't be reviewed, as I can't actually apply it, as it causes CI to > fail as patman tests now fail, and I don't know why. Well the code looks OK to me. I will take a look. Regards, Simon
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 488d73a0ed77..c3314da8a3c7 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 + # Don't put dm.h in header files if ($realfile =~ /\.h$/ && $rawline =~ /^\+#include\s*<(common|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|dm)\.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..187736d617d5 100644 --- a/tools/patman/test_checkpatch.py +++ b/tools/patman/test_checkpatch.py @@ -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()
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> --- Cc: Simon Glass <sjg@chromium.org> This causes a bunch of patman tests, for checkpatch, to now fail and I don't really understand why, at all. And that was before I added a test for the new error, which I had hoped would clear up the problem. --- scripts/checkpatch.pl | 8 +++++++- tools/patman/test_checkpatch.py | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-)