Skip to content

Commit

Permalink
Merge branch 'ticket31176' into ticket31176_merged
Browse files Browse the repository at this point in the history
  • Loading branch information
nmathewson committed Aug 21, 2019
2 parents 4b1e0dd + 0f4b245 commit cc48eff
Show file tree
Hide file tree
Showing 12 changed files with 462 additions and 181 deletions.
3 changes: 2 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions changes/ticket31176
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions scripts/git/pre-commit.git-hook
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
183 changes: 8 additions & 175 deletions scripts/maint/checkIncludes.py
Original file line number Diff line number Diff line change
@@ -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:])
45 changes: 43 additions & 2 deletions scripts/maint/practracker/exceptions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -112,28 +136,46 @@ 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
problem function-size /src/core/or/connection_or.c:connection_or_client_learned_peer_id() 142
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
Expand Down Expand Up @@ -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

Loading

0 comments on commit cc48eff

Please sign in to comment.