March 2019 - tor-commits - lists.torproject.org

[tor/master] practracker: split shutdown code out of main.c
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit ab6ddc7a33f7ae6611ebaa00b6bc526d9c286b41 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 09:37:44 2019 -0400 practracker: split shutdown code out of main.c This is necessary to get the number of includes in main.c back under control. (In the future, we could just use the subsystem manager for this kind of stuff.) --- src/app/main/main.c | 188 +++++++---------------------------------------- src/app/main/main.h | 3 - src/app/main/ntmain.c | 1 + src/app/main/shutdown.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++ src/app/main/shutdown.h | 18 +++++ src/core/include.am | 2 + 6 files changed, 240 insertions(+), 164 deletions(-) diff --git a/src/app/main/main.c b/src/app/main/main.c index e0f635287..21e02f9ef 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -15,6 +15,7 @@ #include "app/config/statefile.h" #include "app/main/main.h" #include "app/main/ntmain.h" +#include "app/main/shutdown.h" #include "app/main/subsysmgr.h" #include "core/mainloop/connection.h" #include "core/mainloop/cpuworker.h" @@ -24,55 +25,35 @@ #include "core/or/channel.h" #include "core/or/channelpadding.h" #include "core/or/circuitpadding.h" -#include "core/or/channeltls.h" #include "core/or/circuitlist.h" -#include "core/or/circuitmux_ewma.h" #include "core/or/command.h" -#include "core/or/connection_edge.h" #include "core/or/connection_or.h" -#include "core/or/dos.h" -#include "core/or/policies.h" -#include "core/or/protover.h" #include "core/or/relay.h" -#include "core/or/scheduler.h" #include "core/or/status.h" -#include "core/or/versions.h" #include "feature/api/tor_api.h" #include "feature/api/tor_api_internal.h" #include "feature/client/addressmap.h" -#include "feature/client/bridges.h" -#include "feature/client/entrynodes.h" -#include "feature/client/transports.h" #include "feature/control/control.h" -#include "feature/dirauth/bwauth.h" #include "feature/dirauth/keypin.h" #include "feature/dirauth/process_descs.h" #include "feature/dircache/consdiffmgr.h" -#include "feature/dircache/dirserv.h" #include "feature/dirparse/routerparse.h" #include "feature/hibernate/hibernate.h" -#include "feature/hs/hs_cache.h" #include "feature/nodelist/authcert.h" -#include "feature/nodelist/microdesc.h" #include "feature/nodelist/networkstatus.h" -#include "feature/nodelist/nodelist.h" #include "feature/nodelist/routerlist.h" #include "feature/relay/dns.h" #include "feature/relay/ext_orport.h" -#include "feature/relay/onion_queue.h" #include "feature/relay/routerkeys.h" #include "feature/relay/routermode.h" #include "feature/rend/rendcache.h" -#include "feature/rend/rendclient.h" #include "feature/rend/rendservice.h" -#include "feature/stats/geoip_stats.h" #include "feature/stats/predict_ports.h" #include "feature/stats/rephist.h" #include "lib/compress/compress.h" #include "lib/buf/buffers.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_s2k.h" -#include "lib/geoip/geoip.h" #include "lib/net/resolve.h" #include "lib/process/waitpid.h" @@ -92,7 +73,6 @@ #include <event2/event.h> -#include "feature/dirauth/dirvote.h" #include "feature/dirauth/authmode.h" #include "feature/dirauth/shared_random.h" @@ -113,8 +93,6 @@ #include <systemd/sd-daemon.h> #endif /* defined(HAVE_SYSTEMD) */ -void evdns_shutdown(int); - #ifdef HAVE_RUST // helper function defined in Rust to output a log message indicating if tor is // running with Rust enabled. See src/rust/tor_util @@ -744,87 +722,6 @@ release_lockfile(void) } } -/** Free all memory that we might have allocated somewhere. - * If postfork, we are a worker process and we want to free - * only the parts of memory that we won't touch. If !postfork, - * Tor is shutting down and we should free everything. - * - * Helps us find the real leaks with sanitizers and the like. Also valgrind - * should then report 0 reachable in its leak report (in an ideal world -- - * in practice libevent, SSL, libc etc never quite free everything). */ -void -tor_free_all(int postfork) -{ - if (!postfork) { - evdns_shutdown(1); - } - geoip_free_all(); - geoip_stats_free_all(); - dirvote_free_all(); - routerlist_free_all(); - networkstatus_free_all(); - addressmap_free_all(); - dirserv_free_fingerprint_list(); - dirserv_free_all(); - dirserv_clear_measured_bw_cache(); - rend_cache_free_all(); - rend_service_authorization_free_all(); - rep_hist_free_all(); - dns_free_all(); - clear_pending_onions(); - circuit_free_all(); - circpad_machines_free(); - entry_guards_free_all(); - pt_free_all(); - channel_tls_free_all(); - channel_free_all(); - connection_free_all(); - connection_edge_free_all(); - scheduler_free_all(); - nodelist_free_all(); - microdesc_free_all(); - routerparse_free_all(); - ext_orport_free_all(); - control_free_all(); - protover_free_all(); - bridges_free_all(); - consdiffmgr_free_all(); - hs_free_all(); - dos_free_all(); - circuitmux_ewma_free_all(); - accounting_free_all(); - protover_summary_cache_free_all(); - - if (!postfork) { - config_free_all(); - or_state_free_all(); - router_free_all(); - routerkeys_free_all(); - policies_free_all(); - } - if (!postfork) { -#ifndef _WIN32 - tor_getpwnam(NULL); -#endif - } - /* stuff in main.c */ - - tor_mainloop_disconnect_pubsub(); - tor_mainloop_free_all(); - - if (!postfork) { - release_lockfile(); - } - tor_libevent_free_all(); - - subsystems_shutdown(); - - /* Stuff in util.c and address.c*/ - if (!postfork) { - esc_router_info(NULL); - } -} - /** * Remove the specified file, and log a warning if the operation fails for * any reason other than the file not existing. Ignores NULL filenames. @@ -838,50 +735,6 @@ tor_remove_file(const char *filename) } } -/** Do whatever cleanup is necessary before shutting Tor down. */ -void -tor_cleanup(void) -{ - const or_options_t *options = get_options(); - if (options->command == CMD_RUN_TOR) { - time_t now = time(NULL); - /* Remove our pid file. We don't care if there was an error when we - * unlink, nothing we could do about it anyways. */ - tor_remove_file(options->PidFile); - /* Remove control port file */ - tor_remove_file(options->ControlPortWriteToFile); - /* Remove cookie authentication file */ - { - char *cookie_fname = get_controller_cookie_file_name(); - tor_remove_file(cookie_fname); - tor_free(cookie_fname); - } - /* Remove Extended ORPort cookie authentication file */ - { - char *cookie_fname = get_ext_or_auth_cookie_file_name(); - tor_remove_file(cookie_fname); - tor_free(cookie_fname); - } - if (accounting_is_enabled(options)) - accounting_record_bandwidth_usage(now, get_or_state()); - or_state_mark_dirty(get_or_state(), 0); /* force an immediate save. */ - or_state_save(now); - if (authdir_mode(options)) { - sr_save_and_cleanup(); - } - if (authdir_mode_tests_reachability(options)) - rep_hist_record_mtbf_data(now, 0); - keypin_close_journal(); - } - - timers_shutdown(); - - tor_free_all(0); /* We could move tor_free_all back into the ifdef below - later, if it makes shutdown unacceptably slow. But for - now, leave it here: it's helped us catch bugs in the - past. */ -} - /** Read/create keys as needed, and echo our fingerprint to stdout. */ static int do_list_fingerprint(void) @@ -1379,6 +1232,30 @@ run_tor_main_loop(void) return do_main_loop(); } +/** Install the publish/subscribe relationships for all the subsystems. */ +static void +pubsub_install(void) +{ + pubsub_builder_t *builder = pubsub_builder_new(); + int r = subsystems_add_pubsub(builder); + tor_assert(r == 0); + r = tor_mainloop_connect_pubsub(builder); // consumes builder + tor_assert(r == 0); +} + +/** Connect the mainloop to its publish/subscribe message delivery events if + * appropriate, and configure the global channels appropriately. */ +static void +pubsub_connect(void) +{ + if (get_options()->command == CMD_RUN_TOR) { + tor_mainloop_connect_pubsub_events(); + /* XXXX For each pubsub channel, its delivery strategy should be set at + * this XXXX point, using tor_mainloop_set_delivery_strategy(). + */ + } +} + /* Main entry point for the Tor process. Called from tor_main(), and by * anybody embedding Tor. */ int @@ -1411,13 +1288,7 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) } #endif /* defined(NT_SERVICE) */ - { - pubsub_builder_t *builder = pubsub_builder_new(); - int r = subsystems_add_pubsub(builder); - tor_assert(r == 0); - r = tor_mainloop_connect_pubsub(builder); // consumes builder - tor_assert(r == 0); - } + pubsub_install(); { int init_rv = tor_init(argc, argv); @@ -1428,12 +1299,7 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) } } - if (get_options()->command == CMD_RUN_TOR) { - tor_mainloop_connect_pubsub_events(); - /* XXXX For each pubsub channel, its delivery strategy should be set at - * this XXXX point, using tor_mainloop_set_delivery_strategy(). - */ - } + pubsub_connect(); if (get_options()->Sandbox && get_options()->command == CMD_RUN_TOR) { sandbox_cfg_t* cfg = sandbox_init_filter(); diff --git a/src/app/main/main.h b/src/app/main/main.h index 23a436703..691008209 100644 --- a/src/app/main/main.h +++ b/src/app/main/main.h @@ -21,9 +21,6 @@ void release_lockfile(void); void tor_remove_file(const char *filename); -void tor_cleanup(void); -void tor_free_all(int postfork); - int tor_init(int argc, char **argv); int run_tor_main_loop(void); diff --git a/src/app/main/ntmain.c b/src/app/main/ntmain.c index 8d2135a58..00afbf375 100644 --- a/src/app/main/ntmain.c +++ b/src/app/main/ntmain.c @@ -24,6 +24,7 @@ #include "app/config/config.h" #include "app/main/main.h" #include "app/main/ntmain.h" +#include "app/main/shutdown.h" #include "core/mainloop/mainloop.h" #include "lib/evloop/compat_libevent.h" #include "lib/fs/winlib.h" diff --git a/src/app/main/shutdown.c b/src/app/main/shutdown.c new file mode 100644 index 000000000..a4e6cb088 --- /dev/null +++ b/src/app/main/shutdown.c @@ -0,0 +1,192 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file shutdown.c + * @brief Code to free global resources used by Tor. + * + * In the future, this should all be handled by the subsystem manager. */ + +#include "core/or/or.h" + +#include "app/config/config.h" +#include "app/config/statefile.h" +#include "app/main/main.h" +#include "app/main/shutdown.h" +#include "app/main/subsysmgr.h" +#include "core/mainloop/connection.h" +#include "core/mainloop/mainloop.h" +#include "core/mainloop/mainloop_pubsub.h" +#include "core/or/channeltls.h" +#include "core/or/circuitlist.h" +#include "core/or/circuitmux_ewma.h" +#include "core/or/circuitpadding.h" +#include "core/or/connection_edge.h" +#include "core/or/dos.h" +#include "core/or/policies.h" +#include "core/or/protover.h" +#include "core/or/scheduler.h" +#include "core/or/versions.h" +#include "feature/client/addressmap.h" +#include "feature/client/bridges.h" +#include "feature/client/entrynodes.h" +#include "feature/client/transports.h" +#include "feature/control/control.h" +#include "feature/control/control.h" +#include "feature/dirauth/authmode.h" +#include "feature/dirauth/bwauth.h" +#include "feature/dirauth/dirvote.h" +#include "feature/dirauth/keypin.h" +#include "feature/dirauth/process_descs.h" +#include "feature/dirauth/shared_random.h" +#include "feature/dircache/consdiffmgr.h" +#include "feature/dircache/dirserv.h" +#include "feature/dirparse/routerparse.h" +#include "feature/hibernate/hibernate.h" +#include "feature/hs/hs_common.h" +#include "feature/nodelist/microdesc.h" +#include "feature/nodelist/networkstatus.h" +#include "feature/nodelist/nodelist.h" +#include "feature/nodelist/routerlist.h" +#include "feature/nodelist/routerlist.h" +#include "feature/relay/dns.h" +#include "feature/relay/ext_orport.h" +#include "feature/relay/onion_queue.h" +#include "feature/relay/routerkeys.h" +#include "feature/rend/rendcache.h" +#include "feature/rend/rendclient.h" +#include "feature/stats/geoip_stats.h" +#include "feature/stats/rephist.h" +#include "lib/evloop/compat_libevent.h" +#include "lib/geoip/geoip.h" +#include "src/feature/relay/router.h" + +void evdns_shutdown(int); + +/** Do whatever cleanup is necessary before shutting Tor down. */ +void +tor_cleanup(void) +{ + const or_options_t *options = get_options(); + if (options->command == CMD_RUN_TOR) { + time_t now = time(NULL); + /* Remove our pid file. We don't care if there was an error when we + * unlink, nothing we could do about it anyways. */ + tor_remove_file(options->PidFile); + /* Remove control port file */ + tor_remove_file(options->ControlPortWriteToFile); + /* Remove cookie authentication file */ + { + char *cookie_fname = get_controller_cookie_file_name(); + tor_remove_file(cookie_fname); + tor_free(cookie_fname); + } + /* Remove Extended ORPort cookie authentication file */ + { + char *cookie_fname = get_ext_or_auth_cookie_file_name(); + tor_remove_file(cookie_fname); + tor_free(cookie_fname); + } + if (accounting_is_enabled(options)) + accounting_record_bandwidth_usage(now, get_or_state()); + or_state_mark_dirty(get_or_state(), 0); /* force an immediate save. */ + or_state_save(now); + if (authdir_mode(options)) { + sr_save_and_cleanup(); + } + if (authdir_mode_tests_reachability(options)) + rep_hist_record_mtbf_data(now, 0); + keypin_close_journal(); + } + + timers_shutdown(); + + tor_free_all(0); /* We could move tor_free_all back into the ifdef below + later, if it makes shutdown unacceptably slow. But for + now, leave it here: it's helped us catch bugs in the + past. */ +} + +/** Free all memory that we might have allocated somewhere. + * If postfork, we are a worker process and we want to free + * only the parts of memory that we won't touch. If !postfork, + * Tor is shutting down and we should free everything. + * + * Helps us find the real leaks with sanitizers and the like. Also valgrind + * should then report 0 reachable in its leak report (in an ideal world -- + * in practice libevent, SSL, libc etc never quite free everything). */ +void +tor_free_all(int postfork) +{ + if (!postfork) { + evdns_shutdown(1); + } + geoip_free_all(); + geoip_stats_free_all(); + dirvote_free_all(); + routerlist_free_all(); + networkstatus_free_all(); + addressmap_free_all(); + dirserv_free_fingerprint_list(); + dirserv_free_all(); + dirserv_clear_measured_bw_cache(); + rend_cache_free_all(); + rend_service_authorization_free_all(); + rep_hist_free_all(); + dns_free_all(); + clear_pending_onions(); + circuit_free_all(); + circpad_machines_free(); + entry_guards_free_all(); + pt_free_all(); + channel_tls_free_all(); + channel_free_all(); + connection_free_all(); + connection_edge_free_all(); + scheduler_free_all(); + nodelist_free_all(); + microdesc_free_all(); + routerparse_free_all(); + ext_orport_free_all(); + control_free_all(); + protover_free_all(); + bridges_free_all(); + consdiffmgr_free_all(); + hs_free_all(); + dos_free_all(); + circuitmux_ewma_free_all(); + accounting_free_all(); + protover_summary_cache_free_all(); + + if (!postfork) { + config_free_all(); + or_state_free_all(); + router_free_all(); + routerkeys_free_all(); + policies_free_all(); + } + if (!postfork) { +#ifndef _WIN32 + tor_getpwnam(NULL); +#endif + } + /* stuff in main.c */ + + tor_mainloop_disconnect_pubsub(); + tor_mainloop_free_all(); + + if (!postfork) { + release_lockfile(); + } + tor_libevent_free_all(); + + subsystems_shutdown(); + + /* Stuff in util.c and address.c*/ + if (!postfork) { + esc_router_info(NULL); + } +} diff --git a/src/app/main/shutdown.h b/src/app/main/shutdown.h new file mode 100644 index 000000000..1bca96a0a --- /dev/null +++ b/src/app/main/shutdown.h @@ -0,0 +1,18 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file shutdown.h + * \brief Header file for shutdown.c. + **/ + +#ifndef TOR_SHUTDOWN_H +#define TOR_SHUTDOWN_H + +void tor_cleanup(void); +void tor_free_all(int postfork); + +#endif /* !defined(TOR_SHUTDOWN_H) */ diff --git a/src/core/include.am b/src/core/include.am index 3a0e907ed..7fb5c1d7a 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -11,6 +11,7 @@ LIBTOR_APP_A_SOURCES = \ src/app/config/confparse.c \ src/app/config/statefile.c \ src/app/main/main.c \ + src/app/main/shutdown.c \ src/app/main/subsystem_list.c \ src/app/main/subsysmgr.c \ src/core/crypto/hs_ntor.c \ @@ -204,6 +205,7 @@ noinst_HEADERS += \ src/app/config/statefile.h \ src/app/main/main.h \ src/app/main/ntmain.h \ + src/app/main/shutdown.h \ src/app/main/subsysmgr.h \ src/core/crypto/hs_ntor.h \ src/core/crypto/onion_crypto.h \

