mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-10 05:03:43 +01:00
Merge branch 'ticket31176' into ticket31176_merged
This commit is contained in:
commit
cc48eff2d3
@ -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:
|
||||
|
5
changes/ticket31176
Normal file
5
changes/ticket31176
Normal file
@ -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.
|
@ -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
|
||||
|
@ -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:])
|
||||
|
@ -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
|
||||
|
||||
|
285
scripts/maint/practracker/includes.py
Executable file
285
scripts/maint/practracker/includes.py
Executable file
@ -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)
|
@ -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)
|
||||
|
@ -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))
|
||||
|
10
src/core/crypto/.may_include
Normal file
10
src/core/crypto/.may_include
Normal file
@ -0,0 +1,10 @@
|
||||
!advisory
|
||||
|
||||
orconfig.h
|
||||
|
||||
lib/crypt_ops/*.h
|
||||
lib/ctime/*.h
|
||||
lib/cc/*.h
|
||||
lib/log/*.h
|
||||
|
||||
core/crypto/*.h
|
20
src/core/mainloop/.may_include
Normal file
20
src/core/mainloop/.may_include
Normal file
@ -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
|
38
src/core/or/.may_include
Normal file
38
src/core/or/.may_include
Normal file
@ -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
|
10
src/core/proto/.may_include
Normal file
10
src/core/proto/.may_include
Normal file
@ -0,0 +1,10 @@
|
||||
!advisory
|
||||
|
||||
orconfig.h
|
||||
|
||||
lib/crypt_ops/*.h
|
||||
lib/buf/*.h
|
||||
|
||||
trunnel/*.h
|
||||
|
||||
core/proto/*.h
|
Loading…
Reference in New Issue
Block a user