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