diff options
Diffstat (limited to 'scripts/maint/practracker')
22 files changed, 1805 insertions, 0 deletions
diff --git a/scripts/maint/practracker/README b/scripts/maint/practracker/README new file mode 100644 index 0000000000..d978b39806 --- /dev/null +++ b/scripts/maint/practracker/README @@ -0,0 +1,21 @@ +Practracker is a simple python tool that keeps track of places where +our code is ugly, and tries to warn us about new ones or ones that +get worse. + +Right now, practracker looks for the following kinds of +best-practices violations: + + .c files greater than 3000 lines long + .h files greater than 500 lines long + .c files with more than 50 includes + .h files with more than 15 includes + + All files that include a local header not listed in a .may_include + file in the same directory, when that .may_include file has an + "!advisory" marker. + +The list of current violations is tracked in exceptions.txt; slight +deviations of the current exceptions cause warnings, whereas large +ones cause practracker to fail. + +For usage information, run "practracker.py --help". diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt new file mode 100644 index 0000000000..711ef67d36 --- /dev/null +++ b/scripts/maint/practracker/exceptions.txt @@ -0,0 +1,325 @@ +# Welcome to the exceptions file for Tor's best-practices tracker! +# +# Each line of this file represents a single violation of Tor's best +# practices -- typically, a violation that we had before practracker.py +# first existed. +# +# There are three kinds of problems that we recognize right now: +# function-size -- a function of more than 100 lines. +# file-size -- a .c file of more than 3000 lines, or a .h +# file with more than 500 lines. +# include-count -- a .c file with more than 50 #includes, +# or a .h file with more than 15 #includes. +# dependency-violation -- a file includes a header that it should +# not, according to an advisory .may_include file. +# +# Each line below represents a single exception that practracker should +# _ignore_. Each line has four parts: +# 1. The word "problem". +# 2. The kind of problem. +# 3. The location of the problem: either a filename, or a +# filename:functionname pair. +# 4. The magnitude of the problem to ignore. +# +# So for example, consider this line: +# problem file-size /src/core/or/connection_or.c 3200 +# +# It tells practracker to allow the mentioned file to be up to 3200 lines +# long, even though ordinarily it would warn about any file with more than +# 3000 lines. +# +# You can either edit this file by hand, or regenerate it completely by +# running `make practracker-regen`. +# +# Remember: It is better to fix the problem than to add a new exception! + +problem file-size /src/app/config/config.c 7525 +problem include-count /src/app/config/config.c 80 +problem function-size /src/app/config/config.c:options_act() 381 +problem function-size /src/app/config/config.c:options_validate_cb() 794 +problem function-size /src/app/config/config.c:options_init_from_torrc() 192 +problem function-size /src/app/config/config.c:options_init_from_string() 103 +problem function-size /src/app/config/config.c:options_init_logs() 125 +problem function-size /src/app/config/config.c:parse_bridge_line() 104 +problem function-size /src/app/config/config.c:pt_parse_transport_line() 190 +problem function-size /src/app/config/config.c:parse_dir_authority_line() 150 +problem function-size /src/app/config/config.c:parse_dir_fallback_line() 101 +problem function-size /src/app/config/config.c:port_parse_config() 435 +problem function-size /src/app/config/config.c:parse_ports() 132 +problem function-size /src/app/config/resolve_addr.c:resolve_my_address() 191 +problem file-size /src/app/config/or_options_st.h 1050 +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() 101 +problem function-size /src/app/main/main.c:sandbox_init_filter() 291 +problem function-size /src/app/main/main.c:run_tor_main_loop() 105 +problem function-size /src/app/main/ntmain.c:nt_service_install() 126 +problem dependency-violation /src/core/crypto/hs_ntor.c 1 +problem dependency-violation /src/core/crypto/hs_ntor.h 1 +problem dependency-violation /src/core/crypto/onion_crypto.c 5 +problem dependency-violation /src/core/crypto/onion_fast.c 1 +problem dependency-violation /src/core/crypto/onion_tap.c 3 +problem dependency-violation /src/core/crypto/relay_crypto.c 9 +problem file-size /src/core/mainloop/connection.c 5700 +problem include-count /src/core/mainloop/connection.c 65 +problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 181 +problem function-size /src/core/mainloop/connection.c:connection_listener_new() 325 +problem function-size /src/core/mainloop/connection.c:connection_handle_listener_read() 161 +problem function-size /src/core/mainloop/connection.c:connection_read_proxy_handshake() 153 +problem function-size /src/core/mainloop/connection.c:retry_listener_ports() 112 +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() 186 +problem function-size /src/core/mainloop/connection.c:connection_handle_write_impl() 241 +problem function-size /src/core/mainloop/connection.c:assert_connection_ok() 143 +problem dependency-violation /src/core/mainloop/connection.c 47 +problem dependency-violation /src/core/mainloop/cpuworker.c 12 +problem include-count /src/core/mainloop/mainloop.c 64 +problem function-size /src/core/mainloop/mainloop.c:conn_close_if_marked() 107 +problem function-size /src/core/mainloop/mainloop.c:run_connection_housekeeping() 123 +problem dependency-violation /src/core/mainloop/mainloop.c 50 +problem dependency-violation /src/core/mainloop/mainloop_pubsub.c 1 +problem dependency-violation /src/core/mainloop/mainloop_sys.c 1 +problem dependency-violation /src/core/mainloop/netstatus.c 4 +problem dependency-violation /src/core/mainloop/periodic.c 2 +problem dependency-violation /src/core/or/address_set.c 1 +problem dependency-violation /src/core/or/cell_queue_st.h 1 +problem file-size /src/core/or/channel.c 3500 +problem dependency-violation /src/core/or/channel.c 9 +problem file-size /src/core/or/channel.h 800 +problem dependency-violation /src/core/or/channel.h 1 +problem dependency-violation /src/core/or/channelpadding.c 6 +problem function-size /src/core/or/channeltls.c:channel_tls_handle_var_cell() 160 +problem function-size /src/core/or/channeltls.c:channel_tls_process_versions_cell() 170 +problem function-size /src/core/or/channeltls.c:channel_tls_process_netinfo_cell() 214 +problem function-size /src/core/or/channeltls.c:channel_tls_process_certs_cell() 246 +problem function-size /src/core/or/channeltls.c:channel_tls_process_authenticate_cell() 202 +problem dependency-violation /src/core/or/channeltls.c 11 +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:choose_good_exit_server_general() 206 +problem dependency-violation /src/core/or/circuitbuild.c 25 +problem include-count /src/core/or/circuitlist.c 55 +problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 109 +problem function-size /src/core/or/circuitlist.c:circuit_free_() 146 +problem function-size /src/core/or/circuitlist.c:circuit_find_to_cannibalize() 101 +problem function-size /src/core/or/circuitlist.c:circuits_handle_oom() 117 +problem dependency-violation /src/core/or/circuitlist.c 19 +problem dependency-violation /src/core/or/circuitlist.h 1 +problem function-size /src/core/or/circuitmux.c:circuitmux_set_policy() 109 +problem function-size /src/core/or/circuitmux.c:circuitmux_attach_circuit() 113 +problem dependency-violation /src/core/or/circuitmux_ewma.c 2 +problem file-size /src/core/or/circuitpadding.c 3101 +problem function-size /src/core/or/circuitpadding.c:circpad_machine_schedule_padding() 113 +problem dependency-violation /src/core/or/circuitpadding.c 6 +problem file-size /src/core/or/circuitpadding.h 813 +problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_relay_hide_intro_circuits() 103 +problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_client_hide_rend_circuits() 112 +problem dependency-violation /src/core/or/circuitpadding_machines.c 1 +problem function-size /src/core/or/circuitstats.c:circuit_build_times_parse_state() 123 +problem dependency-violation /src/core/or/circuitstats.c 11 +problem file-size /src/core/or/circuituse.c 3195 +problem function-size /src/core/or/circuituse.c:circuit_is_acceptable() 128 +problem function-size /src/core/or/circuituse.c:circuit_expire_building() 389 +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() 108 +problem function-size /src/core/or/circuituse.c:circuit_get_open_circ_or_launch() 351 +problem function-size /src/core/or/circuituse.c:connection_ap_handshake_attach_circuit() 244 +problem dependency-violation /src/core/or/circuituse.c 24 +problem function-size /src/core/or/command.c:command_process_create_cell() 156 +problem function-size /src/core/or/command.c:command_process_relay_cell() 132 +problem dependency-violation /src/core/or/command.c 9 +problem file-size /src/core/or/connection_edge.c 4655 +problem include-count /src/core/or/connection_edge.c 65 +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() 193 +problem function-size /src/core/or/connection_edge.c:connection_ap_handle_onion() 185 +problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_rewrite_and_attach() 420 +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() 101 +problem function-size /src/core/or/connection_edge.c:connection_exit_begin_conn() 185 +problem function-size /src/core/or/connection_edge.c:connection_exit_connect() 130 +problem dependency-violation /src/core/or/connection_edge.c 27 +problem dependency-violation /src/core/or/connection_edge.h 1 +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 dependency-violation /src/core/or/connection_or.c 21 +problem dependency-violation /src/core/or/dos.c 6 +problem dependency-violation /src/core/or/onion.c 2 +problem file-size /src/core/or/or.h 1105 +problem include-count /src/core/or/or.h 48 +problem dependency-violation /src/core/or/or.h 1 +problem dependency-violation /src/core/or/or_periodic.c 1 +problem file-size /src/core/or/policies.c 3182 +problem function-size /src/core/or/policies.c:policy_summarize() 107 +problem dependency-violation /src/core/or/policies.c 14 +problem function-size /src/core/or/protover.c:protover_all_supported() 117 +problem dependency-violation /src/core/or/reasons.c 2 +problem file-size /src/core/or/relay.c 3300 +problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 127 +problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 109 +problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 192 +problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 137 +problem function-size /src/core/or/relay.c:handle_relay_cell_command() 369 +problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 128 +problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 146 +problem dependency-violation /src/core/or/relay.c 17 +problem dependency-violation /src/core/or/scheduler.c 1 +problem function-size /src/core/or/scheduler_kist.c:kist_scheduler_run() 171 +problem dependency-violation /src/core/or/scheduler_kist.c 2 +problem function-size /src/core/or/scheduler_vanilla.c:vanilla_scheduler_run() 109 +problem dependency-violation /src/core/or/scheduler_vanilla.c 1 +problem dependency-violation /src/core/or/sendme.c 2 +problem dependency-violation /src/core/or/status.c 13 +problem function-size /src/core/or/versions.c:tor_version_parse() 104 +problem dependency-violation /src/core/proto/proto_cell.c 3 +problem dependency-violation /src/core/proto/proto_control0.c 1 +problem dependency-violation /src/core/proto/proto_ext_or.c 2 +problem dependency-violation /src/core/proto/proto_http.c 1 +problem function-size /src/core/proto/proto_socks.c:parse_socks_client() 110 +problem dependency-violation /src/core/proto/proto_socks.c 8 +problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 109 +problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 125 +problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108 +problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153 +problem file-size /src/feature/client/entrynodes.c 4000 +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 700 +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() 111 +problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113 +problem function-size /src/feature/control/control_auth.c:handle_control_authenticate() 186 +problem function-size /src/feature/control/control_cmd.c:handle_control_extendcircuit() 150 +problem function-size /src/feature/control/control_cmd.c:handle_control_add_onion() 256 +problem function-size /src/feature/control/control_cmd.c:add_onion_helper_keyarg() 118 +problem function-size /src/feature/control/control_events.c:control_event_stream_status() 124 +problem include-count /src/feature/control/control_getinfo.c 56 +problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_misc() 108 +problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_dir() 297 +problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_events() 234 +problem function-size /src/feature/dirauth/bwauth.c:dirserv_read_measured_bandwidths() 121 +problem file-size /src/feature/dirauth/dirvote.c 4734 +problem include-count /src/feature/dirauth/dirvote.c 55 +problem function-size /src/feature/dirauth/dirvote.c:format_networkstatus_vote() 230 +problem function-size /src/feature/dirauth/dirvote.c:networkstatus_compute_bw_weights_v10() 233 +problem function-size /src/feature/dirauth/dirvote.c:networkstatus_compute_consensus() 952 +problem function-size /src/feature/dirauth/dirvote.c:networkstatus_add_detached_signatures() 119 +problem function-size /src/feature/dirauth/dirvote.c:dirvote_add_vote() 161 +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() 281 +problem function-size /src/feature/dirauth/dsigs_parse.c:networkstatus_parse_detached_signatures() 196 +problem function-size /src/feature/dirauth/guardfraction.c:dirserv_read_guardfraction_file_from_str() 109 +problem function-size /src/feature/dirauth/process_descs.c:dirserv_add_descriptor() 125 +problem function-size /src/feature/dirauth/shared_random.c:should_keep_commit() 109 +problem function-size /src/feature/dirauth/voteflags.c:dirserv_compute_performance_thresholds() 175 +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() 165 +problem function-size /src/feature/dircache/dircache.c:directory_handle_command_post() 124 +problem file-size /src/feature/dirclient/dirclient.c 3204 +problem include-count /src/feature/dirclient/dirclient.c 54 +problem function-size /src/feature/dirclient/dirclient.c:directory_get_from_dirserver() 126 +problem function-size /src/feature/dirclient/dirclient.c:directory_initiate_request() 201 +problem function-size /src/feature/dirclient/dirclient.c:directory_send_command() 239 +problem function-size /src/feature/dirclient/dirclient.c:dir_client_decompress_response_body() 111 +problem function-size /src/feature/dirclient/dirclient.c:connection_dir_client_reached_eof() 199 +problem function-size /src/feature/dirclient/dirclient.c:handle_response_fetch_consensus() 104 +problem function-size /src/feature/dircommon/consdiff.c:gen_ed_diff() 203 +problem function-size /src/feature/dircommon/consdiff.c:apply_ed_diff() 158 +problem function-size /src/feature/dirparse/authcert_parse.c:authority_cert_parse_from_string() 181 +problem function-size /src/feature/dirparse/ns_parse.c:routerstatus_parse_entry_from_string() 280 +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() 635 +problem function-size /src/feature/dirparse/parsecommon.c:tokenize_string() 101 +problem function-size /src/feature/dirparse/parsecommon.c:get_next_token() 165 +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() 208 +problem function-size /src/feature/hibernate/hibernate.c:accounting_parse_options() 109 +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() 134 +problem function-size /src/feature/hs/hs_client.c:send_introduce1() 108 +problem function-size /src/feature/hs/hs_common.c:hs_get_responsible_hsdirs() 102 +problem function-size /src/feature/hs/hs_descriptor.c:decrypt_desc_layer() 111 +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() 107 +problem function-size /src/feature/hs/hs_descriptor.c:desc_decode_encrypted_v3() 109 +problem file-size /src/feature/hs/hs_service.c 4300 +problem function-size /src/feature/keymgt/loadkey.c:ed_key_init_from_file() 326 +problem function-size /src/feature/nodelist/authcert.c:trusted_dirs_load_certs_from_string() 123 +problem function-size /src/feature/nodelist/authcert.c:authority_certs_fetch_missing() 295 +problem function-size /src/feature/nodelist/fmt_routerstatus.c:routerstatus_format_entry() 158 +problem function-size /src/feature/nodelist/microdesc.c:microdesc_cache_rebuild() 134 +problem include-count /src/feature/nodelist/networkstatus.c 65 +problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_check_consensus_signature() 175 +problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_set_current_consensus() 289 +problem function-size /src/feature/nodelist/node_select.c:router_pick_directory_server_impl() 122 +problem function-size /src/feature/nodelist/node_select.c:compute_weighted_bandwidths() 204 +problem function-size /src/feature/nodelist/node_select.c:router_pick_trusteddirserver_impl() 112 +problem function-size /src/feature/nodelist/nodelist.c:compute_frac_paths_available() 190 +problem file-size /src/feature/nodelist/routerlist.c 3247 +problem function-size /src/feature/nodelist/routerlist.c:router_rebuild_store() 148 +problem function-size /src/feature/nodelist/routerlist.c:router_add_to_routerlist() 168 +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() 142 +problem function-size /src/feature/nodelist/routerlist.c:update_extrainfo_downloads() 103 +problem function-size /src/feature/relay/dns.c:dns_resolve_impl() 131 +problem function-size /src/feature/relay/dns.c:configure_nameservers() 161 +problem function-size /src/feature/relay/dns.c:evdns_callback() 108 +problem function-size /src/feature/relay/relay_handshake.c:connection_or_compute_authenticate_cell_body() 231 +problem file-size /src/feature/relay/router.c 3600 +problem include-count /src/feature/relay/router.c 57 +problem function-size /src/feature/relay/router.c:init_keys() 254 +problem function-size /src/feature/relay/router.c:get_my_declared_family() 114 +problem function-size /src/feature/relay/router.c:router_build_fresh_unsigned_routerinfo() 113 +problem function-size /src/feature/relay/router.c:router_dump_router_to_string() 372 +problem function-size /src/feature/relay/routerkeys.c:load_ed_keys() 294 +problem function-size /src/feature/rend/rendcache.c:rend_cache_store_v2_desc_as_client() 190 +problem function-size /src/feature/rend/rendclient.c:rend_client_send_introduction() 219 +problem function-size /src/feature/rend/rendcommon.c:rend_encode_v2_descriptors() 221 +problem function-size /src/feature/rend/rendmid.c:rend_mid_establish_intro_legacy() 105 +problem function-size /src/feature/rend/rendparse.c:rend_parse_v2_service_descriptor() 181 +problem function-size /src/feature/rend/rendparse.c:rend_parse_introduction_points() 129 +problem file-size /src/feature/rend/rendservice.c 4504 +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() 143 +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() 334 +problem function-size /src/feature/rend/rendservice.c:rend_service_parse_intro_for_v3() 111 +problem function-size /src/feature/rend/rendservice.c:rend_service_decrypt_intro() 112 +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() 106 +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() 170 +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/lib/compress/compress.c:tor_compress_impl() 127 +problem function-size /src/lib/compress/compress_zstd.c:tor_zstd_compress_process() 123 +problem function-size /src/lib/container/smartlist.c:smartlist_bsearch_idx() 107 +problem function-size /src/lib/crypt_ops/crypto_rand.c:crypto_strongest_rand_syscall() 102 +problem function-size /src/lib/encoding/binascii.c:base64_encode() 106 +problem function-size /src/lib/encoding/confline.c:parse_config_line_from_str_verbose() 117 +problem function-size /src/lib/encoding/cstring.c:unescape_string() 108 +problem function-size /src/lib/fs/dir.c:check_private_dir() 230 +problem function-size /src/lib/math/prob_distr.c:sample_uniform_interval() 145 +problem function-size /src/lib/net/address.c:tor_addr_parse_mask_ports() 195 +problem function-size /src/lib/net/address.c:tor_addr_compare_masked() 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/process/process_unix.c:process_unix_exec() 213 +problem function-size /src/lib/process/process_win32.c:process_win32_exec() 151 +problem function-size /src/lib/process/process_win32.c:process_win32_create_pipe() 109 +problem function-size /src/lib/process/restrict.c:set_max_file_descriptors() 102 +problem function-size /src/lib/process/setuid.c:switch_id() 156 +problem function-size /src/lib/sandbox/sandbox.c:prot_strings() 104 +problem function-size /src/lib/string/scanf.c:tor_vsscanf() 112 +problem function-size /src/lib/tls/tortls_nss.c:tor_tls_context_new() 152 +problem function-size /src/lib/tls/tortls_openssl.c:tor_tls_context_new() 170 +problem function-size /src/lib/tls/x509_nss.c:tor_tls_create_certificate_internal() 121 +problem function-size /src/tools/tor-gencert.c:parse_commandline() 111 +problem function-size /src/tools/tor-resolve.c:build_socks5_resolve_request() 102 +problem function-size /src/tools/tor-resolve.c:do_resolve() 171 +problem function-size /src/tools/tor-resolve.c:main() 112 diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py new file mode 100755 index 0000000000..a5ee728824 --- /dev/null +++ b/scripts/maint/practracker/includes.py @@ -0,0 +1,381 @@ +#!/usr/bin/env python +# Copyright 2018 The Tor Project, Inc. See LICENSE file for licensing info. + +"""This script looks through all the directories for files matching *.c or + *.h, and checks their #include directives to make sure that only "permitted" + headers are included. + + Any #include directives with angle brackets (like #include <stdio.h>) are + ignored -- only directives with quotes (like #include "foo.h") are + considered. + + To decide what includes are permitted, this script looks at a .may_include + file in each directory. This file contains empty lines, #-prefixed + comments, filenames (like "lib/foo/bar.h") and file globs (like lib/*/*.h) + for files that are permitted. + + The script exits with an error if any non-permitted includes are found. + .may_include files that contain "!advisory" are considered advisory. + Advisory .may_include files only result in warnings, rather than errors. +""" + +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +import fnmatch +import os +import re +import sys + +if sys.version_info[0] <= 2: + def open_file(fname): + return open(fname, 'r') +else: + def open_file(fname): + return open(fname, 'r', encoding='utf-8') + +def warn(msg): + print(msg, file=sys.stderr) + +def fname_is_c(fname): + """ Return true iff 'fname' is the name of a file that we should + search for possibly disallowed #include directives. """ + if fname.endswith(".h") or fname.endswith(".c"): + bname = os.path.basename(fname) + return not (bname.startswith(".") or bname.startswith("#")) + else: + return False + +INCLUDE_PATTERN = re.compile(r'\s*#\s*include\s+"([^"]*)"') +RULES_FNAME = ".may_include" + +ALLOWED_PATTERNS = [ + re.compile(r'^.*\*\.(h|inc)$'), + re.compile(r'^.*/.*\.h$'), + re.compile(r'^ext/.*\.c$'), + re.compile(r'^orconfig.h$'), + re.compile(r'^micro-revision.i$'), +] + +TOPDIR = "src" + +def pattern_is_normal(s): + for p in ALLOWED_PATTERNS: + if p.match(s): + return True + return False + +class Error(object): + def __init__(self, location, msg, is_advisory=False): + self.location = location + self.msg = msg + self.is_advisory = is_advisory + + def __str__(self): + return "{} at {}".format(self.msg, self.location) + +class Rules(object): + """ A 'Rules' object is the parsed version of a .may_include file. """ + def __init__(self, dirpath): + self.dirpath = dirpath + if dirpath.startswith("src/"): + self.incpath = dirpath[4:] + else: + self.incpath = dirpath + self.patterns = [] + self.usedPatterns = set() + self.is_advisory = False + + def addPattern(self, pattern): + if pattern == "!advisory": + self.is_advisory = True + return + if not pattern_is_normal(pattern): + warn("Unusual pattern {} in {}".format(pattern, self.dirpath)) + self.patterns.append(pattern) + + def includeOk(self, path): + for pattern in self.patterns: + if fnmatch.fnmatchcase(path, pattern): + self.usedPatterns.add(pattern) + return True + return False + + def applyToLines(self, lines, loc_prefix=""): + lineno = 0 + for line in lines: + lineno += 1 + m = INCLUDE_PATTERN.match(line) + if m: + include = m.group(1) + if not self.includeOk(include): + yield Error("{}{}".format(loc_prefix,str(lineno)), + "Forbidden include of {}".format(include), + is_advisory=self.is_advisory) + + def applyToFile(self, fname, f): + for error in self.applyToLines(iter(f), "{}:".format(fname)): + yield error + + def noteUnusedRules(self): + for p in self.patterns: + if p not in self.usedPatterns: + warn("Pattern {} in {} was never used.".format(p, self.dirpath)) + + def getAllowedDirectories(self): + allowed = [] + for p in self.patterns: + m = re.match(r'^(.*)/\*\.(h|inc)$', p) + if m: + allowed.append(m.group(1)) + continue + m = re.match(r'^(.*)/[^/]*$', p) + if m: + allowed.append(m.group(1)) + continue + + return allowed + + +def normalize_srcdir(fname): + """given the name of a source directory or file, return its name + relative to `src` in a unix-like format. + """ + orig = fname + dirname, dirfile = os.path.split(fname) + if re.match(r'.*\.[ch]$', dirfile): + fname = dirname + + # Now we have a directory. + dirname, result = os.path.split(fname) + for _ in range(100): + # prevent excess looping in case I missed a tricky case + dirname, dirpart = os.path.split(dirname) + if dirpart == 'src' or dirname == "": + #print(orig,"=>",result) + return result + result = "{}/{}".format(dirpart,result) + + print("No progress!") + assert False + +include_rules_cache = {} + +def load_include_rules(fname): + """ Read a rules file from 'fname', and return it as a Rules object. + Return 'None' if fname does not exist. + """ + if fname in include_rules_cache: + return include_rules_cache[fname] + if not os.path.exists(fname): + include_rules_cache[fname] = None + return None + result = Rules(os.path.split(fname)[0]) + with open_file(fname) as f: + for line in f: + line = line.strip() + if line.startswith("#") or not line: + continue + result.addPattern(line) + include_rules_cache[fname] = result + return result + +def get_all_include_rules(): + """Return a list of all the Rules objects we have loaded so far, + sorted by their directory names.""" + return [ rules for (fname,rules) in + sorted(include_rules_cache.items()) + if rules is not None ] + +def remove_self_edges(graph): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects). + + Remove all edges from a node to itself.""" + + for k in list(graph): + graph[k] = [ d for d in graph[k] if d != k ] + +def closure(graph): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects), and completes + its closure. + """ + graph = graph.copy() + changed = False + for k in graph.keys(): + graph[k] = set(graph[k]) + while True: + for k in graph.keys(): + sz = len(graph[k]) + for v in list(graph[k]): + graph[k].update(graph.get(v, [])) + if sz != len(graph[k]): + changed = True + + if not changed: + return graph + changed = False + +def toposort(graph, limit=100): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects). Tries to + perform a topological sort on the graph, arranging the nodes into + "levels", such that every member of each level is only reachable + by members of later levels. + + Returns a list of the members of each level. + + Modifies the input graph, removing every member that could be + sorted. If the graph does not become empty, then it contains a + cycle. + + "limit" is the max depth of the graph after which we give up trying + to sort it and conclude we have a cycle. + """ + all_levels = [] + + n = 0 + while graph: + n += 0 + cur_level = [] + all_levels.append(cur_level) + for k in list(graph): + graph[k] = [ d for d in graph[k] if d in graph ] + if graph[k] == []: + cur_level.append(k) + for k in cur_level: + del graph[k] + n += 1 + if n > limit: + break + + return all_levels + +def consider_include_rules(fname, f): + dirpath = os.path.split(fname)[0] + rules_fname = os.path.join(dirpath, RULES_FNAME) + rules = load_include_rules(os.path.join(dirpath, RULES_FNAME)) + if rules is None: + return + + for err in rules.applyToFile(fname, f): + yield err + + list_unused = False + log_sorted_levels = False + +def walk_c_files(topdir="src"): + """Run through all .c and .h files under topdir, looking for + include-rule violations. Yield those violations.""" + + for dirpath, dirnames, fnames in os.walk(topdir): + for fname in fnames: + if fname_is_c(fname): + fullpath = os.path.join(dirpath,fname) + with open(fullpath) as f: + for err in consider_include_rules(fullpath, f): + yield err + +def open_or_stdin(fname): + if fname == '-': + return sys.stdin + else: + return open(fname) + +def check_subsys_file(fname, uses_dirs): + if not uses_dirs: + # We're doing a distcheck build, or for some other reason there are + # no .may_include files. + print("SKIPPING") + return False + + uses_dirs = { normalize_srcdir(k) : { normalize_srcdir(d) for d in v } + for (k,v) in uses_dirs.items() } + uses_closure = closure(uses_dirs) + ok = True + previous_subsystems = [] + + with open_or_stdin(fname) as f: + for line in f: + _, name, fname = line.split() + fname = normalize_srcdir(fname) + for prev in previous_subsystems: + if fname in uses_closure[prev]: + print("INVERSION: {} uses {}".format(prev,fname)) + ok = False + previous_subsystems.append(fname) + return not ok + +def run_check_includes(topdir, list_unused=False, log_sorted_levels=False, + list_advisories=False, check_subsystem_order=None): + trouble = False + + for err in walk_c_files(topdir): + if err.is_advisory and not list_advisories: + continue + print(err, file=sys.stderr) + if not err.is_advisory: + trouble = True + + if trouble: + warn( + """To change which includes are allowed in a C file, edit the {} + files in its enclosing directory.""".format(RULES_FNAME)) + sys.exit(1) + + if list_unused: + for rules in get_all_include_rules(): + rules.noteUnusedRules() + + uses_dirs = { } + for rules in get_all_include_rules(): + uses_dirs[rules.incpath] = rules.getAllowedDirectories() + + remove_self_edges(uses_dirs) + + if check_subsystem_order: + if check_subsys_file(check_subsystem_order, uses_dirs): + sys.exit(1) + + all_levels = toposort(uses_dirs) + + if log_sorted_levels: + for (n, cur_level) in enumerate(all_levels): + if cur_level: + print(n, cur_level) + + if uses_dirs: + print("There are circular .may_include dependencies in here somewhere:", + uses_dirs) + sys.exit(1) + +def main(argv): + import argparse + + progname = argv[0] + parser = argparse.ArgumentParser(prog=progname) + parser.add_argument("--toposort", action="store_true", + help="Print a topologically sorted list of modules") + parser.add_argument("--list-unused", action="store_true", + help="List unused lines in .may_include files.") + parser.add_argument("--list-advisories", action="store_true", + help="List advisories as well as forbidden includes") + parser.add_argument("--check-subsystem-order", action="store", + help="Check a list of subsystems for ordering") + parser.add_argument("topdir", default="src", nargs="?", + help="Top-level directory for the tor source") + args = parser.parse_args(argv[1:]) + + global TOPDIR + TOPDIR = args.topdir + run_check_includes(topdir=args.topdir, + log_sorted_levels=args.toposort, + list_unused=args.list_unused, + list_advisories=args.list_advisories, + check_subsystem_order=args.check_subsystem_order) + +if __name__ == '__main__': + main(sys.argv) diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py new file mode 100644 index 0000000000..300a4501a9 --- /dev/null +++ b/scripts/maint/practracker/metrics.py @@ -0,0 +1,62 @@ +#!/usr/bin/env 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. + +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +import re + +def get_file_len(f): + """Get file length of file""" + i = -1 + 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'\s*#\s*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", "MOCK_DECL", "HANDLE_DECL", + "ENABLE_GCC_WARNINGS", "ENABLE_GCC_WARNING", + "DUMMY_TYPECHECK_INSTANCE", + "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"} + + in_function = False + found_openbrace = 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 + elif not found_openbrace and line.startswith("{"): + found_openbrace = True + func_start = lineno + else: + # Find the end of a function + if line.startswith("}"): + n_lines = lineno - func_start + 1 + in_function = False + found_openbrace = 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..76ffd64cfb --- /dev/null +++ b/scripts/maint/practracker/practracker.py @@ -0,0 +1,320 @@ +#!/usr/bin/env python + +""" +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, +for C source files and headers. + +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 . + +To regenerate the exceptions file so that it allows all current +problems in the Tor source, use the --regen flag: + $ python3 --regen ./scripts/maint/practracker/practracker.py . +""" + +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +import codecs, os, sys + +import metrics +import util +import problem +import includes +import shutil + +# 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 +# Recommended file size for headers +MAX_H_FILE_SIZE = 500 +# Recommended include count for headers +MAX_H_INCLUDE_COUNT = 15 +# Recommended number of dependency violations +MAX_DEP_VIOLATIONS = 0 + +# Map from problem type to functions that adjust for tolerance +TOLERANCE_FNS = { + 'include-count': lambda n: int(n*1.1), + 'function-size': lambda n: int(n*1.1), + 'file-size': lambda n: int(n*1.02), + 'dependency-violation': lambda n: (n+2) +} + +####################################################### + +# The Tor source code topdir +TOR_TOPDIR = None + +####################################################### + +def open_file(fname): + return codecs.open(fname, 'r', encoding='utf-8') + +def consider_file_size(fname, f): + """Consider the size of 'f' and yield an FileSizeItem for it. + """ + file_size = metrics.get_file_len(f) + yield problem.FileSizeItem(fname, file_size) + +def consider_includes(fname, f): + """Consider the #include count in for 'f' and yield an IncludeCountItem + for it. + """ + include_count = metrics.get_include_count(f) + + yield problem.IncludeCountItem(fname, include_count) + +def consider_function_size(fname, f): + """yield a FunctionSizeItem for every function in f. + """ + + for name, lines in metrics.get_function_lines(f): + canonical_function_name = "%s:%s()" % (fname, name) + yield problem.FunctionSizeItem(canonical_function_name, lines) + +def consider_include_violations(fname, real_fname, f): + n = 0 + for item in includes.consider_include_rules(real_fname, f): + n += 1 + if n: + yield problem.DependencyViolationItem(fname, n) + + +####################################################### + +def consider_all_metrics(files_list): + """Consider metrics for all files, and yield a sequence of problem.Item + object for those issues.""" + for fname in files_list: + with open_file(fname) as f: + for item in consider_metrics_for_file(fname, f): + yield item + +def consider_metrics_for_file(fname, f): + """ + Yield a sequence of problem.Item objects for all of the metrics in + 'f'. + """ + real_fname = fname + # Strip the useless part of the path + if fname.startswith(TOR_TOPDIR): + fname = fname[len(TOR_TOPDIR):] + + # Get file length + for item in consider_file_size(fname, f): + yield item + + # Consider number of #includes + f.seek(0) + for item in consider_includes(fname, f): + yield item + + # Get function length + f.seek(0) + for item in consider_function_size(fname, f): + yield item + + # Check for "upward" includes + f.seek(0) + for item in consider_include_violations(fname, real_fname, f): + yield item + +HEADER="""\ +# Welcome to the exceptions file for Tor's best-practices tracker! +# +# Each line of this file represents a single violation of Tor's best +# practices -- typically, a violation that we had before practracker.py +# first existed. +# +# There are three kinds of problems that we recognize right now: +# function-size -- a function of more than {MAX_FUNCTION_SIZE} lines. +# file-size -- a .c file of more than {MAX_FILE_SIZE} lines, or a .h +# file with more than {MAX_H_FILE_SIZE} lines. +# include-count -- a .c file with more than {MAX_INCLUDE_COUNT} #includes, +# or a .h file with more than {MAX_H_INCLUDE_COUNT} #includes. +# dependency-violation -- a file includes a header that it should +# not, according to an advisory .may_include file. +# +# Each line below represents a single exception that practracker should +# _ignore_. Each line has four parts: +# 1. The word "problem". +# 2. The kind of problem. +# 3. The location of the problem: either a filename, or a +# filename:functionname pair. +# 4. The magnitude of the problem to ignore. +# +# So for example, consider this line: +# problem file-size /src/core/or/connection_or.c 3200 +# +# It tells practracker to allow the mentioned file to be up to 3200 lines +# long, even though ordinarily it would warn about any file with more than +# {MAX_FILE_SIZE} lines. +# +# You can either edit this file by hand, or regenerate it completely by +# running `make practracker-regen`. +# +# Remember: It is better to fix the problem than to add a new exception! + +""".format(**globals()) + +def main(argv): + import argparse + + progname = argv[0] + parser = argparse.ArgumentParser(prog=progname) + parser.add_argument("--regen", action="store_true", + help="Regenerate the exceptions file") + parser.add_argument("--list-overbroad", action="store_true", + help="List over-broad exceptions") + parser.add_argument("--regen-overbroad", action="store_true", + help="Regenerate the exceptions file, " + "removing over-broad exceptions.") + parser.add_argument("--exceptions", + help="Override the location for the exceptions file") + parser.add_argument("--strict", action="store_true", + 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") + 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, + help="Maximum lines per function") + parser.add_argument("--max-dependency-violations", default=MAX_DEP_VIOLATIONS, + help="Maximum number of dependency violations to allow") + parser.add_argument("--include-dir", action="append", + default=["src"], + help="A directory (under topdir) to search for source") + parser.add_argument("topdir", default=".", nargs="?", + help="Top-level directory for the tor source") + args = parser.parse_args(argv[1:]) + + global TOR_TOPDIR + TOR_TOPDIR = args.topdir + if args.exceptions: + exceptions_file = args.exceptions + else: + exceptions_file = os.path.join(TOR_TOPDIR, "scripts/maint/practracker", EXCEPTIONS_FNAME) + + # 0) Configure our thresholds of "what is a problem actually" + filt = problem.ProblemFilter() + 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))) + filt.addThreshold(problem.DependencyViolationItem("*.c", int(args.max_dependency_violations))) + filt.addThreshold(problem.DependencyViolationItem("*.h", int(args.max_dependency_violations))) + + if args.list_overbroad + args.regen + args.regen_overbroad > 1: + print("Cannot use more than one of --regen, --list-overbroad, and " + "--regen-overbroad.", + file=sys.stderr) + sys.exit(1) + + # 1) Get all the .c files we care about + files_list = util.get_tor_c_files(TOR_TOPDIR, args.include_dir) + + # 2) Initialize problem vault and load an optional exceptions file so that + # we don't warn about the past + if args.regen: + tmpname = exceptions_file + ".tmp" + tmpfile = open(tmpname, "w") + problem_file = tmpfile + problem_file.write(HEADER) + ProblemVault = problem.ProblemVault() + else: + ProblemVault = problem.ProblemVault(exceptions_file) + problem_file = sys.stdout + + if args.list_overbroad or args.regen_overbroad: + # If we're looking for overbroad exceptions, don't list problems + # immediately to the problem file. + problem_file = util.NullFile() + + # 2.1) Adjust the exceptions so that we warn only about small problems, + # and produce errors on big ones. + if not (args.regen or args.list_overbroad or args.regen_overbroad or + args.strict): + ProblemVault.set_tolerances(TOLERANCE_FNS) + + # 3) Go through all the files and report problems if they are not exceptions + found_new_issues = 0 + for item in filt.filter(consider_all_metrics(files_list)): + status = ProblemVault.register_problem(item) + if status == problem.STATUS_ERR: + print(item, file=problem_file) + found_new_issues += 1 + elif status == problem.STATUS_WARN: + # warnings always go to stdout. + print("(warning) {}".format(item)) + + if args.regen: + tmpfile.close() + shutil.move(tmpname, exceptions_file) + sys.exit(0) + + if args.regen_overbroad: + tmpname = exceptions_file + ".tmp" + tmpfile = open(tmpname, "w") + tmpfile.write(HEADER) + for item in ProblemVault.list_exceptions_without_overbroad(): + print(item, file=tmpfile) + tmpfile.close() + shutil.move(tmpname, exceptions_file) + 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 and not args.regen and not args.terse: + new_issues_str = """\ +FAILURE: practracker found {} new problem(s) 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.\ + +You can disable this message by setting the TOR_DISABLE_PRACTRACKER environment +variable. +""".format(found_new_issues, exceptions_file) + print(new_issues_str) + + if args.list_overbroad: + def k_fn(tup): + return tup[0].key() + for (ex,p) in sorted(ProblemVault.list_overbroad_exceptions(), key=k_fn): + if p is None: + print(ex, "->", 0) + else: + print(ex, "->", p.metric_value) + + + sys.exit(found_new_issues) + +if __name__ == '__main__': + if os.environ.get("TOR_DISABLE_PRACTRACKER"): + print("TOR_DISABLE_PRACTRACKER is set, skipping practracker tests.", + file=sys.stderr) + sys.exit(0) + main(sys.argv) diff --git a/scripts/maint/practracker/practracker_tests.py b/scripts/maint/practracker/practracker_tests.py new file mode 100755 index 0000000000..e03c9e05ae --- /dev/null +++ b/scripts/maint/practracker/practracker_tests.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python + +"""Some simple tests for practracker metrics""" + +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +import unittest + +try: + # python 2 names the module this way... + from StringIO import StringIO +except ImportError: + # python 3 names the module this way. + from io 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(function_file) + # All functions should have length 2 + for name, lines in metrics.get_function_lines(funcs): + self.assertEqual(name, "fun") + + funcs.seek(0) + + for name, lines in metrics.get_function_lines(funcs): + self.assertEqual(lines, 4) + +class TestIncludeCount(unittest.TestCase): + def test_include_count(self): + f = StringIO(""" + # include <abc.h> + # include "def.h" +#include "ghi.h" +\t#\t include "jkl.h" +""") + self.assertEqual(metrics.get_include_count(f),4) + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py new file mode 100644 index 0000000000..a3255dcc80 --- /dev/null +++ b/scripts/maint/practracker/problem.py @@ -0,0 +1,266 @@ +""" +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. +""" + +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +import os.path +import re +import sys + +STATUS_ERR = 2 +STATUS_WARN = 1 +STATUS_OK = 0 + +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=None): + # Exception dictionary: { problem.key() : Problem object } + self.exceptions = {} + # Exception list: list of Problem objects, in the order added. + self.exception_list = [] + # Exception dictionary: maps key to the problem it was used to + # suppress. + self.used_exception_for = {} + + if exception_fname == None: + return + + 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 lineno, line in enumerate(exception_file, 1): + try: + problem = get_old_problem_from_exception_str(line) + except ValueError as v: + print("Exception file line {} not recognized: {}" + .format(lineno,v), + file=sys.stderr) + continue + + 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 + self.exception_list.append(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. A true + value may be STATUS_ERR to indicate a hard violation, or STATUS_WARN + to indicate a warning. + """ + # This is a new problem, print it + if problem.key() not in self.exceptions: + return STATUS_ERR + + # 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). + status = problem.is_worse_than(self.exceptions[problem.key()]) + + # Remember that we used this exception, so that we can later + # determine whether the exception was overbroad. + self.used_exception_for[problem.key()] = problem + + return status + + def list_overbroad_exceptions(self): + """Return an iterator of tuples containing (ex,prob) where ex is an + exceptions in this vault that are stricter than it needs to be, and + prob is the worst problem (if any) that it covered. + """ + for k in self.exceptions: + e = self.exceptions[k] + p = self.used_exception_for.get(k) + if p is None or e.is_worse_than(p): + yield (e, p) + + def list_exceptions_without_overbroad(self): + """Return an iterator of new problems, such that overbroad + exceptions are replaced with minimally broad versions, or removed. + """ + for e in self.exception_list: + p = self.used_exception_for.get(e.key()) + if p is None: + # This exception wasn't needed at all. + continue + if e.is_worse_than(p): + # The exception is worse than the problem we found. + # Yield the problem as the new exception value. + yield p + else: + # The problem is as bad as the exception, or worse. + # Yield the exception. + yield e + + def set_tolerances(self, fns): + """Adjust the tolerances for the exceptions in this vault. Takes + a map of problem type to a function that adjusts the permitted + function to its new maximum value.""" + for k in self.exceptions: + ex = self.exceptions[k] + fn = fns.get(ex.problem_type) + if fn is not None: + ex.metric_value = fn(ex.metric_value) + +class ProblemFilter(object): + def __init__(self): + self.thresholds = dict() + + def addThreshold(self, item): + self.thresholds[(item.get_type(),item.get_file_type())] = item + + def matches(self, item): + 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) + + def filter(self, sequence): + for item in iter(sequence): + if self.matches(item): + yield item + +class Item(object): + """ + A generic measurement about some aspect of 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.warning_threshold = self.metric_value + self.problem_type = problem_type + + def is_worse_than(self, other_problem): + """Return STATUS_ERR if this is a worse problem than other_problem. + Return STATUS_WARN if it is a little worse, but falls within the + warning threshold. Return STATUS_OK if this problem is not + at all worse than other_problem. + """ + if self.metric_value > other_problem.metric_value: + return STATUS_ERR + elif self.metric_value > other_problem.warning_threshold: + return STATUS_WARN + else: + return STATUS_OK + + def key(self): + """Generate a unique key that describes this problem that can be used as a dictionary key""" + # Item 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) + + 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. + + 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(FileSizeItem, self).__init__("file-size", problem_location, metric_value) + +class IncludeCountItem(Item): + """ + 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(IncludeCountItem, self).__init__("include-count", problem_location, metric_value) + +class FunctionSizeItem(Item): + """ + 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(FunctionSizeItem, self).__init__("function-size", problem_location, metric_value) + +class DependencyViolationItem(Item): + """ + Denotes a dependency violation in a .c or .h file. A dependency violation + occurs when a file includes a file from some module that is not listed + in its .may_include file. + + The 'problem_location' is the file that contains the problem. + + The 'metric_value' is the number of forbidden includes. + """ + def __init__(self, problem_location, metric_value): + super(DependencyViolationItem, self).__init__("dependency-violation", + problem_location, + metric_value) + +comment_re = re.compile(r'#.*$') + +def get_old_problem_from_exception_str(exception_str): + orig_str = exception_str + exception_str = comment_re.sub("", exception_str) + fields = exception_str.split() + if len(fields) == 0: + # empty line or comment + return None + elif len(fields) == 4: + # valid line + _, problem_type, problem_location, metric_value = fields + else: + raise ValueError("Misformatted line {!r}".format(orig_str)) + + if problem_type == "file-size": + return FileSizeItem(problem_location, metric_value) + elif problem_type == "include-count": + return IncludeCountItem(problem_location, metric_value) + elif problem_type == "function-size": + return FunctionSizeItem(problem_location, metric_value) + elif problem_type == "dependency-violation": + return DependencyViolationItem(problem_location, metric_value) + else: + raise ValueError("Unknown exception type {!r}".format(orig_str)) diff --git a/scripts/maint/practracker/test_practracker.sh b/scripts/maint/practracker/test_practracker.sh new file mode 100755 index 0000000000..e29b9106de --- /dev/null +++ b/scripts/maint/practracker/test_practracker.sh @@ -0,0 +1,96 @@ +#!/bin/sh + +# Fail if any subprocess fails unexpectedly +set -e + +umask 077 +unset TOR_DISABLE_PRACTRACKER + +TMPDIR="" +clean () { + if [ -n "$TMPDIR" ] && [ -d "$TMPDIR" ]; then + rm -rf "$TMPDIR" + fi +} +trap clean EXIT HUP INT TERM + +if test "${PRACTRACKER_DIR}" = "" || + test ! -e "${PRACTRACKER_DIR}/practracker.py" ; then + PRACTRACKER_DIR=$(dirname "$0") +fi + +# Change to the tor directory, and canonicalise PRACTRACKER_DIR, +# so paths in practracker output are consistent, even in out-of-tree builds +cd "${PRACTRACKER_DIR}"/../../.. +PRACTRACKER_DIR="scripts/maint/practracker" + +TMPDIR="$(mktemp -d -t pracktracker.test.XXXXXX)" +if test -z "${TMPDIR}" || test ! -d "${TMPDIR}" ; then + echo >&2 "mktemp failed." + exit 1; +fi + +DATA="${PRACTRACKER_DIR}/testdata" + +run_practracker() { + "${PYTHON:-python}" "${PRACTRACKER_DIR}/practracker.py" \ + --include-dir "" \ + --max-file-size=0 \ + --max-function-size=0 \ + --max-h-file-size=0 \ + --max-h-include-count=0 \ + --max-include-count=0 \ + --terse \ + "${DATA}/" "$@" || echo "practracker exit status: $?" +} +compare() { + # we can't use cmp because we need to use -b for windows + diff -b -u "$@" > "${TMPDIR}/test-diff" || true + if test -z "$(cat "${TMPDIR}"/test-diff)"; then + echo "OK" + else + cat "${TMPDIR}/test-diff" + echo "FAILED" + exit 1 + fi +} + +echo "unit tests:" + +"${PYTHON:-python}" "${PRACTRACKER_DIR}/practracker_tests.py" + +echo "ex0:" + +run_practracker --exceptions "${DATA}/ex0.txt" \ + > "${TMPDIR}/ex0-received.txt" 2>&1 + +compare "${TMPDIR}/ex0-received.txt" \ + "${DATA}/ex0-expected.txt" + +echo "ex1:" + +run_practracker --exceptions "${DATA}/ex1.txt" \ + > "${TMPDIR}/ex1-received.txt" 2>&1 + +compare "${TMPDIR}/ex1-received.txt" \ + "${DATA}/ex1-expected.txt" + +echo "ex1.overbroad:" + +run_practracker --exceptions "${DATA}/ex1.txt" --list-overbroad \ + > "${TMPDIR}/ex1-overbroad-received.txt" 2>&1 + +compare "${TMPDIR}/ex1-overbroad-received.txt" \ + "${DATA}/ex1-overbroad-expected.txt" + +echo "ex1.regen:" + +cp "${DATA}/ex1.txt" "${TMPDIR}/ex1-copy.txt" +run_practracker --exceptions "${TMPDIR}/ex1-copy.txt" --regen >/dev/null 2>&1 +compare "${TMPDIR}/ex1-copy.txt" "${DATA}/ex1-regen-expected.txt" + +echo "ex1.regen_overbroad:" + +cp "${DATA}/ex1.txt" "${TMPDIR}/ex1-copy.txt" +run_practracker --exceptions "${TMPDIR}/ex1-copy.txt" --regen-overbroad >/dev/null 2>&1 +compare "${TMPDIR}/ex1-copy.txt" "${DATA}/ex1-regen-overbroad-expected.txt" diff --git a/scripts/maint/practracker/testdata/.may_include b/scripts/maint/practracker/testdata/.may_include new file mode 100644 index 0000000000..8542a35807 --- /dev/null +++ b/scripts/maint/practracker/testdata/.may_include @@ -0,0 +1,4 @@ +!advisory + +permitted.h +ext/good.c diff --git a/scripts/maint/practracker/testdata/a.c b/scripts/maint/practracker/testdata/a.c new file mode 100644 index 0000000000..3c338ab40d --- /dev/null +++ b/scripts/maint/practracker/testdata/a.c @@ -0,0 +1,41 @@ + +#include "one.h" +#include "two.h" +#incldue "three.h" + +# include "permitted.h" + +#include "ext/good.c" +#include "bad.c" + +int +i_am_a_function(void) +{ + call(); + call(); + /* comment + + another */ + + return 3; +} + +# include "five.h" + +long +another_function(long x, + long y) +{ + int abcd; + + abcd = x+y; + abcd *= abcd; + + /* comment here */ + + return abcd + + abcd + + abcd; +} + +/* And a comment to grow! */ diff --git a/scripts/maint/practracker/testdata/b.c b/scripts/maint/practracker/testdata/b.c new file mode 100644 index 0000000000..bef277aaae --- /dev/null +++ b/scripts/maint/practracker/testdata/b.c @@ -0,0 +1,15 @@ + +MOCK_IMPL(int, +foo,(void)) +{ + // blah1 + return 0; +} + +MOCK_IMPL(int, +bar,( long z)) +{ + // blah2 + + return (int)(z+2); +} diff --git a/scripts/maint/practracker/testdata/ex.txt b/scripts/maint/practracker/testdata/ex.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex.txt diff --git a/scripts/maint/practracker/testdata/ex0-expected.txt b/scripts/maint/practracker/testdata/ex0-expected.txt new file mode 100644 index 0000000000..c9fb83bac3 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex0-expected.txt @@ -0,0 +1,13 @@ +Unusual pattern permitted.h in scripts/maint/practracker/testdata +problem file-size a.c 41 +problem include-count a.c 6 +problem function-size a.c:i_am_a_function() 9 +problem function-size a.c:another_function() 12 +problem dependency-violation a.c 4 +problem file-size b.c 15 +problem function-size b.c:foo() 4 +problem function-size b.c:bar() 5 +problem file-size header.h 8 +problem include-count header.h 4 +problem dependency-violation header.h 3 +practracker exit status: 11 diff --git a/scripts/maint/practracker/testdata/ex0.txt b/scripts/maint/practracker/testdata/ex0.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex0.txt diff --git a/scripts/maint/practracker/testdata/ex1-expected.txt b/scripts/maint/practracker/testdata/ex1-expected.txt new file mode 100644 index 0000000000..2713338ae4 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-expected.txt @@ -0,0 +1,5 @@ +Unusual pattern permitted.h in scripts/maint/practracker/testdata +problem function-size a.c:i_am_a_function() 9 +(warning) problem function-size a.c:another_function() 12 +problem function-size b.c:foo() 4 +practracker exit status: 2 diff --git a/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt new file mode 100644 index 0000000000..5ca480dc04 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt @@ -0,0 +1,4 @@ +Unusual pattern permitted.h in scripts/maint/practracker/testdata +problem file-size a.c 45 -> 41 +problem file-size z.c 100 -> 0 +practracker exit status: 3 diff --git a/scripts/maint/practracker/testdata/ex1-regen-expected.txt b/scripts/maint/practracker/testdata/ex1-regen-expected.txt new file mode 100644 index 0000000000..bdf3681edf --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-regen-expected.txt @@ -0,0 +1,46 @@ +# Welcome to the exceptions file for Tor's best-practices tracker! +# +# Each line of this file represents a single violation of Tor's best +# practices -- typically, a violation that we had before practracker.py +# first existed. +# +# There are three kinds of problems that we recognize right now: +# function-size -- a function of more than 100 lines. +# file-size -- a .c file of more than 3000 lines, or a .h +# file with more than 500 lines. +# include-count -- a .c file with more than 50 #includes, +# or a .h file with more than 15 #includes. +# dependency-violation -- a file includes a header that it should +# not, according to an advisory .may_include file. +# +# Each line below represents a single exception that practracker should +# _ignore_. Each line has four parts: +# 1. The word "problem". +# 2. The kind of problem. +# 3. The location of the problem: either a filename, or a +# filename:functionname pair. +# 4. The magnitude of the problem to ignore. +# +# So for example, consider this line: +# problem file-size /src/core/or/connection_or.c 3200 +# +# It tells practracker to allow the mentioned file to be up to 3200 lines +# long, even though ordinarily it would warn about any file with more than +# 3000 lines. +# +# You can either edit this file by hand, or regenerate it completely by +# running `make practracker-regen`. +# +# Remember: It is better to fix the problem than to add a new exception! + +problem file-size a.c 41 +problem include-count a.c 6 +problem function-size a.c:i_am_a_function() 9 +problem function-size a.c:another_function() 12 +problem dependency-violation a.c 4 +problem file-size b.c 15 +problem function-size b.c:foo() 4 +problem function-size b.c:bar() 5 +problem file-size header.h 8 +problem include-count header.h 4 +problem dependency-violation header.h 3 diff --git a/scripts/maint/practracker/testdata/ex1-regen-overbroad-expected.txt b/scripts/maint/practracker/testdata/ex1-regen-overbroad-expected.txt new file mode 100644 index 0000000000..4521029b10 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-regen-overbroad-expected.txt @@ -0,0 +1,45 @@ +# Welcome to the exceptions file for Tor's best-practices tracker! +# +# Each line of this file represents a single violation of Tor's best +# practices -- typically, a violation that we had before practracker.py +# first existed. +# +# There are three kinds of problems that we recognize right now: +# function-size -- a function of more than 100 lines. +# file-size -- a .c file of more than 3000 lines, or a .h +# file with more than 500 lines. +# include-count -- a .c file with more than 50 #includes, +# or a .h file with more than 15 #includes. +# dependency-violation -- a file includes a header that it should +# not, according to an advisory .may_include file. +# +# Each line below represents a single exception that practracker should +# _ignore_. Each line has four parts: +# 1. The word "problem". +# 2. The kind of problem. +# 3. The location of the problem: either a filename, or a +# filename:functionname pair. +# 4. The magnitude of the problem to ignore. +# +# So for example, consider this line: +# problem file-size /src/core/or/connection_or.c 3200 +# +# It tells practracker to allow the mentioned file to be up to 3200 lines +# long, even though ordinarily it would warn about any file with more than +# 3000 lines. +# +# You can either edit this file by hand, or regenerate it completely by +# running `make practracker-regen`. +# +# Remember: It is better to fix the problem than to add a new exception! + +problem file-size a.c 41 +problem include-count a.c 6 +problem function-size a.c:i_am_a_function() 8 +problem function-size a.c:another_function() 11 +problem file-size b.c 15 +problem function-size b.c:bar() 5 +problem dependency-violation a.c 4 +problem dependency-violation header.h 3 +problem file-size header.h 8 +problem include-count header.h 4 diff --git a/scripts/maint/practracker/testdata/ex1.txt b/scripts/maint/practracker/testdata/ex1.txt new file mode 100644 index 0000000000..af8de03291 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1.txt @@ -0,0 +1,18 @@ + +problem file-size a.c 45 +problem include-count a.c 6 +# this problem will produce an error +problem function-size a.c:i_am_a_function() 8 +# this problem will produce a warning +problem function-size a.c:another_function() 11 +problem file-size b.c 15 +# This is removed, and so will produce an error. +# problem function-size b.c:foo() 4 +# This exception isn't used. +problem file-size z.c 100 + +problem function-size b.c:bar() 5 +problem dependency-violation a.c 4 +problem dependency-violation header.h 3 +problem file-size header.h 8 +problem include-count header.h 4 diff --git a/scripts/maint/practracker/testdata/header.h b/scripts/maint/practracker/testdata/header.h new file mode 100644 index 0000000000..1183f5db9a --- /dev/null +++ b/scripts/maint/practracker/testdata/header.h @@ -0,0 +1,8 @@ + +// some forbidden includes +#include "foo.h" +#include "quux.h" +#include "quup.h" + +// a permitted include +#include "permitted.h" diff --git a/scripts/maint/practracker/testdata/not_c_file b/scripts/maint/practracker/testdata/not_c_file new file mode 100644 index 0000000000..e150962c02 --- /dev/null +++ b/scripts/maint/practracker/testdata/not_c_file @@ -0,0 +1,2 @@ + +This isn't a C file, so practracker shouldn't care about it. diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py new file mode 100644 index 0000000000..c52ca2fbbf --- /dev/null +++ b/scripts/maint/practracker/util.py @@ -0,0 +1,61 @@ +# Future imports for Python 2.7, mandatory in 3.0 +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +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/rust/", + "src/ext/" } + +EXCLUDE_FILES = {"orconfig.h"} + +def _norm(p): + return os.path.normcase(os.path.normpath(p)) + +def get_tor_c_files(tor_topdir, include_dirs=None): + """ + 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 } + + if include_dirs is None: + topdirs = [ tor_topdir ] + else: + topdirs = [ os.path.join(tor_topdir, inc) for inc in include_dirs ] + + for topdir in topdirs: + for root, directories, filenames in os.walk(topdir): + # Remove all the directories that are excluded. + directories[:] = [ d for d in directories + if _norm(os.path.join(root,d)) not in exclude_dirs ] + directories.sort() + filenames.sort() + for filename in filenames: + # We only care about .c and .h files + if not (filename.endswith(".c") or filename.endswith(".h")): + continue + if filename in EXCLUDE_FILES: + continue + # Avoid editor temporary files + bname = os.path.basename(filename) + if bname.startswith("."): + continue + if bname.startswith("#"): + continue + + full_path = os.path.join(root,filename) + + files_list.append(full_path) + + return files_list + +class NullFile: + """A file-like object that we can us to suppress output.""" + def __init__(self): + pass + def write(self, s): + pass |