Commit 7c90026 added #include "conf/domain_conf.h" to util/virrandom.c. Fortunately it didn't actually use anything from domain_conf.h, since as far as I'm aware, files in util aren't allowed to reference anything in conf (although the opposite is allowed). So this #include is unnecessary.

I verified it still compiles with the line removed, but have placed a one day moratorium on me doing any "trivial rule" pushes, so will wait for someone else to verify/ACK before pushing. --- src/util/virrandom.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 151cf4b..9092fd2 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -29,7 +29,6 @@ #include "count-one-bits.h" #include "util.h" #include "virterror_internal.h" -#include "conf/domain_conf.h" #define VIR_FROM_THIS VIR_FROM_NONE

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. --- * .gnulib b856fad...44de969 (31): > maint.mk: avoid spurious failure of _sc_search_regexp-using tests > maint.mk: add per-line exclusions to prohibitions > Tests for module 'expl-ieee'. > New module 'expl-ieee'. > Tests for module 'exp-ieee'. > New module 'exp-ieee'. > Tests for module 'expf-ieee'. > New module 'expf-ieee'. > cbrtl-ieee: Work around test failure on IRIX 6.5. > Tests for module 'cbrtl-ieee'. > New module 'cbrtl-ieee'. > Tests for module 'cbrt-ieee'. > New module 'cbrt-ieee'. > Tests for module 'cbrtf-ieee'. > New module 'cbrtf-ieee'. > cbrtf: Work around bug in IRIX 6.5 system function. > Tests for module 'cbrtl'. > New module 'cbrtl'. > Tests for module 'cbrtf'. > New module 'cbrtf'. > cbrt: Provide replacement on MSVC and Minix. > hypotl-ieee: Work around test failure on OSF/1 and native Windows. > hypotf-ieee: Work around test failure on OSF/1 and native Windows. > hypot-ieee: Work around test failure on OSF/1 and native Windows. > Tests for module 'hypotl-ieee'. > New module 'hypotl-ieee'. > Tests for module 'hypot-ieee'. > New module 'hypot-ieee'. > Tests for module 'hypotf-ieee'. > New module 'hypotf-ieee'. > Remove unused variables. .gnulib | 2 +- cfg.mk | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.gnulib b/.gnulib index b856fad..44de969 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit b856fadc1c8dcb53e7efcbb2d0ae7edc022fdb6a +Subproject commit 44de969cd62abbfe3e7cc7641a8dea7673fd2d6d diff --git a/cfg.mk b/cfg.mk index ac6c527..95994df 100644 --- a/cfg.mk +++ b/cfg.mk @@ -348,11 +348,10 @@ sc_prohibit_access_xok: # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0. snp_ = strncmp *\(.+\) sc_prohibit_strncmp: - @grep -nE '! *strncmp *\(|\<$(snp_) *[!=]=|[!=]= *$(snp_)' \ - $$($(VC_LIST_EXCEPT)) \ - | grep -vE ':# *define STR(N?EQLEN|PREFIX)\(' && \ - { echo '$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \ - 1>&2; exit 1; } || : + @prohibit='! *strncmp *\(|\<$(snp_) *[!=]=|[!=]= *$(snp_)' \ + exclude=':# *define STR(N?EQLEN|PREFIX)\(' \ + halt='$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \ + $(_sc_search_regexp) # Use virAsprintf rather than as'printf since *strp is undefined on error. sc_prohibit_asprintf: @@ -569,11 +568,10 @@ func_re := ($(func_or)) # _("...: " # "%s", _("no storage vol w..." sc_libvirt_unmarked_diagnostics: - @grep -nE \ - '\<$(func_re) *\([^"]*"[^"]*[a-z]{3}' $$($(VC_LIST_EXCEPT)) \ - | grep -v '_''(' && \ - { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \ - exit 1; } || : + @prohibit='\<$(func_re) *\([^"]*"[^"]*[a-z]{3}' \ + exclude='_\(' \ + halt='$(ME): found unmarked diagnostic(s)' \ + $(_sc_search_regexp) @{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \ grep -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \ | sed 's/_("[^"][^"]*"//;s/[ ]"%s"//' \ @@ -624,6 +622,26 @@ sc_prohibit_gettext_markup: halt='do not mark these strings for translation' \ $(_sc_search_regexp) +# 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:

On 03/01/2012 09:53 AM, Daniel P. Berrange wrote: > On Thu, Mar 01, 2012 at 11:43:04AM -0500, Laine Stump wrote: >> Commit 7c90026 added #include "conf/domain_conf.h" to >> util/virrandom.c. Fortunately it didn't actually use anything from >> domain_conf.h, since as far as I'm aware, files in util aren't allowed >> to reference anything in conf (although the opposite is allowed). So >> this #include is unnecessary. > > That is correct. util/ must be self-contained Guess what - that sounds like a great syntax rule to write, so we don't slip up in the future. Give me a few minutes, to see what I can come up with. Any other directories that should be avoiding particular includes?