| Message ID | 20220726115234.25310-1-rpalethorpe@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tools: Check headers with checkpatch.pl | expand |
Hi Richie, > checkpatch.pl doesn't load included headers so they must be passed to > it specifically. This change automatically includes headers from the > current directory. Manual intervention is still required if a test > author changes a header located elsewhere. However you can now write > 'make check-header.h', once in the correct directory. > Note that our Sparse based tool (amongst others) loads headers and > checks at least some of the content. Thanks a lot for implementing this. LGTM, just a small nit: target for headers contain .h extension. Shouldn't we add .c for C sources for consistency? It'đ strange, that make check in testcases/misc/math/float/ does not include tfloat.h, but make check-tfloat.h prints errors. But make check in lib/ works as expected (adds also *.h). Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> checkpatch.pl doesn't load included headers so they must be passed to >> it specifically. This change automatically includes headers from the >> current directory. Manual intervention is still required if a test >> author changes a header located elsewhere. However you can now write >> 'make check-header.h', once in the correct directory. > >> Note that our Sparse based tool (amongst others) loads headers and >> checks at least some of the content. > > Thanks a lot for implementing this. > > LGTM, just a small nit: target for headers contain .h extension. > Shouldn't we add .c for C sources for consistency? > > It'đ strange, that make check in testcases/misc/math/float/ does not include > tfloat.h, but make check-tfloat.h prints errors. But make check in lib/ works as > expected (adds also *.h). The Sparse based tool does include headers though. So really we are checking the target which includes more than just a single c file. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Kind regards, > Petr
... > > LGTM, just a small nit: target for headers contain .h extension. > > Shouldn't we add .c for C sources for consistency? > > It'd strange, that make check in testcases/misc/math/float/ does not include > > tfloat.h, but make check-tfloat.h prints errors. But make check in lib/ works as > > expected (adds also *.h). > The Sparse based tool does include headers though. So really we are > checking the target which includes more than just a single c file. Ah, that makes sense then. Thanks! Kind regards, Petr
diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk index dc4df41d3..a00f31b08 100644 --- a/include/mk/env_post.mk +++ b/include/mk/env_post.mk @@ -92,6 +92,7 @@ endif CHECK_TARGETS ?= $(addprefix check-,$(notdir $(patsubst %.c,%,$(sort $(wildcard $(abs_srcdir)/*.c))))) CHECK_TARGETS := $(filter-out $(addprefix check-, $(FILTER_OUT_MAKE_TARGETS)), $(CHECK_TARGETS)) +CHECK_HEADER_TARGETS ?= $(addprefix check-,$(notdir $(sort $(wildcard $(abs_srcdir)/*.h)))) CHECK ?= $(abs_top_srcdir)/tools/sparse/sparse-ltp CHECK_NOFLAGS ?= $(abs_top_srcdir)/scripts/checkpatch.pl -f --no-tree --terse --no-summary --ignore CONST_STRUCT,VOLATILE,SPLIT_STRING SHELL_CHECK ?= $(abs_top_srcdir)/scripts/checkbashisms.pl --force --extra diff --git a/include/mk/generic_leaf_target.inc b/include/mk/generic_leaf_target.inc index 33e9c9ea0..565a282bb 100644 --- a/include/mk/generic_leaf_target.inc +++ b/include/mk/generic_leaf_target.inc @@ -110,6 +110,6 @@ $(INSTALL_FILES): | $(INSTALL_DEPS) install: $(INSTALL_FILES) $(CHECK_TARGETS): | $(CHECK_DEPS) -check: $(CHECK_TARGETS) $(SHELL_CHECK_TARGETS) +check: $(CHECK_HEADER_TARGETS) $(CHECK_TARGETS) $(SHELL_CHECK_TARGETS) # vim: syntax=make diff --git a/include/mk/rules.mk b/include/mk/rules.mk index 32d8d05a7..517863c04 100644 --- a/include/mk/rules.mk +++ b/include/mk/rules.mk @@ -57,6 +57,15 @@ else @-$(CHECK) $(CHECK_FLAGS) $(CPPFLAGS) $(CFLAGS) $< endif +.PHONY: $(CHECK_HEADER_TARGETS) +$(CHECK_HEADER_TARGETS): check-%.h: %.h +ifdef VERBOSE + -$(CHECK_NOFLAGS) $< +else + @echo CHECK $(target_rel_dir)$< + @-$(CHECK_NOFLAGS) $< +endif + .PHONY: $(SHELL_CHECK_TARGETS) $(SHELL_CHECK_TARGETS): check-%.sh: %.sh ifdef VERBOSE
checkpatch.pl doesn't load included headers so they must be passed to it specifically. This change automatically includes headers from the current directory. Manual intervention is still required if a test author changes a header located elsewhere. However you can now write 'make check-header.h', once in the correct directory. Note that our Sparse based tool (amongst others) loads headers and checks at least some of the content. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Suggested-by: Petr Vorel <pvorel@suse.cz> --- include/mk/env_post.mk | 1 + include/mk/generic_leaf_target.inc | 2 +- include/mk/rules.mk | 9 +++++++++ 3 files changed, 11 insertions(+), 1 deletion(-)