[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 07/10] syntax-check: Include libvirt.h and virterror.h in <> form only in external tools



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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]