[libvirt] [PATCH 07/10] syntax-check: Include libvirt.h and virterror.h in <> form only in external tools
Eric Blake
eblake at redhat.com
Wed Apr 17 04:03:59 UTC 2013
On 04/16/2013 07:41 AM, Osier Yang wrote:
> With this patch, include "libvirt.h" and "virterror.h" in "" form
> is only allowed for "internal.h". And only the external tools
> (examples|tools|python|include/libvirt) can include the two headers
> in <> form.
> ---
Hmm. It sounds like you want two syntax checks after all, in order to
allow <> but not "" in the subdirectories; but it can still be done in
two rules instead of four.
> cfg.mk | 30 ++++++++++++++++++++++++++----
> include/libvirt/libvirt-lxc.h | 2 +-
> include/libvirt/libvirt-qemu.h | 2 +-
> python/libvirt-lxc-override.c | 4 ++--
> python/libvirt-override.c | 4 ++--
> python/libvirt-qemu-override.c | 4 ++--
> python/typewrappers.h | 4 ++--
> tests/shunloadhelper.c | 2 --
> 8 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index cb8079c..98c7e40 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -748,14 +748,30 @@ sc_prohibit_duplicate_header:
> sc_prohibit_include_libvirt_h:
I'd name this sc_prohibit_include_libvirt_quote (that affects patch 6);
see below for why...
> @prohibit='^# *include *"libvirt/libvirt\.h"' \
> in_vc_files='\.[ch]$$' \
> - halt='Do not include libvirt/libvirt.h' \
> + halt='Do not include libvirt/libvirt.h in internal source' \
Squash this wording into patch 6.
> $(_sc_search_regexp)
>
> # Don't include "libvirt/virterror.h" in "" form.
> sc_prohibit_include_virterror_h:
> @prohibit='^# *include *"libvirt/virterror\.h"' \
> in_vc_files='\.[ch]$$' \
> - halt='Do not include libvirt/virterror.h' \
> + halt='Do not include libvirt/virterror.h in internal source' \
> + $(_sc_search_regexp)
> +
> +# Don't include "libvirt/libvirt.h" in <> form. Except external tools, e.g.
> +# python binding, examples and tools subdirectories.
> +sc_prohibit_include_libvirt_h_1:
...here's why. The _1 suffix doesn't describe anything. So I'd name
this sc_prohibit_include_libvirt_brackets
> + @prohibit='^# *include *<libvirt/libvirt\.h>' \
> + in_vc_files='\.[ch]$$' \
> + halt='Do not include libvirt/libvirt.h in internal source' \
> + $(_sc_search_regexp)
> +
> +# Don't include "libvirt/virterror.h" in <> form. Except external tools, e.g.
> +# python binding, examples and tools subdirectories.
> +sc_prohibit_include_virterror_h_1:
> + @prohibit='^# *include *<libvirt/virterror\.h>' \
> + in_vc_files='\.[ch]$$' \
> + halt='Do not include libvirt/virterror.h in internal source' \
> $(_sc_search_regexp)
Again, these two can be merged into one.
>
> # We don't use this feature of maint.mk.
> @@ -913,7 +929,13 @@ exclude_file_name_regexp--sc_correct_id_types = \
> exclude_file_name_regexp--sc_m4_quote_check = m4/virt-lib.m4
>
> exclude_file_name_regexp--sc_prohibit_include_libvirt_h = \
> - ^(src/internal\.h)|(include/libvirt/libvirt-(lxc|qemu)\.h)|(python/libvirt-override\.c)|(python/typewrappers\.h)$$
> + ^src/internal\.h$$
>
> exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
> - ^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
> + ^src/internal\.h$$
> +
> +exclude_file_name_regexp--sc_prohibit_include_libvirt_h_1 = \
> + ^(examples/|tools/|python/|include/libvirt/)
> +
> +exclude_file_name_regexp--sc_prohibit_include_virterror_h_1 = \
> + ^(examples/|tools/|python/|include/libvirt/)
Maybe it's even worth merging 6 and 7 into a single patch, or splitting
the cleanup into a first patch, and the syntax check into a second.
> +++ b/tests/shunloadhelper.c
> @@ -28,8 +28,6 @@
> #include <config.h>
> #include "internal.h"
>
> -#include <libvirt/libvirt.h>
> -#include <libvirt/virterror.h>
> #include <stdlib.h>
>
> static void shunloadError(void *userData ATTRIBUTE_UNUSED,
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130416/6bbf805a/attachment-0001.sig>
More information about the libvir-list
mailing list