From 17dd3167494d69b7afcfdb21395aad135b322722 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 15:14:19 +0200
Subject: [PATCH 01/17] Initial commit of practracker.py .

---
 scripts/maint/practracker/metrics.py     |  31 ++++++
 scripts/maint/practracker/practracker.py | 119 +++++++++++++++++++++++
 scripts/maint/practracker/util.py        |  23 +++++
 3 files changed, 173 insertions(+)
 create mode 100644 scripts/maint/practracker/metrics.py
 create mode 100755 scripts/maint/practracker/practracker.py
 create mode 100644 scripts/maint/practracker/util.py

diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py
new file mode 100644
index 0000000000..43ec3f809d
--- /dev/null
+++ b/scripts/maint/practracker/metrics.py
@@ -0,0 +1,31 @@
+#!/usr/bin/python
+
+import re
+
+def file_len(f):
+    """Get file length of file"""
+    for i, l in enumerate(f):
+        pass
+    return i + 1
+
+def function_lines(f):
+    """
+    Return iterator which iterates over functions and returns (function name, function lines)
+    """
+
+    # XXX Buggy! Doesn't work with MOCK_IMPL and ENABLE_GCC_WARNINGS
+    in_function = False
+    for lineno, line in enumerate(f):
+        if not in_function:
+            # find the start of a function
+            m = re.match(r'^([a-zA-Z_][a-zA-Z_0-9]*),?\(', line)
+            if m:
+                func_name = m.group(1)
+                func_start = lineno
+                in_function = True
+        else:
+            # Fund the end of a function
+            if line.startswith("}"):
+                n_lines = lineno - func_start
+                in_function = False
+                yield (func_name, n_lines)
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
new file mode 100755
index 0000000000..c95c432c39
--- /dev/null
+++ b/scripts/maint/practracker/practracker.py
@@ -0,0 +1,119 @@
+#!/usr/bin/python
+
+"""
+Tor code best-practices tracker
+
+Go through the various .c files and collect metrics about them. If the metrics
+violate some of our best practices and they are not found in the optional
+exceptions file ("./exceptions.txt"), then log a violation about them.
+
+The exceptions file is meant to be initialized with the current state of the
+source code as follows: ./practracker.py > ./exceptions.txt
+
+We currently do metrics about file size, function size and number of includes.
+
+TODO:
+    - How is this tool supposed to be used? How should the exception file work?
+      How should the UI work? Does it need special exit codes?
+    - Fix the function_length function so that practracker_tests.py passes.
+"""
+
+import os, sys
+
+import metrics
+import util
+
+# We don't want to run metrics for unittests, automatically-generated C files,
+# external libraries or git leftovers.
+EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"]
+
+# Where the Tor source code is
+TOR_TOPDIR = "../../../"
+# An optional exceptions_file
+EXCEPTIONS_FILE = "./exceptions.txt"
+
+# Recommended file size
+MAX_FILE_SIZE = 3000 # lines
+# Recommended function size
+MAX_FUNCTION_SIZE = 100 # lines
+# Recommended number of #includes
+MAX_INCLUDE_COUNT = 50
+
+#######################################################
+
+def print_violation_if_not_exception(violation_str, exceptions_str):
+    # Check if this violation is already in the optional exceptions file
+    if exceptions_str and violation_str in exceptions_str:
+        return
+
+    print violation_str
+
+#######################################################
+
+def consider_file_size(fname, f, exceptions_str):
+    file_size = metrics.file_len(f)
+    if file_size > MAX_FILE_SIZE:
+        violation_str = "violation file-size %s %d" % (fname, file_size)
+        print_violation_if_not_exception(violation_str, exceptions_str)
+
+def consider_includes(fname, f, exceptions_str):
+    include_count = 0
+    for _, line in enumerate(f):
+        if line.startswith("#include "):
+            include_count += 1
+
+    if include_count > MAX_INCLUDE_COUNT:
+        violation_str = "violation include-count %s %d" % (fname, include_count)
+        print_violation_if_not_exception(violation_str, exceptions_str)
+
+def consider_function_size(fname, f, exceptions_str):
+    for name, lines in metrics.function_lines(f):
+        # Don't worry about functions within our limits
+        if lines <= MAX_FUNCTION_SIZE:
+            continue
+
+        # That's a big function! Issue a violation!
+        canonical_function_name = "%s:%s()" % (fname,name)
+        violation_str = "violation function-size %s %s" % (lines, canonical_function_name)
+        print_violation_if_not_exception(violation_str, exceptions_str)
+
+#######################################################
+
+def consider_all_metrics(files_list, exceptions_str):
+    """Consider metrics for all files"""
+    for fname in files_list:
+        with open(fname, 'r') as f:
+            consider_metrics_for_file(fname, f, exceptions_str)
+
+def consider_metrics_for_file(fname, f, exceptions_str):
+    """
+    Get metrics for file with filename 'fname' and file descriptor 'f'.
+    """
+    # Get file length
+    consider_file_size(fname, f, exceptions_str)
+
+    # Consider number of #includes
+    f.seek(0)
+    consider_includes(fname, f, exceptions_str)
+
+    # Get function length
+    f.seek(0)
+    consider_function_size(fname, f, exceptions_str)
+
+def main():
+    # 1) Get all the .c files we care about
+    files_list = util.get_tor_c_files(TOR_TOPDIR, EXCLUDE_SOURCE_DIRS)
+
+    # 2) Read an optional exceptions file so that we don't warn about the past
+    exceptions_str = None
+    try:
+        with open(EXCEPTIONS_FILE, 'r') as exception_f:
+            exceptions_str = exception_f.read()
+    except IOError:
+        print "No exception file provided"
+
+    # 3) Go through all the files and report violations if they are not exceptions
+    consider_all_metrics(files_list, exceptions_str)
+
+if __name__ == '__main__':
+    main()
diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py
new file mode 100644
index 0000000000..8e9d95ece4
--- /dev/null
+++ b/scripts/maint/practracker/util.py
@@ -0,0 +1,23 @@
+import os
+
+def get_tor_c_files(tor_topdir, exclude_dirs):
+    """
+    Return a list with the .c filenames we want to get metrics of.
+    """
+    files_list = []
+
+    for root, directories, filenames in os.walk(tor_topdir):
+        for filename in filenames:
+            # We only care about .c files
+            if not filename.endswith(".c"):
+                continue
+
+            # Exclude the excluded paths
+            full_path = os.path.join(root,filename)
+            if any(exclude_dir in full_path for exclude_dir in exclude_dirs):
+                continue
+
+            files_list.append(full_path)
+
+    return files_list
+

From a7684fcb57b3d28285b437f20d5eff6ce079cab5 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 15:14:29 +0200
Subject: [PATCH 02/17] Add some failing unittests for the function length
 metric.

---
 .../maint/practracker/practracker_tests.py    | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100755 scripts/maint/practracker/practracker_tests.py

diff --git a/scripts/maint/practracker/practracker_tests.py b/scripts/maint/practracker/practracker_tests.py
new file mode 100755
index 0000000000..f3d225207a
--- /dev/null
+++ b/scripts/maint/practracker/practracker_tests.py
@@ -0,0 +1,46 @@
+"""Some simple tests for practracker metrics"""
+
+import unittest
+
+import StringIO
+
+import metrics
+
+function_file = """static void
+fun(directory_request_t *req, const char *resource)
+{
+  time_t if_modified_since = 0;
+  uint8_t or_diff_from[DIGEST256_LEN];
+}
+
+static void
+fun(directory_request_t *req,
+      const char *resource)
+{
+  time_t if_modified_since = 0;
+  uint8_t or_diff_from[DIGEST256_LEN];
+}
+
+MOCK_IMPL(void,
+fun,(
+       uint8_t dir_purpose,
+       uint8_t router_purpose,
+       const char *resource,
+       int pds_flags,
+       download_want_authority_t want_authority))
+{
+  const routerstatus_t *rs = NULL;
+  const or_options_t *options = get_options();
+}
+"""
+
+class TestFunctionLength(unittest.TestCase):
+    def test_function_length(self):
+        funcs = StringIO.StringIO(function_file)
+        # All functions should have length 2
+        for name, lines in metrics.function_lines(funcs):
+            self.assertEqual(name, "fun")
+            self.assertEqual(lines, 2)
+
+if __name__ == '__main__':
+    unittest.main()

From 371ea65c080471cb6e1a57d9f78e5beaaa4212b8 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 17:05:00 +0200
Subject: [PATCH 03/17] Improve #include counting func and move it to
 metrics.py.

