On 03/25/2014 02:14 PM, Eric Blake wrote: > In 'make syntax-check', we have a rule that prevents layering > violations between the various files in src. However, we > forgot to treat conf/ and the more recently-added access/ as > lower-level directories, and were not detecting cases where > they might have used a driver file. Also, it's not nice that > qemu can use storage/ but none of the other drivers could do so. > > * cfg.mk (sc_prohibit_cross_inclusion): Tighten rules for conf/ > and access/, let all other drivers use storage/. > > Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> > --- > > I noticed this because of my work on domain_conf.h: I want to share > a struct between util/virstoragefile and conf/domain_conf, and ran > into a syntax check when I tried to make util/ depend on conf/. I > fixed things to obey layering with conf/ depending on util/, but in > the process noticed that some layering violations went undetected. Looks like I pushed this without a review, as part of my virstoragefile patches. I also noticed that the rule isn't catching all violations; it catches cases of #include "conf/domain_conf.h" but not "#include "domain_conf.h". So to make it even more useful, it's probably worth an audit of the code base to remove things like '-I util' and '-I conf' from Makefile.am, as well as rewrite all includes to explicitly mention which directory they are expecting to pull headers from, so that cross-directory includes are more obvious. Looks like a bigger cleanup, so more patches on the way, although not currently my highest priority. > +++ b/cfg.mk > @@ -760,17 +760,17 @@ sc_prohibit_gettext_markup: > # lower-level code must not include higher-level headers. > cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) > cross_dirs_re=($(subst / ,/|,$(cross_dirs))) > +mid_dirs=access|conf|cpu|locking|network|node_device|rpc|security|storage > sc_prohibit_cross_inclusion: > @for dir in $(cross_dirs); do \ > case $$dir in \ > util/) safe="util";; \ > - locking/) \ > - safe="($$dir|util|conf|rpc)";; \ > - cpu/ | locking/ | network/ | rpc/ | security/) \ > + access/ | conf/) safe="($$dir|conf|util)";; \ > + locking/) safe="($$dir|util|conf|rpc)";; \ > + cpu/| network/| node_device/| rpc/| security/| storage/) \ > safe="($$dir|util|conf)";; \ > xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ > - qemu/ ) safe="($$dir|util|conf|cpu|network|locking|rpc|security|storage)";; \ > - *) safe="($$dir|util|conf|cpu|network|locking|rpc|security)";; \ > + *) safe="($$dir|$(mid_dirs)|util)";; \ > esac; \ > in_vc_files="^src/$$dir" \ > prohibit='^# *include .$(cross_dirs_re)' \ > -- 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