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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list