---
 scripts/maint/practracker/metrics.py     | 8 ++++++++
 scripts/maint/practracker/practracker.py | 5 +----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py
index 43ec3f809d..525a058405 100644
--- a/scripts/maint/practracker/metrics.py
+++ b/scripts/maint/practracker/metrics.py
@@ -8,6 +8,14 @@ def file_len(f):
         pass
     return i + 1
 
+def get_include_count(f):
+    """Get number of #include statements in the file"""
+    include_count = 0
+    for line in f:
+        if re.match(r' *# *include', line):
+            include_count += 1
+    return include_count
+
 def function_lines(f):
     """
     Return iterator which iterates over functions and returns (function name, function lines)
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index c95c432c39..c3b8a9783b 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -57,10 +57,7 @@ def consider_file_size(fname, f, exceptions_str):
         print_violation_if_not_exception(violation_str, exceptions_str)
 
 def consider_includes(fname, f, exceptions_str):
-    include_count = 0
-    for _, line in enumerate(f):
-        if line.startswith("#include "):
-            include_count += 1
+    include_count = metrics.get_include_count(f)
 
     if include_count > MAX_INCLUDE_COUNT:
         violation_str = "violation include-count %s %d" % (fname, include_count)

From 26c4f6cfd07fea6d49109e60a8b591b92ec59c49 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 18:24:10 +0200
Subject: [PATCH 04/17] Add more intelligent problem tracking.

---
 scripts/maint/practracker/practracker.py | 55 +++++++-------
 scripts/maint/practracker/problem.py     | 96 ++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 28 deletions(-)
 create mode 100644 scripts/maint/practracker/problem.py

diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index c3b8a9783b..4fbca615c5 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -5,7 +5,7 @@ Tor code best-practices tracker
 
 Go through the various .c files and collect metrics about them. If the metrics
 violate some of our best practices and they are not found in the optional
-exceptions file ("./exceptions.txt"), then log a violation about them.
+exceptions file ("./exceptions.txt"), then log a problem about them.
 
 The exceptions file is meant to be initialized with the current state of the
 source code as follows: ./practracker.py > ./exceptions.txt
@@ -22,6 +22,7 @@ import os, sys
 
 import metrics
 import util
+import problem
 
 # We don't want to run metrics for unittests, automatically-generated C files,
 # external libraries or git leftovers.
@@ -41,76 +42,74 @@ MAX_INCLUDE_COUNT = 50
 
 #######################################################
 
-def print_violation_if_not_exception(violation_str, exceptions_str):
-    # Check if this violation is already in the optional exceptions file
-    if exceptions_str and violation_str in exceptions_str:
-        return
-
-    print violation_str
+ProblemVault = None
 
 #######################################################
 
-def consider_file_size(fname, f, exceptions_str):
+def consider_file_size(fname, f):
     file_size = metrics.file_len(f)
     if file_size > MAX_FILE_SIZE:
-        violation_str = "violation file-size %s %d" % (fname, file_size)
-        print_violation_if_not_exception(violation_str, exceptions_str)
+        v = problem.FileSizeProblem(fname, file_size)
+        ProblemVault.register_problem(v)
 
-def consider_includes(fname, f, exceptions_str):
+def consider_includes(fname, f):
     include_count = metrics.get_include_count(f)
 
     if include_count > MAX_INCLUDE_COUNT:
-        violation_str = "violation include-count %s %d" % (fname, include_count)
-        print_violation_if_not_exception(violation_str, exceptions_str)
+        v = problem.IncludeCountProblem(fname, include_count)
+        ProblemVault.register_problem(v)
 
-def consider_function_size(fname, f, exceptions_str):
+def consider_function_size(fname, f):
     for name, lines in metrics.function_lines(f):
         # Don't worry about functions within our limits
         if lines <= MAX_FUNCTION_SIZE:
             continue
 
-        # That's a big function! Issue a violation!
+        # That's a big function! Issue a problem!
         canonical_function_name = "%s:%s()" % (fname,name)
-        violation_str = "violation function-size %s %s" % (lines, canonical_function_name)
-        print_violation_if_not_exception(violation_str, exceptions_str)
+        v = problem.FunctionSizeProblem(canonical_function_name, lines)
+        ProblemVault.register_problem(v)
 
 #######################################################
 
-def consider_all_metrics(files_list, exceptions_str):
+def consider_all_metrics(files_list):
     """Consider metrics for all files"""
     for fname in files_list:
         with open(fname, 'r') as f:
-            consider_metrics_for_file(fname, f, exceptions_str)
+            consider_metrics_for_file(fname, f)
 
-def consider_metrics_for_file(fname, f, exceptions_str):
+def consider_metrics_for_file(fname, f):
     """
     Get metrics for file with filename 'fname' and file descriptor 'f'.
     """
     # Get file length
-    consider_file_size(fname, f, exceptions_str)
+    consider_file_size(fname, f)
 
     # Consider number of #includes
     f.seek(0)
-    consider_includes(fname, f, exceptions_str)
+    consider_includes(fname, f)
 
     # Get function length
     f.seek(0)
-    consider_function_size(fname, f, exceptions_str)
+    consider_function_size(fname, f)
 
 def main():
+    global ProblemVault
+
     # 1) Get all the .c files we care about
     files_list = util.get_tor_c_files(TOR_TOPDIR, EXCLUDE_SOURCE_DIRS)
 
-    # 2) Read an optional exceptions file so that we don't warn about the past
-    exceptions_str = None
+    # 2) Initialize problem vault and load an optional exceptions file so that
+    # we don't warn about the past
     try:
         with open(EXCEPTIONS_FILE, 'r') as exception_f:
-            exceptions_str = exception_f.read()
+            ProblemVault = problem.ProblemVault(exception_f)
     except IOError:
         print "No exception file provided"
+        ProblemVault = problem.ProblemVault(None)
 
-    # 3) Go through all the files and report violations if they are not exceptions
-    consider_all_metrics(files_list, exceptions_str)
+    # 3) Go through all the files and report problems if they are not exceptions
+    consider_all_metrics(files_list)
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
new file mode 100644
index 0000000000..fe6b30915b
--- /dev/null
+++ b/scripts/maint/practracker/problem.py
@@ -0,0 +1,96 @@
+"""
+In this file we define a ProblemVault class where we store all the
+exceptions and all the problems we find with the code.
+
+The ProblemVault is capable of registering problems and also figuring out if a
+problem is worse than a registered exception so that it only warns when things
+get worse.
+"""
+
+class ProblemVault(object):
+    """
+    Singleton where we store the various new problems we
+    found in the code, and also the old problems we read from the exception
+    file.
+    """
+    def __init__(self, exception_file):
+        # Exception dictionary: { problem.key() : Problem object }
+        self.exceptions = {}
+
+        if exception_file:
+            self.register_exceptions(exception_file)
+
+    def register_exceptions(self, exception_file):
+        # Register exceptions
+        for line in exception_file:
+            problem = get_old_problem_from_exception_str(line)
+            if problem is None:
+                continue
+
+            # XXX this might overwrite problems with the same key (e.g. MOCK_IMPL)
+            self.exceptions[problem.key()] = problem
+            #print "Registering exception: %s" % problem
+
+    def register_problem(self, problem):
+        # This is a new problem, print it
+        if problem.key() not in self.exceptions:
+            print problem
+            return
+
+        # If it's an old problem, we don't warn if the situation got better
+        # (e.g. we went from 4k LoC to 3k LoC), but we do warn if the
+        # situation worsened (e.g. we went from 60 includes to 80).
+        if problem.is_worse_than(self.exceptions[problem.key()]):
+            print problem
+            return
+#        else:
+#            print "OK %s better than %s" % (problem, self.exceptions[problem.key()])
+
+
+class Problem(object):
+    def __init__(self, problem_type, problem_location, metric_value):
+        self.problem_location = problem_location
+        self.metric_value = int(metric_value)
+        self.problem_type = problem_type
+
+    def is_worse_than(self, other_problem):
+        """Return True if this is a worse problem than other_problem"""
+        if self.metric_value > other_problem.metric_value:
+            return True
+        return False
+
+    def key(self):
+        """Generate a unique key that describes this problem that can be used as a dictionary key"""
+        return "%s:%s" % (self.problem_location, self.problem_type)
+
+    def __str__(self):
+        return "problem %s %s %s" % (self.problem_type, self.problem_location, self.metric_value)
+
+class FileSizeProblem(Problem):
+    def __init__(self, problem_location, metric_value):
+        super(FileSizeProblem, self).__init__("file-size", problem_location, metric_value)
+
+class IncludeCountProblem(Problem):
+    def __init__(self, problem_location, metric_value):
+        super(IncludeCountProblem, self).__init__("include-count", problem_location, metric_value)
+
+class FunctionSizeProblem(Problem):
+    def __init__(self, problem_location, metric_value):
+        super(FunctionSizeProblem, self).__init__("function-size", problem_location, metric_value)
+
+def get_old_problem_from_exception_str(exception_str):
+    try:
+        _, problem_type, problem_location, metric_value = exception_str.split(" ")
+    except ValueError:
+        return None
+
+    if problem_type == "file-size":
+        return FileSizeProblem(problem_location, metric_value)
+    elif problem_type == "include-count":
+        return IncludeCountProblem(problem_location, metric_value)
+    elif problem_type == "function-size":
+        return FunctionSizeProblem(problem_location, metric_value)
+    else:
+        print "Unknown exception line %s" % exception_str
+        return None
+

From 2a722e2841c9b1631852affd313cc3d0403da7b9 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 19:29:27 +0200
Subject: [PATCH 05/17] Improve metrics implementation.

No longer messes up with MOCK_IMPL. Also update the tests to show that. We are
still being innacurate on the line count in some cases, but that doesnt matter
so much.
---
 scripts/maint/practracker/metrics.py           | 17 ++++++++++++++---
 scripts/maint/practracker/practracker_tests.py |  4 ++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py
index 525a058405..c7d2091d05 100644
--- a/scripts/maint/practracker/metrics.py
+++ b/scripts/maint/practracker/metrics.py
@@ -1,8 +1,12 @@
 #!/usr/bin/python
 
+# Implementation of various source code metrics.
+# These are currently ad-hoc string operations and regexps.
+# We might want to use a proper static analysis library in the future, if we want to get more advanced metrics.
+
 import re
 
-def file_len(f):
+def get_file_len(f):
     """Get file length of file"""
     for i, l in enumerate(f):
         pass
@@ -16,14 +20,20 @@ def get_include_count(f):
             include_count += 1
     return include_count
 
-def function_lines(f):
+def get_function_lines(f):
     """
     Return iterator which iterates over functions and returns (function name, function lines)
     """
 
-    # XXX Buggy! Doesn't work with MOCK_IMPL and ENABLE_GCC_WARNINGS
+    # Skip lines with these terms since they confuse our regexp
+    REGEXP_CONFUSE_TERMS = ["MOCK_IMPL", "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", "DUMMY_TYPECHECK_INSTANCE",
+                            "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"]
+
     in_function = False
     for lineno, line in enumerate(f):
+        if any(x in line for x in REGEXP_CONFUSE_TERMS):
+            continue
+
         if not in_function:
             # find the start of a function
             m = re.match(r'^([a-zA-Z_][a-zA-Z_0-9]*),?\(', line)
@@ -31,6 +41,7 @@ def function_lines(f):
                 func_name = m.group(1)
                 func_start = lineno
                 in_function = True
+
         else:
             # Fund the end of a function
             if line.startswith("}"):
diff --git a/scripts/maint/practracker/practracker_tests.py b/scripts/maint/practracker/practracker_tests.py
index f3d225207a..cdbab2908e 100755
--- a/scripts/maint/practracker/practracker_tests.py
+++ b/scripts/maint/practracker/practracker_tests.py
@@ -40,6 +40,10 @@ class TestFunctionLength(unittest.TestCase):
         # All functions should have length 2
         for name, lines in metrics.function_lines(funcs):
             self.assertEqual(name, "fun")
+
+        funcs.seek(0)
+
+        for name, lines in metrics.function_lines(funcs):
             self.assertEqual(lines, 2)
 
 if __name__ == '__main__':

From 31c1d91ffb85671a9e3dc499655c65622d844333 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 27 Feb 2019 19:30:39 +0200
Subject: [PATCH 06/17] Exit with 1 if new issues were found. Also work with
 python3.

---
 scripts/maint/practracker/practracker.py | 69 +++++++++++++++---------
 scripts/maint/practracker/problem.py     | 27 ++++++----
 2 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index 4fbca615c5..3c22abd444 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 
 """
 Tor code best-practices tracker
