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(a)redhat.com>
---
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