[libvirt] [PATCH] build: prohibit cross-inclusion

[Daniel P. Berrange]

On Fri, Mar 02, 2012 at 06:40:57AM -0700, Eric Blake wrote:
> On 03/02/2012 03:01 AM, Daniel P. Berrange wrote:
> > On Thu, Mar 01, 2012 at 05:40:19PM -0700, Eric Blake wrote:
> >> Make it easier to detect invalid cross-directory includes, by
> >> adding a syntax check.  The check is designed to be extensible:
> >> the default case lists only the non-driver directories, and
> >> specific directories can list a different set (for example,
> >> util/ can only use itself, network/ can only use itself, util/,
> >> or conf/).
> >>
> >> * .gnulib: Update to latest, for syntax check improvment.
> >> * cfg.mk (sc_prohibit_cross_inclusion): New check.
> >> (sc_prohibit_strncmp, sc_libvirt_unmarked_diagnostics): Simplify.
> >> ---
> 
> >> +# Our code is divided into modular subdirectories for a reason, and
> >> +# lower-level code must not include higher-level headers.
> >> +cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
> >> +cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
> >> +sc_prohibit_cross_inclusion:
> >> +	@for dir in $(cross_dirs); do					\
> >> +	  case $$dir in							\
> >> +	    util/) safe="util";;					\
> >> +	    cpu/ | locking/ | network/ | rpc/ | security/)		\
> >> +	      safe="($$dir|util|conf)";;				\
> >> +	    xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";;		\
> >> +	    *) safe="($$dir|util|conf|cpu|network|locking|rpc|security)";; \
> >> +	  esac;								\
> >> +	  in_vc_files="^src/$$dir"					\
> >> +	  prohibit='^# *include .$(cross_dirs_re)'			\
> >> +	  exclude="# *include .$$safe"					\
> >> +	  halt='unsafe cross-directory include'				\
> >> +	    $(_sc_search_regexp)					\
> >> +	done
> >> +
> >>  # When converting an enum to a string, make sure that we track any new
> >>  # elements added to the enum by using a _LAST marker.
> >>  sc_require_enum_last_marker:
> > 
> > ACK this looks good to me
> 
> Thanks; pushed.
> 
> Hmm, should we change things to drop the -Iutil and instead use #include
> "util/util.h", rather than the current #include "util.h", as a way to
> make things even more obvious which submodule various headers are coming
> from?  But that would be a future patch, and doesn't need to hold up
> this one.

Yeah, I think that could be nice - we've already been doing that with
some of the newer dirs like locking/ and security/.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|