Browse Source

Merge branch 'bug29221_more_squashed'

Nick Mathewson 5 years ago
parent
commit
17ff69a268

+ 9 - 2
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
 

+ 5 - 0
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.

+ 15 - 0
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.)

+ 261 - 0
scripts/maint/practracker/exceptions.txt

@@ -0,0 +1,261 @@
+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() 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
+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_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
+problem function-size /src/feature/hs/hs_cell.c:hs_cell_build_establish_intro() 115
+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 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
+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

+ 50 - 0
scripts/maint/practracker/metrics.py

@@ -0,0 +1,50 @@
+#!/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 get_file_len(f):
+    """Get file length of file"""
+    for i, l in enumerate(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 get_function_lines(f):
+    """
+    Return iterator which iterates over functions and returns (function name, function lines)
+    """
+
+    # 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 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
+
+        else:
+            # Find the end of a function
+            if line.startswith("}"):
+                n_lines = lineno - func_start
+                in_function = False
+                yield (func_name, n_lines)

+ 149 - 0
scripts/maint/practracker/practracker.py

@@ -0,0 +1,149 @@
+#!/usr/bin/python3
+
+"""
+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, then log a problem about them.
+
+We currently do metrics about file size, function size and number of includes.
+
+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
+
+import metrics
+import util
+import problem
+
+# 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
+# Recommended function size
+MAX_FUNCTION_SIZE = 100 # lines
+# Recommended number of #includes
+MAX_INCLUDE_COUNT = 50
+
+#######################################################
+
+# ProblemVault singleton
+ProblemVault = None
+
+# The Tor source code topdir
+TOR_TOPDIR = None
+
+#######################################################
+
+def consider_file_size(fname, 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:
+        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:
+        p = problem.IncludeCountProblem(fname, include_count)
+        return ProblemVault.register_problem(p)
+    return False
+
+def consider_function_size(fname, 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)
+        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, and return True if new issues were found"""
+    found_new_issues = False
+    for fname in files_list:
+        with open(fname, 'r') as f:
+            found_new_issues |= consider_metrics_for_file(fname, f)
+    return found_new_issues
+
+def consider_metrics_for_file(fname, 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
+    found_new_issues |= consider_file_size(fname, f)
+
+    # Consider number of #includes
+    f.seek(0)
+    found_new_issues |= consider_includes(fname, f)
+
+    # Get function length
+    f.seek(0)
+    found_new_issues |= consider_function_size(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)
+
+    # 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)
+
+    # 3) Go through all the files and report problems if they are not exceptions
+    found_new_issues = consider_all_metrics(files_list)
+
+    # 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 = """\
+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)
+
+if __name__ == '__main__':
+    main()

+ 50 - 0
scripts/maint/practracker/practracker_tests.py

@@ -0,0 +1,50 @@
+"""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")
+
+        funcs.seek(0)
+
+        for name, lines in metrics.function_lines(funcs):
+            self.assertEqual(lines, 2)
+
+if __name__ == '__main__':
+    unittest.main()

+ 138 - 0
scripts/maint/practracker/problem.py

@@ -0,0 +1,138 @@
+"""
+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.
+"""
+
+import os.path
+import sys
+
+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_fname):
+        # Exception dictionary: { problem.key() : Problem object }
+        self.exceptions = {}
+
+        try:
+            with open(exception_fname, 'r') as exception_f:
+                self.register_exceptions(exception_f)
+        except IOError:
+            print("No exception file provided", file=sys.stderr)
+
+    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
+
+            # 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
+
+    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 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 True
+
+        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)
+        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"""
+        # 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)
+
+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)
+
+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 '{}'".format(exception_str))
+        return None
+

+ 27 - 0
scripts/maint/practracker/util.py

@@ -0,0 +1,27 @@
+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/"}
+
+def get_tor_c_files(tor_topdir):
+    """
+    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(os.path.normcase(exclude_dir) in full_path for exclude_dir in EXCLUDE_SOURCE_DIRS):
+                continue
+
+            files_list.append(full_path)
+
+    return files_list
+