1. Patchew
  2. QEMU
  3. [PATCH v2 0/7] tests: introduce a tree-wide code style checking facility
  4. View patch

[PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files

Daniel P. Berrangé posted 7 patches 3 years, 7 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, John Snow <jsnow@redhat.com>, Jeff Cody <codyprime@gmail.com>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Luc Michel <luc@lmichel.fr>, Damien Hedde <damien.hedde@greensocs.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>, Cleber Rosa <crosa@redhat.com>, Beraldo Leal <bleal@redhat.com>, Michael Roth <michael.roth@amd.com>, Joel Stanley <joel@jms.id.au>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
There is a newer version of this series
[PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files
Posted by Daniel P. Berrangé 3 years, 7 months ago 
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/style-excludes.mak | 17 +++++++++++++++++
 tests/style.mak          |  6 ++++++
 2 files changed, 23 insertions(+)

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)
-- 
2.36.1
Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files
Posted by Peter Maydell 3 years, 7 months ago 
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
Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files
Posted by Daniel P. Berrangé 3 years, 7 months ago 
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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files
Posted by Peter Maydell 3 years, 7 months ago 
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
Re: [PATCH v2 7/7] tests/style: check qemu/osdep.h is included in all .c files
Posted by Daniel P. Berrangé 3 years, 7 months ago 
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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.