@@ -47,69 +47,88 @@ ProblemVault = None
 #######################################################
 
 def consider_file_size(fname, f):
-    file_size = metrics.file_len(f)
+    """Consider file size issues for 'f' and return True if a new issue was found"""
+    file_size = metrics.get_file_len(f)
     if file_size > MAX_FILE_SIZE:
-        v = problem.FileSizeProblem(fname, file_size)
-        ProblemVault.register_problem(v)
+        p = problem.FileSizeProblem(fname, file_size)
+        return ProblemVault.register_problem(p)
+    return False
 
 def consider_includes(fname, f):
+    """Consider #include issues for 'f' and return True if a new issue was found"""
     include_count = metrics.get_include_count(f)
 
     if include_count > MAX_INCLUDE_COUNT:
-        v = problem.IncludeCountProblem(fname, include_count)
-        ProblemVault.register_problem(v)
+        p = problem.IncludeCountProblem(fname, include_count)
+        return ProblemVault.register_problem(p)
+    return False
 
 def consider_function_size(fname, f):
-    for name, lines in metrics.function_lines(f):
+    """Consider the function sizes for 'f' and return True if a new issue was found"""
+    found_new_issues = False
+
+    for name, lines in metrics.get_function_lines(f):
         # Don't worry about functions within our limits
         if lines <= MAX_FUNCTION_SIZE:
             continue
 
         # That's a big function! Issue a problem!
-        canonical_function_name = "%s:%s()" % (fname,name)
-        v = problem.FunctionSizeProblem(canonical_function_name, lines)
-        ProblemVault.register_problem(v)
+        canonical_function_name = "%s:%s()" % (fname, name)
+        p = problem.FunctionSizeProblem(canonical_function_name, lines)
+        found_new_issues |= ProblemVault.register_problem(p)
+
+    return found_new_issues
 
 #######################################################
 
 def consider_all_metrics(files_list):
-    """Consider metrics for all files"""
+    """Consider metrics for all files, and return True if new issues were found"""
+    found_new_issues = False
     for fname in files_list:
         with open(fname, 'r') as f:
-            consider_metrics_for_file(fname, f)
+            found_new_issues |= consider_metrics_for_file(fname, f)
+    return found_new_issues
 
 def consider_metrics_for_file(fname, f):
     """
-    Get metrics for file with filename 'fname' and file descriptor 'f'.
+    Consider the various metrics for file with filename 'fname' and file descriptor 'f'.
+    Return True if we found new issues.
     """
+    # Strip the useless part of the path
+    if fname.startswith(TOR_TOPDIR):
+        fname = fname[len(TOR_TOPDIR):]
+
+    found_new_issues = False
+
     # Get file length
-    consider_file_size(fname, f)
+    found_new_issues |= consider_file_size(fname, f)
 
     # Consider number of #includes
     f.seek(0)
-    consider_includes(fname, f)
+    found_new_issues |= consider_includes(fname, f)
 
     # Get function length
     f.seek(0)
-    consider_function_size(fname, f)
+    found_new_issues |= consider_function_size(fname, f)
+
+    return found_new_issues
 
 def main():
-    global ProblemVault
-
     # 1) Get all the .c files we care about
     files_list = util.get_tor_c_files(TOR_TOPDIR, EXCLUDE_SOURCE_DIRS)
 
     # 2) Initialize problem vault and load an optional exceptions file so that
     # we don't warn about the past
-    try:
-        with open(EXCEPTIONS_FILE, 'r') as exception_f:
-            ProblemVault = problem.ProblemVault(exception_f)
-    except IOError:
-        print "No exception file provided"
-        ProblemVault = problem.ProblemVault(None)
+    global ProblemVault
+    ProblemVault = problem.ProblemVault(EXCEPTIONS_FILE)
 
     # 3) Go through all the files and report problems if they are not exceptions