1 0

[tor/master] practracker compliance: split lint_message into more logical parts
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 4bdff5e3e9d4ea11c7e8043e75d63c4f366558e8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 08:54:20 2019 -0400 practracker compliance: split lint_message into more logical parts --- src/lib/pubsub/pubsub_check.c | 237 ++++++++++++++++++++++++++---------------- 1 file changed, 149 insertions(+), 88 deletions(-) diff --git a/src/lib/pubsub/pubsub_check.c b/src/lib/pubsub/pubsub_check.c index e87c347c4..94a55beeb 100644 --- a/src/lib/pubsub/pubsub_check.c +++ b/src/lib/pubsub/pubsub_check.c @@ -171,115 +171,119 @@ pubsub_cfg_dump(const pubsub_cfg_t *cfg, int severity, const char *prefix) } /** - * Check whether there are any errors or inconsistencies for the message - * described by msg in map. If there are problems, log about - * them, and return -1. Otherwise return 0. + * Helper: fill a bitarray out with entries corresponding to the + * subsystems listed in items. If any subsystem is listed more than + * once, log a warning. Return 0 on success, -1 on failure. **/ static int -lint_message(const pubsub_adjmap_t *map, message_id_t msg) +get_message_bitarray(const pubsub_adjmap_t *map, + message_id_t msg, + const smartlist_t *items, + const char *operation, + bitarray_t **out) { - /* NOTE: Some of the checks in this function are maybe over-zealous, and we - * might not want to have them forever. I've marked them with [?] below. - */ - if (BUG(msg >= map->n_msgs)) - return 0; // LCOV_EXCL_LINE - - const smartlist_t *pub = map->pub_by_msg[msg]; - const smartlist_t *sub = map->sub_by_msg[msg]; + bool ok = true; + *out = bitarray_init_zero((unsigned)map->n_subsystems); + if (! items) + return 0; - const size_t n_pub = smartlist_len_opt(pub); - const size_t n_sub = smartlist_len_opt(sub); + SMARTLIST_FOREACH_BEGIN(items, const pubsub_cfg_t *, cfg) { + if (bitarray_is_set(*out, cfg->subsys)) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is configured to be %s by subsystem " + "%u (%s) more than once.", + msg, get_message_id_name(msg), operation, + cfg->subsys, get_subsys_id_name(cfg->subsys)); + ok = false; + } + bitarray_set(*out, cfg->subsys); + } SMARTLIST_FOREACH_END(cfg); - if (n_pub == 0 && n_sub == 0) { - log_info(LD_MESG, "Nobody is publishing or subscribing to message %u " - "(%s).", - msg, get_message_id_name(msg)); - return 0; // No publishers or subscribers: nothing to do. - } + return ok ? 0 : -1; +} - /* We'll set this to false if there are any problems. */ +/** + * Helper for lint_message: check that all the pubsub_cfg_t items in the two + * respective smartlists obey our local graph topology rules. + * + * (Right now this is just a matter of "each subsystem only + * publishes/subscribes once; no subsystem is a publisher and subscriber for + * the same message.") + * + * Return 0 on success, -1 on failure. + **/ +static int +lint_message_graph(const pubsub_adjmap_t *map, + message_id_t msg, + const smartlist_t *pub, + const smartlist_t *sub) +{ + bitarray_t *published_by = NULL; + bitarray_t *subscribed_by = NULL; bool ok = true; - /* First make sure that if there are publishers, there are subscribers. */ - if (n_pub == 0) { - log_warn(LD_MESG|LD_BUG, - "Message %u (%s) has subscribers, but no publishers.", - msg, get_message_id_name(msg)); + if (get_message_bitarray(map, msg, pub, "published", &published_by) < 0) ok = false; - } else if (n_sub == 0) { - log_warn(LD_MESG|LD_BUG, - "Message %u (%s) has publishers, but no subscribers.", - msg, get_message_id_name(msg)); + if (get_message_bitarray(map, msg, sub, "subscribed", &subscribed_by) < 0) ok = false; + + /* Check whether any subsystem is publishing and subscribing the same + * message. [??] + */ + for (unsigned i = 0; i < map->n_subsystems; ++i) { + if (bitarray_is_set(published_by, i) && + bitarray_is_set(subscribed_by, i)) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is published and subscribed by the same " + "subsystem %u (%s)", + msg, get_message_id_name(msg), + i, get_subsys_id_name(i)); + ok = false; + } } + bitarray_free(published_by); + bitarray_free(subscribed_by); + + return ok ? 0 : -1; +} + +/** + * Helper for lint_message: check that all the pubsub_cfg_t items in the two + * respective smartlists have compatible flags, channels, and types. + **/ +static int +lint_message_consistency(message_id_t msg, + const smartlist_t *pub, + const smartlist_t *sub) +{ + if (!smartlist_len_opt(pub) && !smartlist_len_opt(sub)) + return 0; // LCOV_EXCL_LINE -- this was already checked. + /* The 'all' list has the publishers and the subscribers. */ smartlist_t *all = smartlist_new(); if (pub) smartlist_add_all(all, pub); if (sub) smartlist_add_all(all, sub); + const pubsub_cfg_t *item0 = smartlist_get(all, 0); /* Indicates which subsystems we've found publishing/subscribing here. */ - bitarray_t *published_by = bitarray_init_zero((unsigned)map->n_subsystems); - bitarray_t *subscribed_by = bitarray_init_zero((unsigned)map->n_subsystems); bool pub_excl = false, sub_excl = false, chan_same = true, type_same = true; - /* Make sure that the messages all have the same channel and type; - * check whether the DISP_FLAG_EXCL flag is used; - * and if any subsystem is publishing or subscribing to it twice [??]. + /* Simple message consistency properties across messages. */ SMARTLIST_FOREACH_BEGIN(all, const pubsub_cfg_t *, cfg) { - if (cfg->channel != item0->channel) { - chan_same = false; - } - if (cfg->type != item0->type) { - type_same = false; - } - if (cfg->flags & DISP_FLAG_EXCL) { - if (cfg->is_publish) - pub_excl = true; - else - sub_excl = true; - } - if (cfg->is_publish) { - if (bitarray_is_set(published_by, cfg->subsys)) { - log_warn(LD_MESG|LD_BUG, - "Message %u (%s) is configured to be published by subsystem " - "%u (%s) more than once.", - msg, get_message_id_name(msg), - cfg->subsys, get_subsys_id_name(cfg->subsys)); - ok = false; - } - bitarray_set(published_by, cfg->subsys); - } else { - if (bitarray_is_set(subscribed_by, cfg->subsys)) { - log_warn(LD_MESG|LD_BUG, - "Message %u (%s) is configured to be subscribed by subsystem " - "%u (%s) more than once.", - msg, get_message_id_name(msg), - cfg->subsys, get_subsys_id_name(cfg->subsys)); - ok = false; - } - bitarray_set(subscribed_by, cfg->subsys); - } + chan_same &= (cfg->channel == item0->channel); + type_same &= (cfg->type == item0->type); + if (cfg->is_publish) + pub_excl |= (cfg->flags & DISP_FLAG_EXCL) != 0; + else + sub_excl |= (cfg->flags & DISP_FLAG_EXCL) != 0; } SMARTLIST_FOREACH_END(cfg); - /* Check whether any subsystem is publishing and subscribing the same - * message. [??] - */ - for (unsigned i = 0; i < map->n_subsystems; ++i) { - if (bitarray_is_set(published_by, i) && - bitarray_is_set(subscribed_by, i)) { - log_warn(LD_MESG|LD_BUG, - "Message %u (%s) is published and subscribed by the same " - "subsystem %u (%s)", - msg, get_message_id_name(msg), - i, get_subsys_id_name(i)); - ok = false; - } - } + bool ok = true; if (! chan_same) { log_warn(LD_MESG|LD_BUG, @@ -314,17 +318,74 @@ lint_message(const pubsub_adjmap_t *map, message_id_t msg) ok = false; } + smartlist_free(all); + + return ok ? 0 : -1; +} + +/** + * Check whether there are any errors or inconsistencies for the message + * described by msg in map. If there are problems, log about + * them, and return -1. Otherwise return 0. + **/ +static int +lint_message(const pubsub_adjmap_t *map, message_id_t msg) +{ + /* NOTE: Some of the checks in this function are maybe over-zealous, and we + * might not want to have them forever. I've marked them with [?] below. + */ + if (BUG(msg >= map->n_msgs)) + return 0; // LCOV_EXCL_LINE + + const smartlist_t *pub = map->pub_by_msg[msg]; + const smartlist_t *sub = map->sub_by_msg[msg]; + + const size_t n_pub = smartlist_len_opt(pub); + const size_t n_sub = smartlist_len_opt(sub); + + if (n_pub == 0 && n_sub == 0) { + log_info(LD_MESG, "Nobody is publishing or subscribing to message %u " + "(%s).", + msg, get_message_id_name(msg)); + return 0; // No publishers or subscribers: nothing to do. + } + /* We'll set this to false if there are any problems. */ + bool ok = true; + + /* First make sure that if there are publishers, there are subscribers. */ + if (n_pub == 0) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has subscribers, but no publishers.", + msg, get_message_id_name(msg)); + ok = false; + } else if (n_sub == 0) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has publishers, but no subscribers.", + msg, get_message_id_name(msg)); + ok = false; + } + + /* Check the message graph topology. */ + if (lint_message_graph(map, msg, pub, sub) < 0) + ok = false; + + /* Check whether the messages have the same fields set on them. */ + if (lint_message_consistency(msg, pub, sub) < 0) + ok = false; + if (!ok) { /* There was a problem -- let's log all the publishers and subscribers on * this message */ - SMARTLIST_FOREACH(all, pubsub_cfg_t *, cfg, - pubsub_cfg_dump(cfg, LOG_WARN, " ")); + if (pub) { + SMARTLIST_FOREACH(pub, pubsub_cfg_t *, cfg, + pubsub_cfg_dump(cfg, LOG_WARN, " ")); + } + if (sub) { + SMARTLIST_FOREACH(sub, pubsub_cfg_t *, cfg, + pubsub_cfg_dump(cfg, LOG_WARN, " ")); + } } - smartlist_free(all); - bitarray_free(published_by); - bitarray_free(subscribed_by); - return ok ? 0 : -1; }

1 0

[tor/master] Add a test for pubsub_items_clear_bindings()
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 8d70f217175b69a7b8e5d35b564f50712c882d7e Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 13:23:58 2019 -0400 Add a test for pubsub_items_clear_bindings() --- src/test/test_pubsub_build.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/test_pubsub_build.c b/src/test/test_pubsub_build.c index 19aa1f0e0..f6c6b81a2 100644 --- a/src/test/test_pubsub_build.c +++ b/src/test/test_pubsub_build.c @@ -131,6 +131,14 @@ test_pubsub_build_types_ok(void *arg) tt_assert(items); tt_int_op(smartlist_len(items->items), OP_EQ, 4); + // Make sure that the bindings got build correctly. + SMARTLIST_FOREACH_BEGIN(items->items, pubsub_cfg_t *, item) { + if (item->is_publish) { + tt_assert(item->pub_binding); + tt_ptr_op(item->pub_binding->dispatch_ptr, OP_EQ, dispatcher); + } + } SMARTLIST_FOREACH_END(item); + tt_int_op(dispatcher->n_types, OP_GE, 2); tt_assert(dispatcher->typefns); @@ -138,6 +146,16 @@ test_pubsub_build_types_ok(void *arg) tt_assert(dispatcher->typefns[get_msg_type_id("string")].fmt_fn == ex_str_fmt); + // Now clear the bindings, like we would do before freeing the + // the dispatcher. + pubsub_items_clear_bindings(items); + SMARTLIST_FOREACH_BEGIN(items->items, pubsub_cfg_t *, item) { + if (item->is_publish) { + tt_assert(item->pub_binding); + tt_ptr_op(item->pub_binding->dispatch_ptr, OP_EQ, NULL); + } + } SMARTLIST_FOREACH_END(item); + done: pubsub_connector_free(c); pubsub_builder_free(b);

1 0

[tor/master] pubsub: add test for items_out in builder_finalize()
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit d976cda49f7346e8fab16c062973c38ec4da4f4b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 16:30:56 2019 -0400 pubsub: add test for items_out in builder_finalize() --- src/test/test_pubsub_build.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/test_pubsub_build.c b/src/test/test_pubsub_build.c index 86b5f763a..19aa1f0e0 100644 --- a/src/test/test_pubsub_build.c +++ b/src/test/test_pubsub_build.c @@ -2,6 +2,7 @@ /* See LICENSE for licensing information */ #define DISPATCH_PRIVATE +#define PUBSUB_PRIVATE #include "test/test.h" @@ -119,13 +120,16 @@ test_pubsub_build_types_ok(void *arg) pubsub_builder_t *b = NULL; dispatch_t *dispatcher = NULL; pubsub_connector_t *c = NULL; + pubsub_items_t *items = NULL; b = pubsub_builder_new(); seed_pubsub_builder_basic(b); - dispatcher = pubsub_builder_finalize(b, NULL); + dispatcher = pubsub_builder_finalize(b, &items); b = NULL; tt_assert(dispatcher); + tt_assert(items); + tt_int_op(smartlist_len(items->items), OP_EQ, 4); tt_int_op(dispatcher->n_types, OP_GE, 2); tt_assert(dispatcher->typefns); @@ -138,6 +142,7 @@ test_pubsub_build_types_ok(void *arg) pubsub_connector_free(c); pubsub_builder_free(b); dispatch_free(dispatcher); + pubsub_items_free(items); } /* We fail if the same type is defined in two places with different functions.

1 0

[tor/master] coverage: Exclude lines in pubsub_pub that can only be reached on bug
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 3552cd69bdeefb5d4fbbc905b24120573cc26f81 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 15:56:58 2019 -0400 coverage: Exclude lines in pubsub_pub that can only be reached on bug --- src/lib/pubsub/pubsub_publish.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/pubsub/pubsub_publish.c b/src/lib/pubsub/pubsub_publish.c index 7b070f35c..454a335a7 100644 --- a/src/lib/pubsub/pubsub_publish.c +++ b/src/lib/pubsub/pubsub_publish.c @@ -49,8 +49,10 @@ pubsub_pub_(const pub_binding_t *pub, msg_aux_data_t auxdata) if (BUG(pub->msg_template.msg >= d->n_msgs) || BUG(pub->msg_template.channel >= d->n_queues)) { /* The message ID or channel ID was out of bounds. */ + // LCOV_EXCL_START d->typefns[pub->msg_template.type].free_fn(auxdata); return -1; + // LCOV_EXCL_STOP } if (! d->table[pub->msg_template.msg]) {

1 0

[tor/master] Add test for dispatch_send() fast path.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit b11b4b7bb77c87d2673e816fde114e25a5482a6f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 11:50:45 2019 -0400 Add test for dispatch_send() fast path. --- src/test/test_dispatch.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/test/test_dispatch.c b/src/test/test_dispatch.c index 886f45868..d6fe7e781 100644 --- a/src/test/test_dispatch.c +++ b/src/test/test_dispatch.c @@ -107,6 +107,38 @@ test_dispatch_simple(void *arg) tor_free(recv2_received); } +/* Construct a dispatch_t with a message and no reciever; make sure that it + * gets dropped properly. */ +static void +test_dispatch_no_recipient(void *arg) +{ + (void)arg; + + dispatch_t *d=NULL; + dispatch_cfg_t *cfg=NULL; + int r; + + cfg = dcfg_new(); + r = dcfg_msg_set_type(cfg,0,0); + r += dcfg_msg_set_chan(cfg,0,0); + tt_int_op(r, OP_EQ, 0); + + d = dispatch_new(cfg); + tt_assert(d); + dispatcher_in_use = d; + + msg_aux_data_t data = { .u64 = 7}; + r = dispatch_send(d, 99, 0, 0, 0, data); + tt_int_op(r, OP_EQ, 0); + + r = dispatch_flush(d, 0, INT_MAX); + tt_int_op(r, OP_EQ, 0); + + done: + dispatch_free(d); + dcfg_free(cfg); +} + struct coord { int x; int y; }; static void free_coord(msg_aux_data_t d) @@ -210,6 +242,7 @@ test_dispatch_bad_type_setup(void *arg) struct testcase_t dispatch_tests[] = { T(empty), T(simple), + T(no_recipient), T(with_types), T(bad_type_setup), END_OF_TESTCASES

1 0

[tor/master] Use actual pointers in dispatch_cfg.c.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 47de9c7b0a828de7fb8129413db70bc4e4ecac6d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 15:15:03 2019 -0400 Use actual pointers in dispatch_cfg.c. Previously, I had used integers encoded as pointers. This introduced a flaw: NULL represented both the integer zero, and the absence of a setting. This in turn made the checks in cfg_msg_set_{type,chan}() not actually check for an altered value if the previous value had been set to zero. Also, I had previously kept a pointer to a dispatch_fypefns_t rather than making a copy of it. This meant that if the dispatch_typefns_t were changed between defining the typefns and creating the dispatcher, we'd get the modified version. Found while investigating coverage in pubsub_add_{pub,sub}_() --- src/lib/dispatch/dispatch_cfg.c | 23 ++++++++++++--------- src/lib/dispatch/dispatch_new.c | 46 ++++++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/lib/dispatch/dispatch_cfg.c b/src/lib/dispatch/dispatch_cfg.c index 26e37f469..b3a72ec22 100644 --- a/src/lib/dispatch/dispatch_cfg.c +++ b/src/lib/dispatch/dispatch_cfg.c @@ -37,9 +37,6 @@ dcfg_new(void) return cfg; } -/** DOCDOC */ -#define ID_TO_VOID(id) ((void*)(uintptr_t)(id)) - /** * Associate a message with a datatype. Return 0 on success, -1 if a * different type was previously associated with the message ID. @@ -49,11 +46,12 @@ dcfg_msg_set_type(dispatch_cfg_t *cfg, message_id_t msg, msg_type_id_t type) { smartlist_grow(cfg->type_by_msg, msg+1); - void *oldval = smartlist_get(cfg->type_by_msg, msg); - if (oldval != NULL && oldval != ID_TO_VOID(type)) { + msg_type_id_t *oldval = smartlist_get(cfg->type_by_msg, msg); + if (oldval != NULL && *oldval != type) { return -1; } - smartlist_set(cfg->type_by_msg, msg, ID_TO_VOID(type)); + if (!oldval) + smartlist_set(cfg->type_by_msg, msg, tor_memdup(&type, sizeof(type))); return 0; } @@ -66,11 +64,12 @@ dcfg_msg_set_chan(dispatch_cfg_t *cfg, message_id_t msg, channel_id_t chan) { smartlist_grow(cfg->chan_by_msg, msg+1); - void *oldval = smartlist_get(cfg->chan_by_msg, msg); - if (oldval != NULL && oldval != ID_TO_VOID(chan)) { + channel_id_t *oldval = smartlist_get(cfg->chan_by_msg, msg); + if (oldval != NULL && *oldval != chan) { return -1; } - smartlist_set(cfg->chan_by_msg, msg, ID_TO_VOID(chan)); + if (!oldval) + smartlist_set(cfg->chan_by_msg, msg, tor_memdup(&chan, sizeof(chan))); return 0; } @@ -87,7 +86,8 @@ dcfg_type_set_fns(dispatch_cfg_t *cfg, msg_type_id_t type, if (oldfns && (oldfns->free_fn != fns->free_fn || oldfns->fmt_fn != fns->fmt_fn)) return -1; - smartlist_set(cfg->fns_by_type, type, (dispatch_typefns_t *) fns); + if (!oldfns) + smartlist_set(cfg->fns_by_type, type, tor_memdup(fns, sizeof(*fns))); return 0; } @@ -123,6 +123,9 @@ dcfg_free_(dispatch_cfg_t *cfg) if (!cfg) return; + SMARTLIST_FOREACH(cfg->type_by_msg, msg_type_id_t *, id, tor_free(id)); + SMARTLIST_FOREACH(cfg->chan_by_msg, channel_id_t *, id, tor_free(id)); + SMARTLIST_FOREACH(cfg->fns_by_type, dispatch_typefns_t *, f, tor_free(f)); smartlist_free(cfg->type_by_msg); smartlist_free(cfg->chan_by_msg); smartlist_free(cfg->fns_by_type); diff --git a/src/lib/dispatch/dispatch_new.c b/src/lib/dispatch/dispatch_new.c index a2879016a..b89ef43ea 100644 --- a/src/lib/dispatch/dispatch_new.c +++ b/src/lib/dispatch/dispatch_new.c @@ -17,32 +17,36 @@ #include "lib/dispatch/dispatch_cfg.h" #include "lib/dispatch/dispatch_cfg_st.h" +#include "lib/cc/ctassert.h" #include "lib/intmath/cmp.h" #include "lib/malloc/malloc.h" #include "lib/log/util_bug.h" #include <string.h> -/** Convert a void* in a smartlist to the corresponding integer. */ -#define VOID_TO_ID(p) ((intptr_t)(p)) - -/** Given a smartlist full of void* fields encoding intptr_t values, - * return the largest intptr_t, or dflt if the list is empty. */ -static intptr_t -max_in_sl(const smartlist_t *sl, intptr_t dflt) +/** Given a smartlist full of (possibly NULL) pointers to uint16_t values, + * return the largest value, or dflt if the list is empty. */ +static int +max_in_sl(const smartlist_t *sl, int dflt) { - if (!smartlist_len(sl)) - return dflt; - void *as_ptr = smartlist_get(sl, 0); - intptr_t max = VOID_TO_ID(as_ptr); - SMARTLIST_FOREACH_BEGIN(sl, void *, p) { - intptr_t i = VOID_TO_ID(p); - if (i > max) - max = i; - } SMARTLIST_FOREACH_END(p); - return max; + uint16_t *maxptr = NULL; + SMARTLIST_FOREACH_BEGIN(sl, uint16_t *, u) { + if (!maxptr) + maxptr = u; + else if (*u > *maxptr) + maxptr = u; + } SMARTLIST_FOREACH_END(u); + + return maxptr ? *maxptr : dflt; } +/* The above function is only safe to call if we are sure that channel_id_t + * and msg_type_id_t are really uint16_t. They should be so defined in + * msgtypes.h, but let's be extra cautious. + */ +CTASSERT(sizeof(uint16_t) == sizeof(msg_type_id_t)); +CTASSERT(sizeof(uint16_t) == sizeof(channel_id_t)); + /** Helper: Format an unformattable message auxiliary data item: just return a * copy of the string <>. */ static char * @@ -156,14 +160,14 @@ dispatch_new(const dispatch_cfg_t *cfg) /* Fill in the empty entries in the dispatch tables: * types and channels for each message. */ - SMARTLIST_FOREACH_BEGIN(cfg->type_by_msg, smartlist_t *, type) { + SMARTLIST_FOREACH_BEGIN(cfg->type_by_msg, msg_type_id_t *, type) { if (d->table[type_sl_idx]) - d->table[type_sl_idx]->type = VOID_TO_ID(type); + d->table[type_sl_idx]->type = *type; } SMARTLIST_FOREACH_END(type); - SMARTLIST_FOREACH_BEGIN(cfg->chan_by_msg, smartlist_t *, chan) { + SMARTLIST_FOREACH_BEGIN(cfg->chan_by_msg, channel_id_t *, chan) { if (d->table[chan_sl_idx]) - d->table[chan_sl_idx]->channel = VOID_TO_ID(chan); + d->table[chan_sl_idx]->channel = *chan; } SMARTLIST_FOREACH_END(chan); return d;

1 0

[tor/master] Log warning messages _before_ exiting because of earlier dcfg failure
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit a8ca464cee14ad8d0782f626c7f36216dba2e400 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 15:22:19 2019 -0400 Log warning messages _before_ exiting because of earlier dcfg failure This helps diagnostics. --- src/lib/pubsub/pubsub_build.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/pubsub/pubsub_build.c b/src/lib/pubsub/pubsub_build.c index 64cdcc1d5..1dc20f51f 100644 --- a/src/lib/pubsub/pubsub_build.c +++ b/src/lib/pubsub/pubsub_build.c @@ -281,11 +281,14 @@ pubsub_builder_finalize(pubsub_builder_t *builder, dispatch_t *dispatcher = NULL; tor_assert_nonfatal(builder->n_connectors == 0); - if (builder->n_errors) + if (pubsub_builder_check(builder) < 0) goto err; - if (pubsub_builder_check(builder) < 0) + if (builder->n_errors) { + log_warn(LD_GENERAL, "At least one error occurred previously when " + "configuring the dispatcher."); goto err; + } dispatcher = dispatch_new(builder->cfg);

1 0

[tor/master] Various documentation notes and tweaks for pubsub
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 28fd4996aca0c560d4af0c035b53323951c13476 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 15:35:41 2019 -0400 Various documentation notes and tweaks for pubsub --- src/core/mainloop/mainloop_pubsub.c | 3 +++ src/lib/pubsub/pubsub_macros.h | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/core/mainloop/mainloop_pubsub.c b/src/core/mainloop/mainloop_pubsub.c index aac607126..724a3115c 100644 --- a/src/core/mainloop/mainloop_pubsub.c +++ b/src/core/mainloop/mainloop_pubsub.c @@ -123,6 +123,9 @@ alertfn_immediate(dispatch_t *d, channel_id_t chan, void *arg) /** * Set the strategy to be used for delivering messages on the named channel. + * + * This function needs to be called once globally for each channel, to + * set up how messages are delivered. **/ int tor_mainloop_set_delivery_strategy(const char *msg_channel_name, diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h index 12cfa7d39..11c7c63ec 100644 --- a/src/lib/pubsub/pubsub_macros.h +++ b/src/lib/pubsub/pubsub_macros.h @@ -165,6 +165,10 @@ /* Macro to declare common elements shared by DECLARE_MESSAGE and * DECLARE_MESSAGE_INT. Don't call this directly. + * + * Note that the "msg_arg_name" string constant is defined in each + * translation unit. This might be undesirable; we can tweak it in the + * future if need be. */ #define DECLARE_MESSAGE_COMMON__(messagename, typename, c_type) \ typedef c_type msg_arg_type__ ##messagename; \ @@ -179,6 +183,8 @@ * C identifier. * * "typename" is a unique identifier for the type of the auxiliary data. + * It needs to be defined somewhere in Tor, using + * "DISPATCH_DEFINE_TYPE." * * "c_type" is a C pointer type (like "char *" or "struct foo *"). */ @@ -203,7 +209,8 @@ * "messagename" is a unique identifier for the message; it must be a valid * C identifier. * - * "typename" is a unique identifier for the type of the auxiliary data. + * "typename" is a unique identifier for the type of the auxiliary data. It + * needs to be defined somewhere in Tor, using "DISPATCH_DEFINE_TYPE." * * "c_type" is a C integer type, like "int" or "bool". It needs to fit inside * a uint64_t. @@ -231,6 +238,8 @@ * * Before you use this, you need to include the header where DECLARE_MESSAGE*() * was used for this message. + * + * You can only use this once per message in each subsystem. */ #define DECLARE_PUBLISH(messagename) \ static pub_binding_t pub_binding__ ##messagename; \ @@ -256,6 +265,8 @@ * "hookfn". The type of this function will be: * static void hookfn(const msg_t *, const c_type) * where c_type is the c type that you declared in the header. + * + * You can only use this once per message in each subsystem. */ #define DECLARE_SUBSCRIBE(messagename, hookfn) \ static void hookfn(const msg_t *, \ @@ -341,7 +352,8 @@ DISPATCH_ADD_SUB_(connector, channel, messagename, DISP_FLAG_EXCL) /** - * Publish a given message with a given argument. + * Publish a given message with a given argument. (Takes ownership of the + * argument if it is a pointer.) */ #define PUBLISH(messagename, arg) \ publish_fn__ ##messagename(arg)

1 0

[tor/master] Rename DISPATCH_DEFINE_TYPE() to DISPATCH_REGISTER_TYPE()
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 3f0bfe1d29e571f1450286085e62246b01509b3b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 14 15:39:14 2019 -0400 Rename DISPATCH_DEFINE_TYPE() to DISPATCH_REGISTER_TYPE() Also fix a grammar error in a comment. --- src/lib/pubsub/pubsub_build.c | 12 ++++++------ src/lib/pubsub/pubsub_connect.h | 10 +++++----- src/lib/pubsub/pubsub_macros.h | 12 ++++++------ src/test/test_pubsub_build.c | 6 +++--- src/test/test_pubsub_msg.c | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/lib/pubsub/pubsub_build.c b/src/lib/pubsub/pubsub_build.c index 1dc20f51f..e44b7d76e 100644 --- a/src/lib/pubsub/pubsub_build.c +++ b/src/lib/pubsub/pubsub_build.c @@ -208,16 +208,16 @@ pubsub_add_sub_(pubsub_connector_t *con, } /** - * Use con to define a the functions to use for manipulating the type + * Use con to define the functions to use for manipulating the type * type. Any function pointers left as NULL will be implemented as * no-ops. **/ int -pubsub_connector_define_type_(pubsub_connector_t *con, - msg_type_id_t type, - dispatch_typefns_t *fns, - const char *file, - unsigned line) +pubsub_connector_register_type_(pubsub_connector_t *con, + msg_type_id_t type, + dispatch_typefns_t *fns, + const char *file, + unsigned line) { pubsub_type_cfg_t *cfg = tor_malloc_zero(sizeof(*cfg)); cfg->type = type; diff --git a/src/lib/pubsub/pubsub_connect.h b/src/lib/pubsub/pubsub_connect.h index e4d860005..bdcb33d2f 100644 --- a/src/lib/pubsub/pubsub_connect.h +++ b/src/lib/pubsub/pubsub_connect.h @@ -45,10 +45,10 @@ int pubsub_add_sub_(struct pubsub_connector_t *con, const char *file, unsigned line); -int pubsub_connector_define_type_(struct pubsub_connector_t *, - msg_type_id_t, - dispatch_typefns_t *, - const char *file, - unsigned line); +int pubsub_connector_register_type_(struct pubsub_connector_t *, + msg_type_id_t, + dispatch_typefns_t *, + const char *file, + unsigned line); #endif diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h index 11c7c63ec..f0429dbde 100644 --- a/src/lib/pubsub/pubsub_macros.h +++ b/src/lib/pubsub/pubsub_macros.h @@ -71,7 +71,7 @@ * } * * Where did these types come from? Somewhere in the code, you need to call - * DISPATCH_DEFINE_TYPE() to make sure that the dispatcher can manage the + * DISPATCH_REGISTER_TYPE() to make sure that the dispatcher can manage the * message auxiliary data. It associates a vtbl-like structure with the * type name, so that the dispatcher knows how to manipulate the type you're * giving it. @@ -100,7 +100,7 @@ * .fmt_fn = boolean_fmt, * .free_fn = boolean_free, * }; - * DISPATCH_DEFINE_TYPE(con, boolean, &boolean_fns); + * DISPATCH_REGISTER_TYPE(con, boolean, &boolean_fns); * } * * @@ -184,7 +184,7 @@ * * "typename" is a unique identifier for the type of the auxiliary data. * It needs to be defined somewhere in Tor, using - * "DISPATCH_DEFINE_TYPE." + * "DISPATCH_REGISTER_TYPE." * * "c_type" is a C pointer type (like "char *" or "struct foo *"). */ @@ -210,7 +210,7 @@ * C identifier. * * "typename" is a unique identifier for the type of the auxiliary data. It - * needs to be defined somewhere in Tor, using "DISPATCH_DEFINE_TYPE." + * needs to be defined somewhere in Tor, using "DISPATCH_REGISTER_TYPE." * * "c_type" is a C integer type, like "int" or "bool". It needs to fit inside * a uint64_t. @@ -362,8 +362,8 @@ * Use a given connector to declare that the functions to be used to manipuate * a certain C type. **/ -#define DISPATCH_DEFINE_TYPE(con, type, fns) \ - pubsub_connector_define_type_((con), \ +#define DISPATCH_REGISTER_TYPE(con, type, fns) \ + pubsub_connector_register_type_((con), \ get_msg_type_id(#type), \ (fns), \ __FILE__, \ diff --git a/src/test/test_pubsub_build.c b/src/test/test_pubsub_build.c index f6c6b81a2..9ffe428c0 100644 --- a/src/test/test_pubsub_build.c +++ b/src/test/test_pubsub_build.c @@ -86,7 +86,7 @@ seed_dispatch_builder(pubsub_builder_t *b, { c = pubsub_connector_for_subsystem(b, get_subsys_id("sys1")); - DISPATCH_DEFINE_TYPE(c, int, &intfns); + DISPATCH_REGISTER_TYPE(c, int, &intfns); if (fl1 != FLAG_SKIP) DISPATCH_ADD_PUB_(c, main, bunch_of_coconuts, fl1); if (fl2 != FLAG_SKIP) @@ -96,7 +96,7 @@ seed_dispatch_builder(pubsub_builder_t *b, { c = pubsub_connector_for_subsystem(b, get_subsys_id("sys2")); - DISPATCH_DEFINE_TYPE(c, string, &stringfns); + DISPATCH_REGISTER_TYPE(c, string, &stringfns); if (fl3 != FLAG_SKIP) DISPATCH_ADD_PUB_(c, main, yes_we_have_no, fl3); if (fl4 != FLAG_SKIP) @@ -178,7 +178,7 @@ test_pubsub_build_types_decls_conflict(void *arg) { c = pubsub_connector_for_subsystem(b, get_subsys_id("sys3")); // Extra declaration of int: we don't allow this. - DISPATCH_DEFINE_TYPE(c, int, &stringfns); + DISPATCH_REGISTER_TYPE(c, int, &stringfns); pubsub_connector_free(c); } diff --git a/src/test/test_pubsub_msg.c b/src/test/test_pubsub_msg.c index 41a8a25ad..73c7c9f54 100644 --- a/src/test/test_pubsub_msg.c +++ b/src/test/test_pubsub_msg.c @@ -103,7 +103,7 @@ setup_dispatcher(const struct testcase_t *testcase) { con = pubsub_connector_for_subsystem(builder, get_subsys_id("types")); - pubsub_connector_define_type_(con, + pubsub_connector_register_type_(con, get_msg_type_id("string"), &stringfns, "nowhere.c", 99);

1 0