commit 203e9138d1f714e5ad42884f75d50cd31c3bae3b Author: Nick Mathewson <nickm@torproject.org> Date: Tue Mar 26 19:56:39 2019 -0400 Remove message/subsystem numbers from log messages Having the numbers in those messages makes some of the unit test unstable, by causing them to depend on the initialization order of the naming objects. --- src/lib/pubsub/pubsub_check.c | 46 +++++++++++++++++++++---------------------- src/test/test_pubsub_build.c | 18 +++++++++-------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/lib/pubsub/pubsub_check.c b/src/lib/pubsub/pubsub_check.c index 94a55beeb..d308dc58a 100644 --- a/src/lib/pubsub/pubsub_check.c +++ b/src/lib/pubsub/pubsub_check.c @@ -190,10 +190,10 @@ get_message_bitarray(const pubsub_adjmap_t *map, 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)); + "Message \"%s\" is configured to be %s by subsystem " + "\"%s\" more than once.", + get_message_id_name(msg), operation, + get_subsys_id_name(cfg->subsys)); ok = false; } bitarray_set(*out, cfg->subsys); @@ -234,10 +234,10 @@ lint_message_graph(const pubsub_adjmap_t *map, 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)); + "Message \"%s\" is published and subscribed by the same " + "subsystem \"%s\".", + get_message_id_name(msg), + get_subsys_id_name(i)); ok = false; } } @@ -287,16 +287,16 @@ lint_message_consistency(message_id_t msg, if (! chan_same) { log_warn(LD_MESG|LD_BUG, - "Message %u (%s) is associated with multiple inconsistent " + "Message \"%s\" is associated with multiple inconsistent " "channels.", - msg, get_message_id_name(msg)); + get_message_id_name(msg)); ok = false; } if (! type_same) { log_warn(LD_MESG|LD_BUG, - "Message %u (%s) is associated with multiple inconsistent " + "Message \"%s\" is associated with multiple inconsistent " "message types.", - msg, get_message_id_name(msg)); + get_message_id_name(msg)); ok = false; } @@ -305,16 +305,16 @@ lint_message_consistency(message_id_t msg, */ if (pub_excl && smartlist_len(pub) > 1) { log_warn(LD_MESG|LD_BUG, - "Message %u (%s) has multiple publishers, but at least one is " + "Message \"%s\" has multiple publishers, but at least one is " "marked as exclusive.", - msg, get_message_id_name(msg)); + get_message_id_name(msg)); ok = false; } if (sub_excl && smartlist_len(sub) > 1) { log_warn(LD_MESG|LD_BUG, - "Message %u (%s) has multiple subscribers, but at least one is " + "Message \"%s\" has multiple subscribers, but at least one is " "marked as exclusive.", - msg, get_message_id_name(msg)); + get_message_id_name(msg)); ok = false; } @@ -344,9 +344,9 @@ lint_message(const pubsub_adjmap_t *map, message_id_t msg) 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)); + log_info(LD_MESG, "Nobody is publishing or subscribing to message " + "\"%s\".", + 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. */ @@ -355,13 +355,13 @@ lint_message(const pubsub_adjmap_t *map, message_id_t msg) /* 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)); + "Message \"%s\" has subscribers, but no publishers.", + 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)); + "Message \"%s\" has publishers, but no subscribers.", + get_message_id_name(msg)); ok = false; } diff --git a/src/test/test_pubsub_build.c b/src/test/test_pubsub_build.c index 9ffe428c0..ce5bf6008 100644 --- a/src/test/test_pubsub_build.c +++ b/src/test/test_pubsub_build.c @@ -242,9 +242,9 @@ test_pubsub_build_missing_pubsub(void *arg) tt_assert(dispatcher == NULL); expect_log_msg_containing( - "Message 0 (bunch_of_coconuts) has publishers, but no subscribers."); + "Message \"bunch_of_coconuts\" has publishers, but no subscribers."); expect_log_msg_containing( - "Message 1 (yes_we_have_no) has subscribers, but no publishers."); + "Message \"yes_we_have_no\" has subscribers, but no publishers."); done: pubsub_builder_free(b); @@ -311,7 +311,7 @@ test_pubsub_build_channels_conflict(void *arg) b = NULL; tt_assert(dispatcher == NULL); - expect_log_msg_containing("Message 0 (bunch_of_coconuts) is associated " + expect_log_msg_containing("Message \"bunch_of_coconuts\" is associated " "with multiple inconsistent channels."); done: @@ -350,7 +350,7 @@ test_pubsub_build_types_conflict(void *arg) b = NULL; tt_assert(dispatcher == NULL); - expect_log_msg_containing("Message 0 (bunch_of_coconuts) is associated " + expect_log_msg_containing("Message \"bunch_of_coconuts\" is associated " "with multiple inconsistent message types."); done: @@ -383,8 +383,8 @@ test_pubsub_build_pubsub_same(void *arg) b = NULL; tt_assert(dispatcher == NULL); - expect_log_msg_containing("Message 0 (bunch_of_coconuts) is published " - "and subscribed by the same subsystem 0 (sys1)"); + expect_log_msg_containing("Message \"bunch_of_coconuts\" is published " + "and subscribed by the same subsystem \"sys1\"."); done: pubsub_builder_free(b); @@ -523,9 +523,11 @@ test_pubsub_build_pubsub_redundant(void *arg) tt_assert(dispatcher == NULL); expect_log_msg_containing( - "is configured to be published by subsystem 1 (sys2) more than once"); + "Message \"yes_we_have_no\" is configured to be published by " + "subsystem \"sys2\" more than once."); expect_log_msg_containing( - "is configured to be subscribed by subsystem 1 (sys2) more than once"); + "Message \"bunch_of_coconuts\" is configured to be subscribed by " + "subsystem \"sys2\" more than once."); done: pubsub_builder_free(b);