-    consider_all_metrics(files_list)
+    found_new_issues = consider_all_metrics(files_list)
+
+    if found_new_issues:
+        sys.exit(1)
+    else:
+        sys.exit(0)
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index fe6b30915b..e23d3bbf0e 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -13,12 +13,15 @@ class ProblemVault(object):
     found in the code, and also the old problems we read from the exception
     file.
     """
-    def __init__(self, exception_file):
+    def __init__(self, exception_fname):
         # Exception dictionary: { problem.key() : Problem object }
         self.exceptions = {}
 
-        if exception_file:
-            self.register_exceptions(exception_file)
+        try:
+            with open(exception_fname, 'r') as exception_f:
+                self.register_exceptions(exception_f)
+        except IOError:
+            print("No exception file provided")
 
     def register_exceptions(self, exception_file):
         # Register exceptions
@@ -27,25 +30,27 @@ class ProblemVault(object):
             if problem is None:
                 continue
 
-            # XXX this might overwrite problems with the same key (e.g. MOCK_IMPL)
             self.exceptions[problem.key()] = problem
             #print "Registering exception: %s" % problem
 
     def register_problem(self, problem):
+        """
+        Register this problem to the problem value. Return True if it was a new
+        problem or it worsens an already existing problem.
+        """
         # This is a new problem, print it
         if problem.key() not in self.exceptions:
-            print problem
-            return
+            print(problem)
+            return True
 
         # If it's an old problem, we don't warn if the situation got better
         # (e.g. we went from 4k LoC to 3k LoC), but we do warn if the
         # situation worsened (e.g. we went from 60 includes to 80).
         if problem.is_worse_than(self.exceptions[problem.key()]):
-            print problem
-            return
-#        else:
-#            print "OK %s better than %s" % (problem, self.exceptions[problem.key()])
+            print(problem)
+            return True
 
+        return False
 
 class Problem(object):
     def __init__(self, problem_type, problem_location, metric_value):
@@ -91,6 +96,6 @@ def get_old_problem_from_exception_str(exception_str):
     elif problem_type == "function-size":
         return FunctionSizeProblem(problem_location, metric_value)
     else:
-        print "Unknown exception line %s" % exception_str
+        print("Unknown exception line {}".format(exception_str))
         return None
 

From 58de565988160edec1fc5f0fbc2a794a765bce6f Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Thu, 28 Feb 2019 12:09:10 +0200
Subject: [PATCH 07/17] Call practracker as part of check-local.

- Introduce 'make check-best-practices'.
- Fix up Tor topdir etc to work with the way 'make check-local' gets called.
- Make practracker less likely to print useless stuff.
---
 Makefile.am                              | 11 ++++-
 scripts/maint/practracker/practracker.py | 54 ++++++++++++++----------
 scripts/maint/practracker/problem.py     |  2 +-
 scripts/maint/practracker/util.py        |  8 +++-
 4 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b40b2e51bc..415c2f2b4e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -161,7 +161,9 @@ EXTRA_DIST+= \
 	README						\
 	ReleaseNotes					\
 	scripts/maint/checkIncludes.py                  \
-	scripts/maint/checkSpace.pl
+	scripts/maint/checkSpace.pl \
+	scripts/maint/practracker
+
 
 ## This tells etags how to find mockable function definitions.
 AM_ETAGSFLAGS=--regex='{c}/MOCK_IMPL([^,]+,\W*\([a-zA-Z0-9_]+\)\W*,/\1/s'
@@ -224,7 +226,7 @@ shellcheck:
                 fi; \
 	fi
 
-check-local: check-spaces check-changes check-includes shellcheck
+check-local: check-spaces check-changes check-includes check-best-practices shellcheck
 
 need-chutney-path:
 	@if test ! -d "$$CHUTNEY_PATH"; then \
@@ -345,6 +347,11 @@ if USEPYTHON
 	$(PYTHON) $(top_srcdir)/scripts/maint/checkIncludes.py
 endif
 
+check-best-practices:
+if USEPYTHON
+	$(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir)
+endif
+
 check-docs: all
 	$(PERL) $(top_builddir)/scripts/maint/checkOptionDocs.pl
 
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index 3c22abd444..541c5cbfb3 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -1,21 +1,21 @@
 #!/usr/bin/python3
 
 """
-Tor code best-practices tracker
+Best-practices tracker for Tor source code.
 
 Go through the various .c files and collect metrics about them. If the metrics
 violate some of our best practices and they are not found in the optional
-exceptions file ("./exceptions.txt"), then log a problem about them.
-
-The exceptions file is meant to be initialized with the current state of the
-source code as follows: ./practracker.py > ./exceptions.txt
+exceptions file, then log a problem about them.
 
 We currently do metrics about file size, function size and number of includes.
 
