diff --git a/changes/ticket31175 b/changes/ticket31175 new file mode 100644 index 0000000000..cff13761a4 --- /dev/null +++ b/changes/ticket31175 @@ -0,0 +1,3 @@ + o Minor features (development tools): + - Our best-practices tracker now looks at headers as well as + C files. Closes ticket 31175. diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 75a8b50967..8e4618a72a 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -45,6 +45,7 @@ problem function-size /src/app/config/config.c:parse_dir_fallback_line() 101 problem function-size /src/app/config/config.c:parse_port_config() 446 problem function-size /src/app/config/config.c:parse_ports() 168 problem function-size /src/app/config/config.c:getinfo_helper_config() 113 +problem file-size /src/app/config/or_options_st.h 1112 problem include-count /src/app/main/main.c 68 problem function-size /src/app/main/main.c:dumpstats() 102 problem function-size /src/app/main/main.c:tor_init() 137 @@ -67,6 +68,7 @@ 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 file-size /src/core/or/channel.c 3487 +problem file-size /src/core/or/channel.h 780 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 @@ -86,6 +88,7 @@ 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 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 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 function-size /src/core/or/circuitstats.c:circuit_build_times_parse_state() 123 @@ -114,6 +117,8 @@ 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 file-size /src/core/or/policies.c 3249 problem function-size /src/core/or/policies.c:policy_summarize() 107 problem function-size /src/core/or/protover.c:protover_all_supported() 117 @@ -136,6 +141,7 @@ problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153 problem file-size /src/feature/client/entrynodes.c 3824 problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 155 problem function-size /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 246 +problem file-size /src/feature/client/entrynodes.h 639 problem function-size /src/feature/client/transports.c:handle_proxy_line() 108 problem function-size /src/feature/client/transports.c:parse_method_line_helper() 110 problem function-size /src/feature/client/transports.c:create_managed_proxy_environment() 109 diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py index 82f1cd64e9..9f69b2ac1f 100644 --- a/scripts/maint/practracker/metrics.py +++ b/scripts/maint/practracker/metrics.py @@ -27,7 +27,9 @@ def get_function_lines(f): # 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", + REGEXP_CONFUSE_TERMS = {"MOCK_IMPL", "MOCK_DECL", "HANDLE_DECL", + "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", + "DUMMY_TYPECHECK_INSTANCE", "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"} in_function = False diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 7e51edb48f..0fdfd4a40a 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -35,6 +35,10 @@ MAX_FILE_SIZE = 3000 # lines MAX_FUNCTION_SIZE = 100 # lines # Recommended number of #includes MAX_INCLUDE_COUNT = 50 +# Recommended file size for headers +MAX_H_FILE_SIZE = 500 +# Recommended include count for headers +MAX_H_INCLUDE_COUNT = 15 # Map from problem type to functions that adjust for tolerance TOLERANCE_FNS = { @@ -161,8 +165,12 @@ def main(argv): help="Make all warnings into errors") parser.add_argument("--terse", action="store_true", help="Do not emit helpful instructions.") + parser.add_argument("--max-h-file-size", default=MAX_H_FILE_SIZE, + help="Maximum lines per .H file") + parser.add_argument("--max-h-include-count", default=MAX_H_INCLUDE_COUNT, + help="Maximum includes per .H file") parser.add_argument("--max-file-size", default=MAX_FILE_SIZE, - help="Maximum lines per C file size") + help="Maximum lines per C file") parser.add_argument("--max-include-count", default=MAX_INCLUDE_COUNT, help="Maximum includes per C file") parser.add_argument("--max-function-size", default=MAX_FUNCTION_SIZE, @@ -180,9 +188,11 @@ def main(argv): # 0) Configure our thresholds of "what is a problem actually" filt = problem.ProblemFilter() - filt.addThreshold(problem.FileSizeItem("*", int(args.max_file_size))) - filt.addThreshold(problem.IncludeCountItem("*", int(args.max_include_count))) - filt.addThreshold(problem.FunctionSizeItem("*", int(args.max_function_size))) + filt.addThreshold(problem.FileSizeItem("*.c", int(args.max_file_size))) + filt.addThreshold(problem.IncludeCountItem("*.c", int(args.max_include_count))) + filt.addThreshold(problem.FileSizeItem("*.h", int(args.max_h_file_size))) + filt.addThreshold(problem.IncludeCountItem("*.h", int(args.max_h_include_count))) + filt.addThreshold(problem.FunctionSizeItem("*.c", int(args.max_function_size))) # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR) diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 73519d446f..13c8e55143 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -108,10 +108,11 @@ class ProblemFilter(object): self.thresholds = dict() def addThreshold(self, item): - self.thresholds[item.get_type()] = item + self.thresholds[(item.get_type(),item.get_file_type())] = item def matches(self, item): - filt = self.thresholds.get(item.get_type(), None) + key = (item.get_type(), item.get_file_type()) + filt = self.thresholds.get(key, None) if filt is None: return False return item.is_worse_than(filt) @@ -158,6 +159,12 @@ class Item(object): def get_type(self): return self.problem_type + def get_file_type(self): + if self.problem_location.endswith(".h"): + return "*.h" + else: + return "*.c" + class FileSizeItem(Item): """ Denotes a problem with the size of a .c file. diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 5a8876a0f6..695668f561 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -5,12 +5,14 @@ import os EXCLUDE_SOURCE_DIRS = {"src/test/", "src/trunnel/", "src/rust/", "src/ext/", ".git/"} +EXCLUDE_FILES = {"orconfig.h"} + def _norm(p): return os.path.normcase(os.path.normpath(p)) def get_tor_c_files(tor_topdir): """ - Return a list with the .c filenames we want to get metrics of. + Return a list with the .c and .h filenames we want to get metrics of. """ files_list = [] exclude_dirs = { _norm(os.path.join(tor_topdir, p)) for p in EXCLUDE_SOURCE_DIRS } @@ -23,8 +25,10 @@ def get_tor_c_files(tor_topdir): directories.sort() filenames.sort() for filename in filenames: - # We only care about .c files - if not filename.endswith(".c"): + # We only care about .c and .h files + if not (filename.endswith(".c") or filename.endswith(".h")): + continue + if filename in EXCLUDE_FILES: continue full_path = os.path.join(root,filename)