diff options
| -rw-r--r-- | Makefile.am | 3 | ||||
| -rw-r--r-- | changes/ticket31176 | 5 | ||||
| -rwxr-xr-x | scripts/git/pre-commit.git-hook | 4 | ||||
| -rwxr-xr-x | scripts/maint/checkIncludes.py | 183 | ||||
| -rw-r--r-- | scripts/maint/practracker/exceptions.txt | 45 | ||||
| -rwxr-xr-x | scripts/maint/practracker/includes.py | 285 | ||||
| -rwxr-xr-x | scripts/maint/practracker/practracker.py | 23 | ||||
| -rw-r--r-- | scripts/maint/practracker/problem.py | 17 | ||||
| -rw-r--r-- | src/core/crypto/.may_include | 10 | ||||
| -rw-r--r-- | src/core/mainloop/.may_include | 20 | ||||
| -rw-r--r-- | src/core/or/.may_include | 38 | ||||
| -rw-r--r-- | src/core/proto/.may_include | 10 |
12 files changed, 462 insertions, 181 deletions
diff --git a/Makefile.am b/Makefile.am index bb5dee52e9..d3cce3934d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -169,6 +169,7 @@ EXTRA_DIST+= \ scripts/maint/checkIncludes.py \ scripts/maint/checkSpace.pl \ scripts/maint/practracker/exceptions.txt \ + scripts/maint/practracker/includes.py \ scripts/maint/practracker/metrics.py \ scripts/maint/practracker/practracker.py \ scripts/maint/practracker/practracker_tests.py \ @@ -379,7 +380,7 @@ endif check-includes: if USEPYTHON - $(PYTHON) $(top_srcdir)/scripts/maint/checkIncludes.py + $(PYTHON) $(top_srcdir)/scripts/maint/practracker/includes.py endif check-best-practices: diff --git a/changes/ticket31176 b/changes/ticket31176 new file mode 100644 index 0000000000..5fcdeab3af --- /dev/null +++ b/changes/ticket31176 @@ -0,0 +1,5 @@ + o Major features (developer tools): + - Our best-practices tracker now integrates with our include-checker tool + to keep track of the layering violations that we have not yet fixed. + We hope to reduce this number over time to improve Tor's modularity. + Closes ticket 31176. diff --git a/scripts/git/pre-commit.git-hook b/scripts/git/pre-commit.git-hook index 2a29837198..7c7cf88574 100755 --- a/scripts/git/pre-commit.git-hook +++ b/scripts/git/pre-commit.git-hook @@ -36,8 +36,8 @@ elif [ -d src/common ]; then src/tools/*.[ch] fi -if test -e scripts/maint/checkIncludes.py; then - python scripts/maint/checkIncludes.py +if test -e scripts/maint/practracker/includes.py; then + python scripts/maint/practracker/includes.py fi if [ -e scripts/maint/practracker/practracker.py ]; then diff --git a/scripts/maint/checkIncludes.py b/scripts/maint/checkIncludes.py index ec9350b9b1..926b201b35 100755 --- a/scripts/maint/checkIncludes.py +++ b/scripts/maint/checkIncludes.py @@ -1,181 +1,14 @@ #!/usr/bin/python # Copyright 2018 The Tor Project, Inc. See LICENSE file for licensing info. -"""This script looks through all the directories for files matching *.c or - *.h, and checks their #include directives to make sure that only "permitted" - headers are included. +# This file is no longer here; see practracker/includes.py for this +# functionality. This is a stub file that exists so that older git +# hooks will know where to look. - Any #include directives with angle brackets (like #include <stdio.h>) are - ignored -- only directives with quotes (like #include "foo.h") are - considered. +import sys, os - To decide what includes are permitted, this script looks at a .may_include - file in each directory. This file contains empty lines, #-prefixed - comments, filenames (like "lib/foo/bar.h") and file globs (like lib/*/*.h) - for files that are permitted. -""" +dirname = os.path.split(sys.argv[0])[0] +new_location = os.path.join(dirname, "practracker", "includes.py") +python = sys.executable - -from __future__ import print_function - -import fnmatch -import os -import re -import sys - -# Global: Have there been any errors? -trouble = False - -if sys.version_info[0] <= 2: - def open_file(fname): - return open(fname, 'r') -else: - def open_file(fname): - return open(fname, 'r', encoding='utf-8') - -def warn(msg): - print(msg, file=sys.stderr) - -def err(msg): - """ Declare that an error has happened, and remember that there has - been an error. """ - global trouble - trouble = True - print(msg, file=sys.stderr) - -def fname_is_c(fname): - """ Return true iff 'fname' is the name of a file that we should - search for possibly disallowed #include directives. """ - return fname.endswith(".h") or fname.endswith(".c") - -INCLUDE_PATTERN = re.compile(r'\s*#\s*include\s+"([^"]*)"') -RULES_FNAME = ".may_include" - -ALLOWED_PATTERNS = [ - re.compile(r'^.*\*\.(h|inc)$'), - re.compile(r'^.*/.*\.h$'), - re.compile(r'^ext/.*\.c$'), - re.compile(r'^orconfig.h$'), - re.compile(r'^micro-revision.i$'), -] - -def pattern_is_normal(s): - for p in ALLOWED_PATTERNS: - if p.match(s): - return True - return False - -class Rules(object): - """ A 'Rules' object is the parsed version of a .may_include file. """ - def __init__(self, dirpath): - self.dirpath = dirpath - if dirpath.startswith("src/"): - self.incpath = dirpath[4:] - else: - self.incpath = dirpath - self.patterns = [] - self.usedPatterns = set() - - def addPattern(self, pattern): - if not pattern_is_normal(pattern): - warn("Unusual pattern {} in {}".format(pattern, self.dirpath)) - self.patterns.append(pattern) - - def includeOk(self, path): - for pattern in self.patterns: - if fnmatch.fnmatchcase(path, pattern): - self.usedPatterns.add(pattern) - return True - return False - - def applyToLines(self, lines, context=""): - lineno = 0 - for line in lines: - lineno += 1 - m = INCLUDE_PATTERN.match(line) - if m: - include = m.group(1) - if not self.includeOk(include): - err("Forbidden include of {} on line {}{}".format( - include, lineno, context)) - - def applyToFile(self, fname): - with open_file(fname) as f: - #print(fname) - self.applyToLines(iter(f), " of {}".format(fname)) - - def noteUnusedRules(self): - for p in self.patterns: - if p not in self.usedPatterns: - print("Pattern {} in {} was never used.".format(p, self.dirpath)) - - def getAllowedDirectories(self): - allowed = [] - for p in self.patterns: - m = re.match(r'^(.*)/\*\.(h|inc)$', p) - if m: - allowed.append(m.group(1)) - continue - m = re.match(r'^(.*)/[^/]*$', p) - if m: - allowed.append(m.group(1)) - continue - - return allowed - -def load_include_rules(fname): - """ Read a rules file from 'fname', and return it as a Rules object. """ - result = Rules(os.path.split(fname)[0]) - with open_file(fname) as f: - for line in f: - line = line.strip() - if line.startswith("#") or not line: - continue - result.addPattern(line) - return result - -list_unused = False -log_sorted_levels = False - -uses_dirs = { } - -for dirpath, dirnames, fnames in os.walk("src"): - if ".may_include" in fnames: - rules = load_include_rules(os.path.join(dirpath, RULES_FNAME)) - for fname in fnames: - if fname_is_c(fname): - rules.applyToFile(os.path.join(dirpath,fname)) - if list_unused: - rules.noteUnusedRules() - - uses_dirs[rules.incpath] = rules.getAllowedDirectories() - -if trouble: - err( -"""To change which includes are allowed in a C file, edit the {} -files in its enclosing directory.""".format(RULES_FNAME)) - sys.exit(1) - -all_levels = [] - -n = 0 -while uses_dirs: - n += 0 - cur_level = [] - for k in list(uses_dirs): - uses_dirs[k] = [ d for d in uses_dirs[k] - if (d in uses_dirs and d != k)] - if uses_dirs[k] == []: - cur_level.append(k) - for k in cur_level: - del uses_dirs[k] - n += 1 - if cur_level and log_sorted_levels: - print(n, cur_level) - if n > 100: - break - -if uses_dirs: - print("There are circular .may_include dependencies in here somewhere:", - uses_dirs) - sys.exit(1) +os.execl(python, python, new_location, *sys.argv[1:]) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 4f3943f21c..ce6004fe16 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -52,6 +52,11 @@ problem function-size /src/app/main/main.c:tor_init() 137 problem function-size /src/app/main/main.c:sandbox_init_filter() 291 problem function-size /src/app/main/main.c:run_tor_main_loop() 105 problem function-size /src/app/main/ntmain.c:nt_service_install() 126 +problem dependency-violation /src/core/crypto/hs_ntor.c 1 +problem dependency-violation /src/core/crypto/onion_crypto.c 5 +problem dependency-violation /src/core/crypto/onion_fast.c 1 +problem dependency-violation /src/core/crypto/onion_tap.c 3 +problem dependency-violation /src/core/crypto/relay_crypto.c 9 problem file-size /src/core/mainloop/connection.c 5569 problem include-count /src/core/mainloop/connection.c 62 problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 185 @@ -64,34 +69,51 @@ problem function-size /src/core/mainloop/connection.c:connection_handle_read_imp problem function-size /src/core/mainloop/connection.c:connection_buf_read_from_socket() 180 problem function-size /src/core/mainloop/connection.c:connection_handle_write_impl() 241 problem function-size /src/core/mainloop/connection.c:assert_connection_ok() 143 +problem dependency-violation /src/core/mainloop/connection.c 44 +problem dependency-violation /src/core/mainloop/cpuworker.c 12 problem include-count /src/core/mainloop/mainloop.c 63 problem function-size /src/core/mainloop/mainloop.c:conn_close_if_marked() 108 problem function-size /src/core/mainloop/mainloop.c:run_connection_housekeeping() 123 +problem dependency-violation /src/core/mainloop/mainloop.c 49 +problem dependency-violation /src/core/mainloop/mainloop_pubsub.c 1 +problem dependency-violation /src/core/mainloop/mainloop_sys.c 1 +problem dependency-violation /src/core/mainloop/netstatus.c 4 +problem dependency-violation /src/core/mainloop/periodic.c 2 +problem dependency-violation /src/core/or/address_set.c 1 problem file-size /src/core/or/channel.c 3487 problem file-size /src/core/or/channel.h 780 +problem dependency-violation /src/core/or/channel.c 9 +problem dependency-violation /src/core/or/channelpadding.c 6 problem function-size /src/core/or/channeltls.c:channel_tls_handle_var_cell() 160 problem function-size /src/core/or/channeltls.c:channel_tls_process_versions_cell() 170 problem function-size /src/core/or/channeltls.c:channel_tls_process_netinfo_cell() 214 problem function-size /src/core/or/channeltls.c:channel_tls_process_certs_cell() 246 problem function-size /src/core/or/channeltls.c:channel_tls_process_authenticate_cell() 202 +problem dependency-violation /src/core/or/channeltls.c 10 problem include-count /src/core/or/circuitbuild.c 54 problem function-size /src/core/or/circuitbuild.c:get_unique_circ_id_by_chan() 128 problem function-size /src/core/or/circuitbuild.c:circuit_extend() 147 problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 206 +problem dependency-violation /src/core/or/circuitbuild.c 25 problem include-count /src/core/or/circuitlist.c 55 problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 109 problem function-size /src/core/or/circuitlist.c:circuit_free_() 143 problem function-size /src/core/or/circuitlist.c:circuit_find_to_cannibalize() 101 problem function-size /src/core/or/circuitlist.c:circuit_about_to_free() 120 problem function-size /src/core/or/circuitlist.c:circuits_handle_oom() 117 +problem dependency-violation /src/core/or/circuitlist.c 19 problem function-size /src/core/or/circuitmux.c:circuitmux_set_policy() 109 problem function-size /src/core/or/circuitmux.c:circuitmux_attach_circuit() 113 +problem dependency-violation /src/core/or/circuitmux_ewma.c 2 problem file-size /src/core/or/circuitpadding.c 3043 problem function-size /src/core/or/circuitpadding.c:circpad_machine_schedule_padding() 107 problem file-size /src/core/or/circuitpadding.h 809 +problem dependency-violation /src/core/or/circuitpadding.c 6 problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_relay_hide_intro_circuits() 103 problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_client_hide_rend_circuits() 112 +problem dependency-violation /src/core/or/circuitpadding_machines.c 1 problem function-size /src/core/or/circuitstats.c:circuit_build_times_parse_state() 123 +problem dependency-violation /src/core/or/circuitstats.c 11 problem file-size /src/core/or/circuituse.c 3162 problem function-size /src/core/or/circuituse.c:circuit_is_acceptable() 128 problem function-size /src/core/or/circuituse.c:circuit_expire_building() 394 @@ -100,8 +122,10 @@ problem function-size /src/core/or/circuituse.c:circuit_build_failed() 149 problem function-size /src/core/or/circuituse.c:circuit_launch_by_extend_info() 108 problem function-size /src/core/or/circuituse.c:circuit_get_open_circ_or_launch() 352 problem function-size /src/core/or/circuituse.c:connection_ap_handshake_attach_circuit() 244 +problem dependency-violation /src/core/or/circuituse.c 23 problem function-size /src/core/or/command.c:command_process_create_cell() 156 problem function-size /src/core/or/command.c:command_process_relay_cell() 132 +problem dependency-violation /src/core/or/command.c 8 problem file-size /src/core/or/connection_edge.c 4596 problem include-count /src/core/or/connection_edge.c 65 problem function-size /src/core/or/connection_edge.c:connection_ap_expire_beginning() 117 @@ -112,6 +136,7 @@ problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_sen problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_socks_resolved() 101 problem function-size /src/core/or/connection_edge.c:connection_exit_begin_conn() 185 problem function-size /src/core/or/connection_edge.c:connection_exit_connect() 102 +problem dependency-violation /src/core/or/connection_edge.c 27 problem file-size /src/core/or/connection_or.c 3122 problem include-count /src/core/or/connection_or.c 51 problem function-size /src/core/or/connection_or.c:connection_or_group_set_badness_() 105 @@ -119,21 +144,38 @@ problem function-size /src/core/or/connection_or.c:connection_or_client_learned_ problem function-size /src/core/or/connection_or.c:connection_or_compute_authenticate_cell_body() 231 problem file-size /src/core/or/or.h 1103 problem include-count /src/core/or/or.h 49 +problem dependency-violation /src/core/or/connection_or.c 20 +problem dependency-violation /src/core/or/dos.c 5 +problem dependency-violation /src/core/or/onion.c 2 +problem dependency-violation /src/core/or/or_periodic.c 1 problem file-size /src/core/or/policies.c 3249 problem function-size /src/core/or/policies.c:policy_summarize() 107 +problem dependency-violation /src/core/or/policies.c 14 problem function-size /src/core/or/protover.c:protover_all_supported() 117 -problem file-size /src/core/or/relay.c 3264 problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 127 +problem file-size /src/core/or/relay.c 3263 +problem dependency-violation /src/core/or/reasons.c 2 problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 109 problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 192 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 137 problem function-size /src/core/or/relay.c:handle_relay_cell_command() 369 problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 128 problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 146 +problem dependency-violation /src/core/or/relay.c 16 +problem dependency-violation /src/core/or/scheduler.c 1 problem function-size /src/core/or/scheduler_kist.c:kist_scheduler_run() 171 +problem dependency-violation /src/core/or/scheduler_kist.c 2 problem function-size /src/core/or/scheduler_vanilla.c:vanilla_scheduler_run() 109 +problem dependency-violation /src/core/or/scheduler_vanilla.c 1 +problem dependency-violation /src/core/or/sendme.c 2 +problem dependency-violation /src/core/or/status.c 12 problem function-size /src/core/or/versions.c:tor_version_parse() 104 +problem dependency-violation /src/core/proto/proto_cell.c 3 +problem dependency-violation /src/core/proto/proto_control0.c 1 +problem dependency-violation /src/core/proto/proto_ext_or.c 2 +problem dependency-violation /src/core/proto/proto_http.c 1 problem function-size /src/core/proto/proto_socks.c:parse_socks_client() 110 +problem dependency-violation /src/core/proto/proto_socks.c 8 problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 109 problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 126 problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108 @@ -283,4 +325,3 @@ problem function-size /src/tools/tor-gencert.c:parse_commandline() 111 problem function-size /src/tools/tor-resolve.c:build_socks5_resolve_request() 102 problem function-size /src/tools/tor-resolve.c:do_resolve() 171 problem function-size /src/tools/tor-resolve.c:main() 112 - diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py new file mode 100755 index 0000000000..fcc527c983 --- /dev/null +++ b/scripts/maint/practracker/includes.py @@ -0,0 +1,285 @@ +#!/usr/bin/python +# Copyright 2018 The Tor Project, Inc. See LICENSE file for licensing info. + +"""This script looks through all the directories for files matching *.c or + *.h, and checks their #include directives to make sure that only "permitted" + headers are included. + + Any #include directives with angle brackets (like #include <stdio.h>) are + ignored -- only directives with quotes (like #include "foo.h") are + considered. + + To decide what includes are permitted, this script looks at a .may_include + file in each directory. This file contains empty lines, #-prefixed + comments, filenames (like "lib/foo/bar.h") and file globs (like lib/*/*.h) + for files that are permitted. +""" + + +from __future__ import print_function + +import fnmatch +import os +import re +import sys + +if sys.version_info[0] <= 2: + def open_file(fname): + return open(fname, 'r') +else: + def open_file(fname): + return open(fname, 'r', encoding='utf-8') + +def warn(msg): + print(msg, file=sys.stderr) + +def fname_is_c(fname): + """ Return true iff 'fname' is the name of a file that we should + search for possibly disallowed #include directives. """ + return fname.endswith(".h") or fname.endswith(".c") + +INCLUDE_PATTERN = re.compile(r'\s*#\s*include\s+"([^"]*)"') +RULES_FNAME = ".may_include" + +ALLOWED_PATTERNS = [ + re.compile(r'^.*\*\.(h|inc)$'), + re.compile(r'^.*/.*\.h$'), + re.compile(r'^ext/.*\.c$'), + re.compile(r'^orconfig.h$'), + re.compile(r'^micro-revision.i$'), +] + +def pattern_is_normal(s): + for p in ALLOWED_PATTERNS: + if p.match(s): + return True + return False + +class Error(object): + def __init__(self, location, msg, is_advisory=False): + self.location = location + self.msg = msg + self.is_advisory = is_advisory + + def __str__(self): + return "{} at {}".format(self.msg, self.location) + +class Rules(object): + """ A 'Rules' object is the parsed version of a .may_include file. """ + def __init__(self, dirpath): + self.dirpath = dirpath + if dirpath.startswith("src/"): + self.incpath = dirpath[4:] + else: + self.incpath = dirpath + self.patterns = [] + self.usedPatterns = set() + self.is_advisory = False + + def addPattern(self, pattern): + if pattern == "!advisory": + self.is_advisory = True + return + if not pattern_is_normal(pattern): + warn("Unusual pattern {} in {}".format(pattern, self.dirpath)) + self.patterns.append(pattern) + + def includeOk(self, path): + for pattern in self.patterns: + if fnmatch.fnmatchcase(path, pattern): + self.usedPatterns.add(pattern) + return True + return False + + def applyToLines(self, lines, loc_prefix=""): + lineno = 0 + for line in lines: + lineno += 1 + m = INCLUDE_PATTERN.match(line) + if m: + include = m.group(1) + if not self.includeOk(include): + yield Error("{}{}".format(loc_prefix,str(lineno)), + "Forbidden include of {}".format(include), + is_advisory=self.is_advisory) + + def applyToFile(self, fname, f): + for error in self.applyToLines(iter(f), "{}:".format(fname)): + yield error + + def noteUnusedRules(self): + for p in self.patterns: + if p not in self.usedPatterns: + warn("Pattern {} in {} was never used.".format(p, self.dirpath)) + + def getAllowedDirectories(self): + allowed = [] + for p in self.patterns: + m = re.match(r'^(.*)/\*\.(h|inc)$', p) + if m: + allowed.append(m.group(1)) + continue + m = re.match(r'^(.*)/[^/]*$', p) + if m: + allowed.append(m.group(1)) + continue + + return allowed + +include_rules_cache = {} + +def load_include_rules(fname): + """ Read a rules file from 'fname', and return it as a Rules object. + Return 'None' if fname does not exist. + """ + if fname in include_rules_cache: + return include_rules_cache[fname] + if not os.path.exists(fname): + include_rules_cache[fname] = None + return None + result = Rules(os.path.split(fname)[0]) + with open_file(fname) as f: + for line in f: + line = line.strip() + if line.startswith("#") or not line: + continue + result.addPattern(line) + include_rules_cache[fname] = result + return result + +def get_all_include_rules(): + """Return a list of all the Rules objects we have loaded so far, + sorted by their directory names.""" + return [ rules for (fname,rules) in + sorted(include_rules_cache.items()) + if rules is not None ] + +def remove_self_edges(graph): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects). + + Remove all edges from a node to itself.""" + + for k in list(graph): + graph[k] = [ d for d in graph[k] if d != k ] + +def toposort(graph, limit=100): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects). Tries to + perform a topological sort on the graph, arranging the nodes into + "levels", such that every member of each level is only reachable + by members of later levels. + + Returns a list of the members of each level. + + Modifies the input graph, removing every member that could be + sorted. If the graph does not become empty, then it contains a + cycle. + + "limit" is the max depth of the graph after which we give up trying + to sort it and conclude we have a cycle. + """ + all_levels = [] + + n = 0 + while graph: + n += 0 + cur_level = [] + all_levels.append(cur_level) + for k in list(graph): + graph[k] = [ d for d in graph[k] if d in graph ] + if graph[k] == []: + cur_level.append(k) + for k in cur_level: + del graph[k] + n += 1 + if n > limit: + break + + return all_levels + +def consider_include_rules(fname, f): + dirpath = os.path.split(fname)[0] + rules_fname = os.path.join(dirpath, RULES_FNAME) + rules = load_include_rules(os.path.join(dirpath, RULES_FNAME)) + if rules is None: + return + + for err in rules.applyToFile(fname, f): + yield err + + list_unused = False + log_sorted_levels = False + +def walk_c_files(topdir="src"): + """Run through all c and h files under topdir, looking for + include-rule violations. Yield those violations.""" + + for dirpath, dirnames, fnames in os.walk(topdir): + for fname in fnames: + if fname_is_c(fname): + fullpath = os.path.join(dirpath,fname) + with open(fullpath) as f: + for err in consider_include_rules(fullpath, f): + yield err + +def run_check_includes(topdir, list_unused=False, log_sorted_levels=False, + list_advisories=False): + trouble = False + + for err in walk_c_files(topdir): + if err.is_advisory and not list_advisories: + continue + print(err, file=sys.stderr) + if not err.is_advisory: + trouble = True + + if trouble: + err( + """To change which includes are allowed in a C file, edit the {} + files in its enclosing directory.""".format(RULES_FNAME)) + sys.exit(1) + + if list_unused: + for rules in get_all_include_rules(): + rules.noteUnusedRules() + + uses_dirs = { } + for rules in get_all_include_rules(): + uses_dirs[rules.incpath] = rules.getAllowedDirectories() + + remove_self_edges(uses_dirs) + all_levels = toposort(uses_dirs) + + if log_sorted_levels: + for (n, cur_level) in enumerate(all_levels): + if cur_level: + print(n, cur_level) + + if uses_dirs: + print("There are circular .may_include dependencies in here somewhere:", + uses_dirs) + sys.exit(1) + +def main(argv): + import argparse + + progname = argv[0] + parser = argparse.ArgumentParser(prog=progname) + parser.add_argument("--toposort", action="store_true", + help="Print a topologically sorted list of modules") + parser.add_argument("--list-unused", action="store_true", + help="List unused lines in .may_include files.") + parser.add_argument("--list-advisories", action="store_true", + help="List advisories as well as forbidden includes") + parser.add_argument("topdir", default="src", nargs="?", + help="Top-level directory for the tor source") + args = parser.parse_args(argv[1:]) + + run_check_includes(topdir=args.topdir, + log_sorted_levels=args.toposort, + list_unused=args.list_unused, + list_advisories=args.list_advisories) + +if __name__ == '__main__': + main(sys.argv) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 0fdfd4a40a..6483b88da1 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -25,6 +25,7 @@ import os, sys import metrics import util import problem +import includes # The filename of the exceptions file (it should be placed in the practracker directory) EXCEPTIONS_FNAME = "./exceptions.txt" @@ -39,12 +40,15 @@ MAX_INCLUDE_COUNT = 50 MAX_H_FILE_SIZE = 500 # Recommended include count for headers MAX_H_INCLUDE_COUNT = 15 +# Recommended number of dependency violations +MAX_DEP_VIOLATIONS = 0 # Map from problem type to functions that adjust for tolerance TOLERANCE_FNS = { 'include-count': lambda n: int(n*1.1), 'function-size': lambda n: int(n*1.1), - 'file-size': lambda n: int(n*1.02) + 'file-size': lambda n: int(n*1.02), + 'dependency-violation': lambda n: (n+2) } ####################################################### @@ -83,6 +87,14 @@ def consider_function_size(fname, f): canonical_function_name = "%s:%s()" % (fname, name) yield problem.FunctionSizeItem(canonical_function_name, lines) +def consider_include_violations(fname, real_fname, f): + n = 0 + for item in includes.consider_include_rules(real_fname, f): + n += 1 + if n: + yield problem.DependencyViolationItem(fname, n) + + ####################################################### def consider_all_metrics(files_list): @@ -98,6 +110,7 @@ def consider_metrics_for_file(fname, f): Yield a sequence of problem.Item objects for all of the metrics in 'f'. """ + real_fname = fname # Strip the useless part of the path if fname.startswith(TOR_TOPDIR): fname = fname[len(TOR_TOPDIR):] @@ -116,6 +129,11 @@ def consider_metrics_for_file(fname, f): for item in consider_function_size(fname, f): yield item + # Check for "upward" includes + f.seek(0) + for item in consider_include_violations(fname, real_fname, f): + yield item + HEADER="""\ # Welcome to the exceptions file for Tor's best-practices tracker! # @@ -175,6 +193,8 @@ def main(argv): help="Maximum includes per C file") parser.add_argument("--max-function-size", default=MAX_FUNCTION_SIZE, help="Maximum lines per function") + parser.add_argument("--max-dependency-violations", default=MAX_DEP_VIOLATIONS, + help="Maximum number of dependency violations to allow") parser.add_argument("topdir", default=".", nargs="?", help="Top-level directory for the tor source") args = parser.parse_args(argv[1:]) @@ -193,6 +213,7 @@ def main(argv): filt.addThreshold(problem.FileSizeItem("*.h", int(args.max_h_file_size))) filt.addThreshold(problem.IncludeCountItem("*.h", int(args.max_h_include_count))) filt.addThreshold(problem.FunctionSizeItem("*.c", int(args.max_function_size))) + filt.addThreshold(problem.DependencyViolationItem("*", int(args.max_dependency_violations))) # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR) diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 13c8e55143..88e1044fb7 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -198,6 +198,21 @@ class FunctionSizeItem(Item): def __init__(self, problem_location, metric_value): super(FunctionSizeItem, self).__init__("function-size", problem_location, metric_value) +class DependencyViolationItem(Item): + """ + Denotes a dependency violation in a .c or .h file. A dependency violation + occurs when a file includes a file from some module that is not listed + in its .may_include file. + + The 'problem_location' is the file that contains the problem. + + The 'metric_value' is the number of forbidden includes. + """ + def __init__(self, problem_location, metric_value): + super(DependencyViolationItem, self).__init__("dependency-violation", + problem_location, + metric_value) + comment_re = re.compile(r'#.*$') def get_old_problem_from_exception_str(exception_str): @@ -219,5 +234,7 @@ def get_old_problem_from_exception_str(exception_str): return IncludeCountItem(problem_location, metric_value) elif problem_type == "function-size": return FunctionSizeItem(problem_location, metric_value) + elif problem_type == "dependency-violation": + return DependencyViolationItem(problem_location, metric_value) else: raise ValueError("Unknown exception type {!r}".format(orig_str)) diff --git a/src/core/crypto/.may_include b/src/core/crypto/.may_include new file mode 100644 index 0000000000..5782a36797 --- /dev/null +++ b/src/core/crypto/.may_include @@ -0,0 +1,10 @@ +!advisory + +orconfig.h + +lib/crypt_ops/*.h +lib/ctime/*.h +lib/cc/*.h +lib/log/*.h + +core/crypto/*.h diff --git a/src/core/mainloop/.may_include b/src/core/mainloop/.may_include new file mode 100644 index 0000000000..79d6a130a4 --- /dev/null +++ b/src/core/mainloop/.may_include @@ -0,0 +1,20 @@ +!advisory + +orconfig.h + +lib/container/*.h +lib/dispatch/*.h +lib/evloop/*.h +lib/pubsub/*.h +lib/subsys/*.h +lib/buf/*.h +lib/crypt_ops/*.h +lib/err/*.h +lib/tls/*.h +lib/net/*.h +lib/evloop/*.h +lib/geoip/*.h +lib/sandbox/*.h +lib/compress/*.h + +core/mainloop/*.h
\ No newline at end of file diff --git a/src/core/or/.may_include b/src/core/or/.may_include new file mode 100644 index 0000000000..5173e8a2b6 --- /dev/null +++ b/src/core/or/.may_include @@ -0,0 +1,38 @@ +!advisory + +orconfig.h + +lib/arch/*.h +lib/buf/*.h +lib/cc/*.h +lib/compress/*.h +lib/container/*.h +lib/crypt_ops/*.h +lib/ctime/*.h +lib/defs/*.h +lib/encoding/*.h +lib/err/*.h +lib/evloop/*.h +lib/fs/*.h +lib/geoip/*.h +lib/intmath/*.h +lib/log/*.h +lib/malloc/*.h +lib/math/*.h +lib/net/*.h +lib/pubsub/*.h +lib/string/*.h +lib/subsys/*.h +lib/test/*.h +lib/testsupport/*.h +lib/thread/*.h +lib/time/*.h +lib/tls/*.h +lib/wallclock/*.h + +trunnel/*.h + +core/mainloop/*.h +core/proto/*.h +core/crypto/*.h +core/or/*.h
\ No newline at end of file diff --git a/src/core/proto/.may_include b/src/core/proto/.may_include new file mode 100644 index 0000000000..c1647a5cf9 --- /dev/null +++ b/src/core/proto/.may_include @@ -0,0 +1,10 @@ +!advisory + +orconfig.h + +lib/crypt_ops/*.h +lib/buf/*.h + +trunnel/*.h + +core/proto/*.h
\ No newline at end of file |