-TODO:
-    - How is this tool supposed to be used? How should the exception file work?
-      How should the UI work? Does it need special exit codes?
-    - Fix the function_length function so that practracker_tests.py passes.
+practracker.py should be run with its second argument pointing to the Tor
+top-level source directory like this:
+  $ python3 ./scripts/maint/practracker/practracker.py .
+
+The exceptions file is meant to be initialized once with the current state of
+the source code and then get saved in the repository for ever after:
+  $ python3 ./scripts/maint/practracker/practracker.py . > ./scripts/maint/practracker/exceptions.txt
 """
 
 import os, sys
@@ -24,14 +24,8 @@ import metrics
 import util
 import problem
 
-# We don't want to run metrics for unittests, automatically-generated C files,
-# external libraries or git leftovers.
-EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"]
-
-# Where the Tor source code is
-TOR_TOPDIR = "../../../"
-# An optional exceptions_file
-EXCEPTIONS_FILE = "./exceptions.txt"
+# The filename of the exceptions file (it should be placed in the practracker directory)
+EXCEPTIONS_FNAME = "./exceptions.txt"
 
 # Recommended file size
 MAX_FILE_SIZE = 3000 # lines
@@ -42,8 +36,12 @@ MAX_INCLUDE_COUNT = 50
 
 #######################################################
 
+# ProblemVault singleton
 ProblemVault = None
 
+# The Tor source code topdir
+TOR_TOPDIR = None
+
 #######################################################
 
 def consider_file_size(fname, f):
@@ -114,21 +112,31 @@ def consider_metrics_for_file(fname, f):
     return found_new_issues
 
 def main():
+    if (len(sys.argv) != 2):
+        print("Usage:\n\t$ practracker.py <tor topdir>\n\t(e.g. $ practracker.py ~/tor/)")
+        return
+
+    global TOR_TOPDIR
+    TOR_TOPDIR = sys.argv[1]
+    exceptions_file = os.path.join(TOR_TOPDIR, "scripts/maint/practracker", EXCEPTIONS_FNAME)
+
     # 1) Get all the .c files we care about
-    files_list = util.get_tor_c_files(TOR_TOPDIR, EXCLUDE_SOURCE_DIRS)
+    files_list = util.get_tor_c_files(TOR_TOPDIR)
 
     # 2) Initialize problem vault and load an optional exceptions file so that
     # we don't warn about the past
     global ProblemVault
-    ProblemVault = problem.ProblemVault(EXCEPTIONS_FILE)
+    ProblemVault = problem.ProblemVault(exceptions_file)
 
     # 3) Go through all the files and report problems if they are not exceptions
     found_new_issues = consider_all_metrics(files_list)
 
-    if found_new_issues:
-        sys.exit(1)
-    else:
-        sys.exit(0)
+    # If new issues were found, try to give out some advice to the developer on how to resolve it.
+    if (found_new_issues):
+        new_issues_str = "practracker FAILED as indicated by the problem lines above. Please use the exceptions file ({}) to find any previous state of these problems. If you are unable to fix the underlying best-practices issue right now then you need to either update the relevant exception line or add a new one.".format(exceptions_file)
+        print(new_issues_str)
+
+    sys.exit(found_new_issues)
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index e23d3bbf0e..9f5771678e 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -96,6 +96,6 @@ def get_old_problem_from_exception_str(exception_str):
     elif problem_type == "function-size":
         return FunctionSizeProblem(problem_location, metric_value)
     else:
-        print("Unknown exception line {}".format(exception_str))
+#        print("Unknown exception line '{}'".format(exception_str))
         return None
 
diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py
index 8e9d95ece4..905cb8a5e0 100644
--- a/scripts/maint/practracker/util.py
+++ b/scripts/maint/practracker/util.py
@@ -1,6 +1,10 @@
 import os
 
-def get_tor_c_files(tor_topdir, exclude_dirs):
+# We don't want to run metrics for unittests, automatically-generated C files,
+# external libraries or git leftovers.
+EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"]
+
+def get_tor_c_files(tor_topdir):
     """
     Return a list with the .c filenames we want to get metrics of.
     """
@@ -14,7 +18,7 @@ def get_tor_c_files(tor_topdir, exclude_dirs):
 
             # Exclude the excluded paths
             full_path = os.path.join(root,filename)
-            if any(exclude_dir in full_path for exclude_dir in exclude_dirs):
+            if any(exclude_dir in full_path for exclude_dir in EXCLUDE_SOURCE_DIRS):
                 continue
 
             files_list.append(full_path)

From 4795f2d3a0df72d0e51a51eb1fbafdc68bfbc77c Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Thu, 28 Feb 2019 12:18:30 +0200
Subject: [PATCH 08/17] Fold in an initial practracker exceptions file.

---
 scripts/maint/practracker/exceptions.txt | 260 +++++++++++++++++++++++
 1 file changed, 260 insertions(+)
 create mode 100644 scripts/maint/practracker/exceptions.txt

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
new file mode 100644
index 0000000000..a42db81a06
--- /dev/null
+++ b/scripts/maint/practracker/exceptions.txt
@@ -0,0 +1,260 @@
+problem function-size /src/core/proto/proto_socks.c:parse_socks_client() 112
+problem file-size /src/core/or/channel.c 3476
+problem function-size /src/core/or/scheduler_kist.c:kist_scheduler_run() 171
+problem function-size /src/core/or/scheduler_vanilla.c:vanilla_scheduler_run() 109
+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 function-size /src/core/or/protover.c:protover_all_supported() 116
+problem function-size /src/core/or/circuitmux.c:circuitmux_detach_all_circuits() 109
+problem function-size /src/core/or/circuitmux.c:circuitmux_set_policy() 110
+problem function-size /src/core/or/circuitmux.c:circuitmux_attach_circuit() 129
+problem include-count /src/core/or/circuitlist.c 54
+problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 128
+problem function-size /src/core/or/circuitlist.c:circuit_free_() 137
+problem function-size /src/core/or/circuitlist.c:circuit_find_to_cannibalize() 102
+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 file-size /src/core/or/connection_or.c 3111
+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() 144
+problem function-size /src/core/or/connection_or.c:connection_or_compute_authenticate_cell_body() 235
+problem file-size /src/core/or/policies.c 3163
+problem function-size /src/core/or/policies.c:policy_summarize() 107
+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 file-size /src/core/or/circuituse.c 3146
+problem function-size /src/core/or/circuituse.c:circuit_is_acceptable() 129
+problem function-size /src/core/or/circuituse.c:circuit_expire_building() 394
+problem function-size /src/core/or/circuituse.c:circuit_log_ancient_one_hop_circuits() 126
+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() 110
+problem function-size /src/core/or/circuituse.c:circuit_get_open_circ_or_launch() 354
+problem function-size /src/core/or/circuituse.c:connection_ap_handshake_attach_circuit() 244
+problem file-size /src/core/or/connection_edge.c 4550
+problem include-count /src/core/or/connection_edge.c 64
+problem function-size /src/core/or/connection_edge.c:connection_ap_expire_beginning() 117
+problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_rewrite() 192
+problem function-size /src/core/or/connection_edge.c:connection_ap_handle_onion() 188
+problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_rewrite_and_attach() 423
+problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_send_begin() 111
+problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_socks_resolved() 106
+problem function-size /src/core/or/connection_edge.c:connection_exit_begin_conn() 184
+problem function-size /src/core/or/connection_edge.c:connection_exit_connect() 102
+problem function-size /src/core/or/circuitstats.c:circuit_build_times_parse_state() 124
+problem function-size /src/core/or/versions.c:tor_version_parse() 104
+problem file-size /src/core/or/circuitbuild.c 3060
+problem include-count /src/core/or/circuitbuild.c 53
+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 file-size /src/core/or/relay.c 3183
+problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 123
+problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 101
+problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 194
+problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 139
+problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell() 520
+problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 130
+problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 147
+problem file-size /src/core/mainloop/mainloop.c 3050
+problem include-count /src/core/mainloop/mainloop.c 65
+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 function-size /src/core/mainloop/mainloop.c:CALLBACK() 116
+problem file-size /src/core/mainloop/connection.c 5547
+problem include-count /src/core/mainloop/connection.c 60
+problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 184
+problem function-size /src/core/mainloop/connection.c:connection_listener_new() 328
+problem function-size /src/core/mainloop/connection.c:connection_handle_listener_read() 161
+problem function-size /src/core/mainloop/connection.c:connection_connect_sockaddr() 103
+problem function-size /src/core/mainloop/connection.c:connection_proxy_connect() 148
+problem function-size /src/core/mainloop/connection.c:connection_read_proxy_handshake() 153
+problem function-size /src/core/mainloop/connection.c:retry_listener_ports() 116
+problem function-size /src/core/mainloop/connection.c:connection_handle_read_impl() 111
+problem function-size /src/core/mainloop/connection.c:connection_buf_read_from_socket() 177
+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 function-size /src/app/config/confparse.c:config_assign_value() 205
+problem function-size /src/app/config/confparse.c:config_get_assigned_option() 129
+problem file-size /src/app/config/config.c 8488
+problem include-count /src/app/config/config.c 84
+problem function-size /src/app/config/config.c:options_act_reversible() 296
+problem function-size /src/app/config/config.c:options_act() 588
+problem function-size /src/app/config/config.c:resolve_my_address() 192
+problem function-size /src/app/config/config.c:options_validate() 1207
+problem function-size /src/app/config/config.c:options_init_from_torrc() 202
+problem function-size /src/app/config/config.c:options_init_from_string() 173
+problem function-size /src/app/config/config.c:options_init_logs() 146
+problem function-size /src/app/config/config.c:parse_bridge_line() 104
+problem function-size /src/app/config/config.c:parse_transport_line() 191
+problem function-size /src/app/config/config.c:parse_dir_authority_line() 151
+problem function-size /src/app/config/config.c:parse_dir_fallback_line() 102
+problem function-size /src/app/config/config.c:parse_port_config() 452
+problem function-size /src/app/config/config.c:parse_ports() 170
+problem function-size /src/app/config/config.c:getinfo_helper_config() 116
+problem function-size /src/app/main/ntmain.c:nt_service_install() 125
+problem include-count /src/app/main/main.c 83
+problem function-size /src/app/main/main.c:dumpstats() 102
+problem function-size /src/app/main/main.c:tor_init() 136
+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/tools/tor-resolve.c:build_socks5_resolve_request() 104
+problem function-size /src/tools/tor-resolve.c:do_resolve() 173
+problem function-size /src/tools/tor-resolve.c:main() 112
+problem function-size /src/tools/tor-gencert.c:parse_commandline() 111
+problem function-size /src/feature/keymgt/loadkey.c:ed_key_init_from_file() 333
+problem function-size /src/feature/dircommon/consdiff.c:gen_ed_diff() 204
+problem function-size /src/feature/dircommon/consdiff.c:apply_ed_diff() 159
+problem file-size /src/feature/control/control.c 7592
+problem include-count /src/feature/control/control.c 83
+problem function-size /src/feature/control/control.c:handle_control_authenticate() 188
+problem function-size /src/feature/control/control.c:getinfo_helper_misc() 109
+problem function-size /src/feature/control/control.c:getinfo_helper_dir() 304
+problem function-size /src/feature/control/control.c:getinfo_helper_events() 236
+problem function-size /src/feature/control/control.c:handle_control_extendcircuit() 151
+problem function-size /src/feature/control/control.c:handle_control_authchallenge() 115
+problem function-size /src/feature/control/control.c:handle_control_hsfetch() 114
+problem function-size /src/feature/control/control.c:handle_control_hspost() 117
+problem function-size /src/feature/control/control.c:handle_control_add_onion() 293
+problem function-size /src/feature/control/control.c:add_onion_helper_keyarg() 125
+problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 239
+problem function-size /src/feature/control/control.c:control_event_stream_status() 119
+problem function-size /src/feature/stats/rephist.c:rep_hist_load_mtbf_data() 185
+problem function-size /src/feature/stats/rephist.c:rep_hist_format_exit_stats() 148
+problem function-size /src/feature/dircache/consdiffmgr.c:consdiffmgr_cleanup() 115
+problem function-size /src/feature/dircache/consdiffmgr.c:consdiffmgr_rescan_flavor_() 111
+problem function-size /src/feature/dircache/consdiffmgr.c:consensus_diff_worker_threadfn() 132
+problem function-size /src/feature/dircache/dircache.c:handle_get_current_consensus() 166
+problem function-size /src/feature/dircache/dircache.c:directory_handle_command_post() 120
+problem function-size /src/feature/hibernate/hibernate.c:accounting_parse_options() 109
+problem function-size /src/feature/relay/routerkeys.c:load_ed_keys() 294
+problem file-size /src/feature/relay/router.c 3221
+problem include-count /src/feature/relay/router.c 56
+problem function-size /src/feature/relay/router.c:init_keys() 252
+problem function-size /src/feature/relay/router.c:get_my_declared_family() 114
+problem function-size /src/feature/relay/router.c:router_build_fresh_descriptor() 190
+problem function-size /src/feature/relay/router.c:router_dump_router_to_string() 375
+problem function-size /src/feature/relay/router.c:extrainfo_dump_to_string() 208
+problem function-size /src/feature/relay/dns.c:dns_resolve_impl() 134
+problem function-size /src/feature/relay/dns.c:configure_nameservers() 161
+problem function-size /src/feature/relay/dns.c:evdns_callback() 109
+problem function-size /src/feature/dirparse/authcert_parse.c:authority_cert_parse_from_string() 182
+problem function-size /src/feature/dirparse/ns_parse.c:routerstatus_parse_entry_from_string() 286
+problem function-size /src/feature/dirparse/ns_parse.c:networkstatus_verify_bw_weights() 389
+problem function-size /src/feature/dirparse/ns_parse.c:networkstatus_parse_vote_from_string() 638
+problem function-size /src/feature/dirparse/parsecommon.c:tokenize_string() 103
+problem function-size /src/feature/dirparse/parsecommon.c:get_next_token() 159
+problem function-size /src/feature/dirparse/routerparse.c:router_parse_entry_from_string() 554
+problem function-size /src/feature/dirparse/routerparse.c:extrainfo_parse_entry_from_string() 210
+problem function-size /src/feature/dirparse/microdesc_parse.c:microdescs_parse_from_string() 154
+problem file-size /src/feature/dirauth/dirvote.c 4729
+problem include-count /src/feature/dirauth/dirvote.c 53
+problem function-size /src/feature/dirauth/dirvote.c:format_networkstatus_vote() 251
+problem function-size /src/feature/dirauth/dirvote.c:networkstatus_compute_bw_weights_v10() 235
+problem function-size /src/feature/dirauth/dirvote.c:networkstatus_compute_consensus() 962
+problem function-size /src/feature/dirauth/dirvote.c:networkstatus_add_detached_signatures() 123
+problem function-size /src/feature/dirauth/dirvote.c:dirvote_add_vote() 162
+problem function-size /src/feature/dirauth/dirvote.c:dirvote_compute_consensuses() 164
+problem function-size /src/feature/dirauth/dirvote.c:dirserv_generate_networkstatus_vote_obj() 293
+problem function-size /src/feature/dirauth/guardfraction.c:dirserv_read_guardfraction_file_from_str() 110
+problem function-size /src/feature/dirauth/shared_random.c:should_keep_commit() 110
+problem function-size /src/feature/dirauth/process_descs.c:dirserv_add_descriptor() 125
+problem function-size /src/feature/dirauth/bwauth.c:dirserv_read_measured_bandwidths() 127
+problem function-size /src/feature/dirauth/dsigs_parse.c:networkstatus_parse_detached_signatures() 196
+problem function-size /src/feature/dirauth/voteflags.c:dirserv_compute_performance_thresholds() 172
+problem file-size /src/feature/hs/hs_service.c 4171
+problem function-size /src/feature/hs/hs_common.c:hs_get_responsible_hsdirs() 104
+problem function-size /src/feature/hs/hs_client.c:send_introduce1() 104
+problem function-size /src/feature/hs/hs_client.c:hs_config_client_authorization() 108
+problem function-size /src/feature/hs/hs_config.c:config_generic_service() 149
+problem function-size /src/feature/hs/hs_cell.c:hs_cell_build_establish_intro() 115
+problem function-size /src/feature/hs/hs_cell.c:hs_cell_parse_introduce2() 147
+problem file-size /src/feature/hs/hs_descriptor.c 3108
+problem function-size /src/feature/hs/hs_descriptor.c:desc_encode_v3() 108
+problem function-size /src/feature/hs/hs_descriptor.c:decrypt_desc_layer() 110
+problem function-size /src/feature/hs/hs_descriptor.c:decode_introduction_point() 122
+problem function-size /src/feature/hs/hs_descriptor.c:desc_decode_superencrypted_v3() 109
+problem function-size /src/feature/hs/hs_descriptor.c:desc_decode_encrypted_v3() 109
+problem file-size /src/feature/dirclient/dirclient.c 3215
+problem include-count /src/feature/dirclient/dirclient.c 51
+problem function-size /src/feature/dirclient/dirclient.c:directory_get_from_dirserver() 131
+problem function-size /src/feature/dirclient/dirclient.c:directory_initiate_request() 201
+problem function-size /src/feature/dirclient/dirclient.c:directory_send_command() 241
+problem function-size /src/feature/dirclient/dirclient.c:dir_client_decompress_response_body() 114
+problem function-size /src/feature/dirclient/dirclient.c:connection_dir_client_reached_eof() 189
+problem function-size /src/feature/dirclient/dirclient.c:handle_response_fetch_consensus() 105
+problem function-size /src/feature/rend/rendmid.c:rend_mid_establish_intro_legacy() 104
+problem function-size /src/feature/rend/rendparse.c:rend_parse_v2_service_descriptor() 187
+problem function-size /src/feature/rend/rendparse.c:rend_decrypt_introduction_points() 104
+problem function-size /src/feature/rend/rendparse.c:rend_parse_introduction_points() 131
+problem file-size /src/feature/rend/rendservice.c 4487
+problem function-size /src/feature/rend/rendservice.c:rend_service_prune_list_impl_() 107
+problem function-size /src/feature/rend/rendservice.c:rend_config_service() 164
+problem function-size /src/feature/rend/rendservice.c:rend_service_load_auth_keys() 178
+problem function-size /src/feature/rend/rendservice.c:rend_service_receive_introduction() 332
+problem function-size /src/feature/rend/rendservice.c:rend_service_parse_intro_for_v3() 115
+problem function-size /src/feature/rend/rendservice.c:rend_service_decrypt_intro() 115
+problem function-size /src/feature/rend/rendservice.c:rend_service_intro_has_opened() 126
+problem function-size /src/feature/rend/rendservice.c:rend_service_rendezvous_has_opened() 117
+problem function-size /src/feature/rend/rendservice.c:directory_post_to_hs_dir() 108
+problem function-size /src/feature/rend/rendservice.c:upload_service_descriptor() 111
+problem function-size /src/feature/rend/rendservice.c:rend_consider_services_intro_points() 169
+problem function-size /src/feature/rend/rendcache.c:rend_cache_store_v2_desc_as_client() 193
+problem function-size /src/feature/rend/rendclient.c:rend_client_send_introduction() 220
+problem function-size /src/feature/rend/rendcommon.c:rend_encode_v2_descriptors() 225
+problem function-size /src/feature/nodelist/authcert.c:trusted_dirs_load_certs_from_string() 124
+problem function-size /src/feature/nodelist/authcert.c:authority_certs_fetch_missing() 296
+problem function-size /src/feature/nodelist/microdesc.c:microdesc_cache_rebuild() 134
+problem function-size /src/feature/nodelist/fmt_routerstatus.c:routerstatus_format_entry() 166
+problem include-count /src/feature/nodelist/networkstatus.c 61
+problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_check_consensus_signature() 176
+problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_set_current_consensus() 293
+problem file-size /src/feature/nodelist/routerlist.c 3234
+problem function-size /src/feature/nodelist/routerlist.c:router_rebuild_store() 148
+problem function-size /src/feature/nodelist/routerlist.c:router_add_to_routerlist() 169
+problem function-size /src/feature/nodelist/routerlist.c:routerlist_remove_old_routers() 121
+problem function-size /src/feature/nodelist/routerlist.c:update_consensus_router_descriptor_downloads() 136
+problem function-size /src/feature/nodelist/routerlist.c:update_extrainfo_downloads() 103
+problem function-size /src/feature/nodelist/nodelist.c:compute_frac_paths_available() 187
+problem function-size /src/feature/nodelist/node_select.c:router_pick_directory_server_impl() 123
+problem function-size /src/feature/nodelist/node_select.c:compute_weighted_bandwidths() 205
+problem function-size /src/feature/nodelist/node_select.c:router_pick_trusteddirserver_impl() 114
+problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153
+problem function-size /src/feature/client/transports.c:handle_proxy_line() 108
+problem function-size /src/feature/client/transports.c:parse_method_line_helper() 112
+problem function-size /src/feature/client/transports.c:create_managed_proxy_environment() 109
+problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 126
+problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 112
+problem file-size /src/feature/client/entrynodes.c 3817
+problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 153
+problem function-size /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 246
+problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108
+problem function-size /src/lib/log/log.c:parse_log_severity_config() 101
+problem function-size /src/lib/math/prob_distr.c:sample_uniform_interval() 145
+problem function-size /src/lib/process/process_win32.c:process_win32_exec() 138
+problem function-size /src/lib/process/process_win32.c:process_win32_create_pipe() 112
+problem function-size /src/lib/process/setuid.c:switch_id() 156
+problem function-size /src/lib/process/restrict.c:set_max_file_descriptors() 102
+problem function-size /src/lib/process/process_unix.c:process_unix_exec() 220
+problem function-size /src/lib/container/smartlist.c:smartlist_bsearch_idx() 109
+problem function-size /src/lib/sandbox/sandbox.c:prot_strings() 104
+problem function-size /src/lib/compress/compress_zstd.c:tor_zstd_compress_process() 126
+problem function-size /src/lib/compress/compress.c:tor_compress_impl() 133
+problem function-size /src/lib/crypt_ops/crypto_rand.c:crypto_strongest_rand_syscall() 102
+problem function-size /src/lib/string/scanf.c:tor_vsscanf() 112
+problem function-size /src/lib/encoding/confline.c:parse_config_line_from_str_verbose() 119
+problem function-size /src/lib/encoding/binascii.c:base64_encode() 107
+problem function-size /src/lib/encoding/cstring.c:unescape_string() 108
+problem function-size /src/lib/tls/tortls_openssl.c:tor_tls_context_new() 171
+problem function-size /src/lib/tls/x509_nss.c:tor_tls_create_certificate_internal() 126
+problem function-size /src/lib/tls/tortls_nss.c:tor_tls_context_new() 147
+problem function-size /src/lib/osinfo/uname.c:get_uname() 116
+problem function-size /src/lib/net/address.c:tor_addr_parse_mask_ports() 198
+problem function-size /src/lib/net/address.c:tor_addr_compare_masked() 111
+problem function-size /src/lib/net/resolve.c:tor_addr_lookup() 110
+problem function-size /src/lib/net/inaddr.c:tor_inet_pton() 107
+problem function-size /src/lib/net/socketpair.c:tor_ersatz_socketpair() 102
+problem function-size /src/lib/fs/dir.c:check_private_dir() 231

From 8bacc1dad170a9a29633d95c91a4139af6a2bfe7 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Tue, 5 Mar 2019 15:01:07 +0200
Subject: [PATCH 09/17] practracker: Improve documentation in problem.py .

---
 scripts/maint/practracker/problem.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index 9f5771678e..2330098ef3 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -53,6 +53,10 @@ class ProblemVault(object):
         return False
 
 class Problem(object):
+    """
+    A generic problem in our source code. See the subclasses below for the
+    specific problems we are trying to tackle.
+    """
     def __init__(self, problem_type, problem_location, metric_value):
         self.problem_location = problem_location
         self.metric_value = int(metric_value)
@@ -72,14 +76,35 @@ class Problem(object):
         return "problem %s %s %s" % (self.problem_type, self.problem_location, self.metric_value)
 
 class FileSizeProblem(Problem):
+    """
+    Denotes a problem with the size of a .c file.
+
+    The 'problem_location' is the filesystem path of the .c file, and the
+    'metric_value' is the number of lines in the .c file.
+    """
     def __init__(self, problem_location, metric_value):
         super(FileSizeProblem, self).__init__("file-size", problem_location, metric_value)
 
 class IncludeCountProblem(Problem):
+    """
+    Denotes a problem with the number of #includes in a .c file.
+
+    The 'problem_location' is the filesystem path of the .c file, and the
+    'metric_value' is the number of #includes in the .c file.
+    """
     def __init__(self, problem_location, metric_value):
         super(IncludeCountProblem, self).__init__("include-count", problem_location, metric_value)
 
 class FunctionSizeProblem(Problem):
+    """
+    Denotes a problem with a size of a function in a .c file.
+
+    The 'problem_location' is "<path>:<function>()" where <path> is the
+    filesystem path of the .c file and <function> is the name of the offending
+    function.
+
+    The 'metric_value' is the size of the offending function in lines.
+    """
     def __init__(self, problem_location, metric_value):
         super(FunctionSizeProblem, self).__init__("function-size", problem_location, metric_value)
 

From 8c9835c6e5de6f17affb9b7d9f5802dee54a9e37 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Tue, 5 Mar 2019 15:03:27 +0200
Subject: [PATCH 10/17] practracker: Normalize filesystem paths across Windows
 and Posix.

This was causing issues because the exceptions file is written using Posix
paths, whereas practracker in Windows was trying to match Windows paths ("\"
instead of "/").
---
 scripts/maint/practracker/problem.py | 7 ++++++-
 scripts/maint/practracker/util.py    | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index 2330098ef3..00e029d1c4 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -7,6 +7,8 @@ problem is worse than a registered exception so that it only warns when things
 get worse.
 """
 
