| Message ID | 20220704152303.760983-8-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests: introduce a tree-wide code style checking facility | expand |
On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé <berrange@redhat.com> wrote: > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > + > +sc_c_file_osdep_h: > + @require='#include "qemu/osdep.h"' \ > + in_vc_files='\.c$$' \ > + halt='all C files must include qemu/osdep.h' \ > + $(_sc_search_regexp) The rule is not just "included in all C files", but "included as the *first* include in all C files". thanks -- PMM
On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote: > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > > + > > +sc_c_file_osdep_h: > > + @require='#include "qemu/osdep.h"' \ > > + in_vc_files='\.c$$' \ > > + halt='all C files must include qemu/osdep.h' \ > > + $(_sc_search_regexp) > > The rule is not just "included in all C files", but "included > as the *first* include in all C files". Oh right, so we can copy a rule from libvirt to validate that. It would look like this, but s,config.h,qemu/osdep.h, # Print each file name for which the first #include does not match # $(config_h_header). Like grep -m 1, this only looks at the first match. perl_config_h_first_ = \ -e 'BEGIN {$$ret = 0}' \ -e 'if (/^\# *include\b/) {' \ -e ' if (not m{^\# *include $(config_h_header)}) {' \ -e ' print "$$ARGV\n";' \ -e ' $$ret = 1;' \ -e ' }' \ -e ' \# Move on to next file after first include' \ -e ' close ARGV;' \ -e '}' \ -e 'END {exit $$ret}' # You must include <config.h> before including any other header file. # This can possibly be via a package-specific header, if given by syntax-check.mk. sc_require_config_h_first: @if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \ files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \ perl -n $(perl_config_h_first_) $$files || \ { echo 'the above files include some other header' \ 'before <config.h>' 1>&2; exit 1; } || :; \ else :; \ fi With regards, Daniel
On Mon, 4 Jul 2022 at 16:50, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote: > > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > > > > + > > > +sc_c_file_osdep_h: > > > + @require='#include "qemu/osdep.h"' \ > > > + in_vc_files='\.c$$' \ > > > + halt='all C files must include qemu/osdep.h' \ > > > + $(_sc_search_regexp) > > > > The rule is not just "included in all C files", but "included > > as the *first* include in all C files". > > Oh right, so we can copy a rule from libvirt to validate that. > > It would look like this, but s,config.h,qemu/osdep.h, > > > # Print each file name for which the first #include does not match > # $(config_h_header). Like grep -m 1, this only looks at the first match. > perl_config_h_first_ = \ > -e 'BEGIN {$$ret = 0}' \ > -e 'if (/^\# *include\b/) {' \ > -e ' if (not m{^\# *include $(config_h_header)}) {' \ > -e ' print "$$ARGV\n";' \ > -e ' $$ret = 1;' \ > -e ' }' \ > -e ' \# Move on to next file after first include' \ > -e ' close ARGV;' \ > -e '}' \ > -e 'END {exit $$ret}' > > # You must include <config.h> before including any other header file. > # This can possibly be via a package-specific header, if given by syntax-check.mk. > sc_require_config_h_first: > @if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \ > files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \ > perl -n $(perl_config_h_first_) $$files || \ > { echo 'the above files include some other header' \ > 'before <config.h>' 1>&2; exit 1; } || :; \ > else :; \ > fi As an example syntax checking rule I think this makes a pretty convincing case for the argument "make is the wrong language/framework for this job"... thanks -- PMM
On Mon, Jul 04, 2022 at 04:55:45PM +0100, Peter Maydell wrote: > On Mon, 4 Jul 2022 at 16:50, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Mon, Jul 04, 2022 at 04:47:16PM +0100, Peter Maydell wrote: > > > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > > > > > > + > > > > +sc_c_file_osdep_h: > > > > + @require='#include "qemu/osdep.h"' \ > > > > + in_vc_files='\.c$$' \ > > > > + halt='all C files must include qemu/osdep.h' \ > > > > + $(_sc_search_regexp) > > > > > > The rule is not just "included in all C files", but "included > > > as the *first* include in all C files". > > > > Oh right, so we can copy a rule from libvirt to validate that. > > > > It would look like this, but s,config.h,qemu/osdep.h, > > > > > > # Print each file name for which the first #include does not match > > # $(config_h_header). Like grep -m 1, this only looks at the first match. > > perl_config_h_first_ = \ > > -e 'BEGIN {$$ret = 0}' \ > > -e 'if (/^\# *include\b/) {' \ > > -e ' if (not m{^\# *include $(config_h_header)}) {' \ > > -e ' print "$$ARGV\n";' \ > > -e ' $$ret = 1;' \ > > -e ' }' \ > > -e ' \# Move on to next file after first include' \ > > -e ' close ARGV;' \ > > -e '}' \ > > -e 'END {exit $$ret}' > > > > # You must include <config.h> before including any other header file. > > # This can possibly be via a package-specific header, if given by syntax-check.mk. > > sc_require_config_h_first: > > @if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then \ > > files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') && \ > > perl -n $(perl_config_h_first_) $$files || \ > > { echo 'the above files include some other header' \ > > 'before <config.h>' 1>&2; exit 1; } || :; \ > > else :; \ > > fi > > As an example syntax checking rule I think this makes a pretty > convincing case for the argument "make is the wrong language/framework > for this job"... Matching contextually across multiple lines of text is admittedly hard. IME most of the usage of this syntax checking facility we had in libvirt can be handled using single line matches, which are trivial to provide. With regards, Daniel
diff --git a/tests/style-excludes.mak b/tests/style-excludes.mak index 931dd03a64..df158e1d9d 100644 --- a/tests/style-excludes.mak +++ b/tests/style-excludes.mak @@ -14,3 +14,20 @@ exclude_file_name_regexp--sc_prohibit_doubled_word = \ tests/qemu-iotests/142(\.out)? \ tests/qtest/arm-cpu-features\.c \ ui/cursor\.c + +exclude_file_name_regexp--sc_c_file_osdep_h = \ + contrib/plugins/.* \ + linux-user/(mips64|x86_64)/(signal|cpu_loop)\.c \ + pc-bios/.* \ + scripts/coverity-scan/model\.c \ + scripts/xen-detect\.c \ + subprojects/.* \ + target/hexagon/(gen_semantics|gen_dectree_import)\.c \ + target/s390x/gen-features\.c \ + tests/migration/s390x/a-b-bios\.c \ + tests/multiboot/.* \ + tests/plugin/.* \ + tests/tcg/.* \ + tests/uefi-test-tools/.* \ + tests/unit/test-rcu-(simpleq|slist|tailq)\.c \ + tools/ebpf/rss.bpf.c diff --git a/tests/style.mak b/tests/style.mak index 4056bde619..301d978155 100644 --- a/tests/style.mak +++ b/tests/style.mak @@ -52,3 +52,9 @@ sc_prohibit_doubled_word: | $(GREP) . \ && { echo '$(ME): doubled words' 1>&2; exit 1; } \ || : + +sc_c_file_osdep_h: + @require='#include "qemu/osdep.h"' \ + in_vc_files='\.c$$' \ + halt='all C files must include qemu/osdep.h' \ + $(_sc_search_regexp)
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/style-excludes.mak | 17 +++++++++++++++++ tests/style.mak | 6 ++++++ 2 files changed, 23 insertions(+)