- tor-commits - lists.torproject.org

[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

[tor/master] Merge branch 'tor-github/pr/859'
by asn＠torproject.org 27 Mar '19

27 Mar '19

1 0