+import os.path
+
 class ProblemVault(object):
     """
     Singleton where we store the various new problems we
@@ -70,7 +72,10 @@ class Problem(object):
 
     def key(self):
         """Generate a unique key that describes this problem that can be used as a dictionary key"""
-        return "%s:%s" % (self.problem_location, self.problem_type)
+        # Problem location is a filesystem path, so we need to normalize this
+        # across platforms otherwise same paths are not gonna match.
+        canonical_location = os.path.normcase(self.problem_location)
+        return "%s:%s" % (canonical_location, self.problem_type)
 
     def __str__(self):
         return "problem %s %s %s" % (self.problem_type, self.problem_location, self.metric_value)
diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py
index 905cb8a5e0..966c625158 100644
--- a/scripts/maint/practracker/util.py
+++ b/scripts/maint/practracker/util.py
@@ -18,7 +18,7 @@ def get_tor_c_files(tor_topdir):
 
             # Exclude the excluded paths
             full_path = os.path.join(root,filename)
-            if any(exclude_dir in full_path for exclude_dir in EXCLUDE_SOURCE_DIRS):
+            if any(os.path.normcase(exclude_dir) in full_path for exclude_dir in EXCLUDE_SOURCE_DIRS):
                 continue
 
             files_list.append(full_path)

From 157f7ba93ed8800dd34eafd4184a893b37965829 Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Tue, 5 Mar 2019 15:17:54 +0200
Subject: [PATCH 11/17] practracker: Update exceptions file :)

---
 scripts/maint/practracker/exceptions.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index a42db81a06..da354d8f83 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -58,7 +58,7 @@ problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open()
 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 139
 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell() 520
 problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 130
-problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 147
+problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 148
 problem file-size /src/core/mainloop/mainloop.c 3050
 problem include-count /src/core/mainloop/mainloop.c 65
 problem function-size /src/core/mainloop/mainloop.c:conn_close_if_marked() 108
@@ -190,7 +190,7 @@ problem function-size /src/feature/rend/rendmid.c:rend_mid_establish_intro_legac
 problem function-size /src/feature/rend/rendparse.c:rend_parse_v2_service_descriptor() 187
 problem function-size /src/feature/rend/rendparse.c:rend_decrypt_introduction_points() 104
 problem function-size /src/feature/rend/rendparse.c:rend_parse_introduction_points() 131
-problem file-size /src/feature/rend/rendservice.c 4487
+problem file-size /src/feature/rend/rendservice.c 4509
 problem function-size /src/feature/rend/rendservice.c:rend_service_prune_list_impl_() 107
 problem function-size /src/feature/rend/rendservice.c:rend_config_service() 164
 problem function-size /src/feature/rend/rendservice.c:rend_service_load_auth_keys() 178

From ec8c5b3fea78b2596d3b68571fd47b8e41d4a3ea Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Tue, 12 Mar 2019 14:35:26 +0200
Subject: [PATCH 12/17] practracker: Improvements based on last Nick's review.

---
 scripts/maint/practracker/metrics.py | 6 +++---
 scripts/maint/practracker/problem.py | 3 ++-
 scripts/maint/practracker/util.py    | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py
index c7d2091d05..56b9e8383e 100644
--- a/scripts/maint/practracker/metrics.py
+++ b/scripts/maint/practracker/metrics.py
@@ -26,8 +26,8 @@ def get_function_lines(f):
     """
 
     # Skip lines with these terms since they confuse our regexp
-    REGEXP_CONFUSE_TERMS = ["MOCK_IMPL", "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", "DUMMY_TYPECHECK_INSTANCE",
-                            "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"]
+    REGEXP_CONFUSE_TERMS = {"MOCK_IMPL", "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", "DUMMY_TYPECHECK_INSTANCE",
+                            "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"}
 
     in_function = False
     for lineno, line in enumerate(f):
@@ -43,7 +43,7 @@ def get_function_lines(f):
                 in_function = True
 
         else:
-            # Fund the end of a function
+            # Find the end of a function
             if line.startswith("}"):
                 n_lines = lineno - func_start
                 in_function = False
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index 00e029d1c4..60ee3eb242 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -8,6 +8,7 @@ get worse.
 """
 
 import os.path
+import sys
 
 class ProblemVault(object):
     """
@@ -23,7 +24,7 @@ class ProblemVault(object):
             with open(exception_fname, 'r') as exception_f:
                 self.register_exceptions(exception_f)
         except IOError:
-            print("No exception file provided")
+            print("No exception file provided", file=sys.stderr)
 
     def register_exceptions(self, exception_file):
         # Register exceptions
diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py
index 966c625158..63de72d5a3 100644
--- a/scripts/maint/practracker/util.py
+++ b/scripts/maint/practracker/util.py
@@ -2,7 +2,7 @@ import os
 
 # We don't want to run metrics for unittests, automatically-generated C files,
 # external libraries or git leftovers.
-EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"]
+EXCLUDE_SOURCE_DIRS = {"/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"}
 
 def get_tor_c_files(tor_topdir):
     """

From f0302d51abf2f4c6d7660b82ad5038d1cbf9e5d7 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 12 Mar 2019 09:22:53 -0400
Subject: [PATCH 13/17] practracker: Be more careful about excluding "confusing
 terms"

Previously we excluded any line containing one of these terms from
consideration as the start or end of a function.  Now we're more
careful, and we only ignore these terms when they appear to be
starting a function definition.
---
 scripts/maint/practracker/metrics.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py
index 56b9e8383e..5fa305a868 100644
--- a/scripts/maint/practracker/metrics.py
+++ b/scripts/maint/practracker/metrics.py
@@ -25,20 +25,20 @@ def get_function_lines(f):
     Return iterator which iterates over functions and returns (function name, function lines)
     """
 
-    # Skip lines with these terms since they confuse our regexp
+    # Skip lines that look like they are defining functions with these
+    # names: they aren't real function definitions.
     REGEXP_CONFUSE_TERMS = {"MOCK_IMPL", "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", "DUMMY_TYPECHECK_INSTANCE",
                             "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"}
 
     in_function = False
     for lineno, line in enumerate(f):
-        if any(x in line for x in REGEXP_CONFUSE_TERMS):
-            continue
-
         if not in_function:
             # find the start of a function
             m = re.match(r'^([a-zA-Z_][a-zA-Z_0-9]*),?\(', line)
             if m:
                 func_name = m.group(1)
+                if func_name in REGEXP_CONFUSE_TERMS:
+                    continue
                 func_start = lineno
                 in_function = True
 

From e2512950b69d5f9f030673a15e8cf72b930538c7 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 12 Mar 2019 09:32:22 -0400
Subject: [PATCH 14/17] Improve failure message from check-best-practices

---
 doc/HACKING/HelpfulTools.md              | 15 +++++++++++++++
 scripts/maint/practracker/practracker.py |  9 ++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md
index d499238526..cba57e875d 100644
--- a/doc/HACKING/HelpfulTools.md
+++ b/doc/HACKING/HelpfulTools.md
@@ -371,3 +371,18 @@ source code. Here's how to use it:
 
   6. See the Doxygen manual for more information; this summary just
      scratches the surface.
+
+Style and best-pratices checking
+--------------------------------
+
+We use scripts to check for various problems in the formatting and style
+of our source code.  The "check-spaces" test detects a bunch of violations
+of our coding style on the local level.  The "check-best-practices" test
+looks for violations of some of our complexity guidelines.
+
+You can tell the tool about exceptions to the complexity guidelines via its
+exceptions file (scripts/maint/practracker/exceptions.txt).  But before you
+do this, consider whether you shouldn't fix the underlying problem.  Maybe
+that file really _is_ too big.  Maybe that function really _is_ doing too
+much.  (On the other hand, for stable release series, it is sometimes better
+to leave things unrefactored.)
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index 541c5cbfb3..08b74c264f 100755
--- a/scripts/maint/practracker/practracker.py
+++ b/scripts/maint/practracker/practracker.py
@@ -133,7 +133,14 @@ def main():
 
     # If new issues were found, try to give out some advice to the developer on how to resolve it.
     if (found_new_issues):
-        new_issues_str = "practracker FAILED as indicated by the problem lines above. Please use the exceptions file ({}) to find any previous state of these problems. If you are unable to fix the underlying best-practices issue right now then you need to either update the relevant exception line or add a new one.".format(exceptions_file)
+        new_issues_str = """\
+FAILURE: practracker found new problems in the code: see warnings above.
+
+Please fix the problems if you can, and update the exceptions file
+({}) if you can't.
+
+See doc/HACKING/HelpfulTools.md for more information on using practracker.\
+""".format(exceptions_file)
         print(new_issues_str)
 
     sys.exit(found_new_issues)

From 21c6e295ca74e2ab50a759123bca1fae36af36b3 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 12 Mar 2019 20:37:25 -0400
Subject: [PATCH 15/17] grandfather in two more functions

---
 scripts/maint/practracker/exceptions.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index da354d8f83..3fa68aff47 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -166,7 +166,9 @@ problem function-size /src/feature/dirauth/bwauth.c:dirserv_read_measured_bandwi
 problem function-size /src/feature/dirauth/dsigs_parse.c:networkstatus_parse_detached_signatures() 196
 problem function-size /src/feature/dirauth/voteflags.c:dirserv_compute_performance_thresholds() 172
 problem file-size /src/feature/hs/hs_service.c 4171
+problem function-size /src/feature/hs/hs_cell.c:hs_cell_parse_introduce2() 154
 problem function-size /src/feature/hs/hs_common.c:hs_get_responsible_hsdirs() 104
+problem function-size /src/feature/hs/hs_common.c:hs_get_extend_info_from_lspecs() 101
 problem function-size /src/feature/hs/hs_client.c:send_introduce1() 104
 problem function-size /src/feature/hs/hs_client.c:hs_config_client_authorization() 108
 problem function-size /src/feature/hs/hs_config.c:config_generic_service() 149

From 3e38efdf165098062972300687975c3372cec70e Mon Sep 17 00:00:00 2001
From: George Kadianakis <desnacked@riseup.net>
Date: Wed, 13 Mar 2019 12:59:44 +0200
Subject: [PATCH 16/17] practracker: Fix duplicate exceptions and handle them
 more "gracefully".

---
 scripts/maint/practracker/exceptions.txt | 1 -
 scripts/maint/practracker/problem.py     | 6 ++++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index 3fa68aff47..abe615f04a 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -173,7 +173,6 @@ problem function-size /src/feature/hs/hs_client.c:send_introduce1() 104
 problem function-size /src/feature/hs/hs_client.c:hs_config_client_authorization() 108
 problem function-size /src/feature/hs/hs_config.c:config_generic_service() 149
 problem function-size /src/feature/hs/hs_cell.c:hs_cell_build_establish_intro() 115
-problem function-size /src/feature/hs/hs_cell.c:hs_cell_parse_introduce2() 147
 problem file-size /src/feature/hs/hs_descriptor.c 3108
 problem function-size /src/feature/hs/hs_descriptor.c:desc_encode_v3() 108
 problem function-size /src/feature/hs/hs_descriptor.c:decrypt_desc_layer() 110
diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py
index 60ee3eb242..61420fb785 100644
--- a/scripts/maint/practracker/problem.py
+++ b/scripts/maint/practracker/problem.py
@@ -33,6 +33,12 @@ class ProblemVault(object):
             if problem is None:
                 continue
 
+            # Fail if we see dup exceptions. There is really no reason to have dup exceptions.
+            if problem.key() in self.exceptions:
+                print("Duplicate exceptions lines found in exception file:\n\t{}\n\t{}\nAborting...".format(problem, self.exceptions[problem.key()]),
+                      file=sys.stderr)
+                sys.exit(1)
+
             self.exceptions[problem.key()] = problem
             #print "Registering exception: %s" % problem
 

From 504aef6228d3a8460f43b9c9cf2cd4175ce4e1f7 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 13 Mar 2019 09:29:10 -0400
Subject: [PATCH 17/17] Changes file for practracker (29221)

---
 changes/bug29221 | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 changes/bug29221

diff --git a/changes/bug29221 b/changes/bug29221
new file mode 100644
index 0000000000..fbe08aa9a0
--- /dev/null
+++ b/changes/bug29221
@@ -0,0 +1,5 @@
+  o Minor features (development tools):
+    - Tor's test scripts now check for files and functions that seem
+      too long and complicated.  Existing overlong functions and files are
+      accepted for now, but should eventually be refactored. Closes
+      ticket 29221.