[tor-commits] [tor/master] Pubsub: macros for ease-of-use and typesafety.

asn at torproject.org asn at torproject.org
Wed Mar 27 12:31:31 UTC 2019


commit 24df14eb096e73438d6045ff3b2840499a9af9b5
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Jan 14 11:29:21 2019 -0500

    Pubsub: macros for ease-of-use and typesafety.
---
 src/lib/pubsub/include.am      |   1 +
 src/lib/pubsub/pubsub.h        |   1 +
 src/lib/pubsub/pubsub_macros.h | 350 ++++++++++++++++++++++++
 src/test/include.am            |   1 +
 src/test/test.c                |   1 +
 src/test/test.h                |   1 +
 src/test/test_pubsub_build.c   | 596 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 951 insertions(+)

diff --git a/src/lib/pubsub/include.am b/src/lib/pubsub/include.am
index 0ab2fd7b3..c0ec13d03 100644
--- a/src/lib/pubsub/include.am
+++ b/src/lib/pubsub/include.am
@@ -22,4 +22,5 @@ noinst_HEADERS +=					\
 	src/lib/pubsub/pubsub_builder_st.h		\
 	src/lib/pubsub/pubsub_connect.h			\
 	src/lib/pubsub/pubsub_flags.h			\
+	src/lib/pubsub/pubsub_macros.h			\
 	src/lib/pubsub/pubsub_publish.h
diff --git a/src/lib/pubsub/pubsub.h b/src/lib/pubsub/pubsub.h
index 303b36ad5..1c51f7a78 100644
--- a/src/lib/pubsub/pubsub.h
+++ b/src/lib/pubsub/pubsub.h
@@ -80,6 +80,7 @@
 #include "lib/pubsub/pub_binding_st.h"
 #include "lib/pubsub/pubsub_connect.h"
 #include "lib/pubsub/pubsub_flags.h"
+#include "lib/pubsub/pubsub_macros.h"
 #include "lib/pubsub/pubsub_publish.h"
 
 #endif
diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h
new file mode 100644
index 000000000..aed9c5128
--- /dev/null
+++ b/src/lib/pubsub/pubsub_macros.h
@@ -0,0 +1,350 @@
+/* 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 msg.h
+ * \brief Macros to help with the publish/subscribe dispatch API.
+ *
+ * The dispatch API allows different subsystems of Tor to communicate with
+ * another asynchronously via a shared "message" system.  Some subsystems
+ * declare that they publish a given message, and others declare that they
+ * subscribe to it.  Both subsystems depend on the message, but not upon one
+ * another.
+ *
+ * To declare a message, use DECLARE_MESSAGE() (for messages that take their
+ * data as a pointer) or DECLARE_MESSAGE_INT() (for messages that take their
+ * data as an integer.  For example, you might say
+ *
+ *     DECLARE_MESSAGE(new_circuit, circ, circuit_handle_t *);
+ * or
+ *     DECLARE_MESSAGE_INT(shutdown_requested, boolean, bool);
+ *
+ * Every message has a unique name, a "type name" that the dispatch system
+ * uses to manage associated data, and a C type name.  You'll want to put
+ * these declarations in a header, to be included by all publishers and all
+ * subscribers.
+ *
+ * When a subsystem wants to publish a message, it uses DECLARE_PUBLISH() at
+ * file scope to create necessary static functions.  Then, in its subsystem
+ * initialization (in the "bind to dispatcher" callback) (TODO: name this
+ * properly!), it calls DISPATCH_ADD_PUB() to tell the dispatcher about its
+ * intent to publish.  When it actually wants to publish, it uses the
+ * PUBLISH() macro.  For example:
+ *
+ *     // At file scope
+ *     DECLARE_PUBLISH(shutdown_requested);
+ *
+ *     static void bind_to_dispatcher(pubsub_connector_t *con)
+ *     {
+ *         DISPATCH_ADD_PUB(con, mainchannel, shutdown_requested);
+ *     }
+ *
+ *     // somewhere in a function
+ *        {
+ *            PUBLISH(shutdown_requested, true);
+ *        }
+ *
+ * When a subsystem wants to subscribe to a message, it uses
+ * DECLARE_SUBSCRIBE() at file scope to declare static functions.  It must
+ * declare a hook function that receives the message type.  Then, in its "bind
+ * to dispatcher" function, it calls DISPATCHER_ADD_SUB() to tell the
+ * dispatcher about its intent to subscribe.  When another module publishes
+ * the message, the dispatcher will call the provided hook function.
+ *
+ *     // At file scope.  The first argument is the message that you're
+ *     // subscribing to; the second argument is the hook function to declare.
+ *     DECLARE_SUBSCRIBE(shutdown_requested, on_shutdown_req_cb);
+ *
+ *     // You need to declare this function.
+ *     static void on_shutdown_req_cb(const msg_t *msg,
+ *                                    bool value)
+ *     {
+ *         // (do something here.)
+ *     }
+ *
+ *     static void bind_to_dispatcher(pubsub_connector_t *con)
+ *     {
+ *         DISPATCH_ADD_SUB(con, mainchannel, shutdown_requested);
+ *     }
+ *
+ * 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
+ * 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.
+ *
+ * For example, the "boolean" type we're using above could be defined as:
+ *
+ *    static char *boolean_fmt(msg_aux_data_t d)
+ *    {
+ *        // This is used for debugging and dumping messages.
+ *        if (d.u64)
+ *            return tor_strdup("true");
+ *        else
+ *            return tor_strdup("false");
+ *    }
+ *
+ *    static void boolean_free(msg_aux_data_t d)
+ *    {
+ *        // We don't actually need to do anything to free a boolean.
+ *        // We could use "NULL" instead of this function, but I'm including
+ *        // it as an example.
+ *    }
+ *
+ *    static void bind_to_dispatcher(pubsub_connector_t *con)
+ *    {
+ *        dispatch_typefns_t boolean_fns = {
+ *            .fmt_fn = boolean_fmt,
+ *            .free_fn = boolean_free,
+ *        };
+ *        DISPATCH_DEFINE_TYPE(con, boolean, &boolean_fns);
+ *    }
+ *
+ *
+ *
+ * So, how does this all work?  (You can stop reading here, unless you're
+ * debugging something.)
+ *
+ * When you declare a message in a header with DECLARE_MESSAGE() or
+ * DECLARE_MESSAGE_INT(), it creates five things:
+ *
+ *    * two typedefs for the message argument (constant and non-constant
+ *      variants).
+ *    * a constant string to hold the declared message type name
+ *    * two inline functions, to coerce the message argument type to and from
+ *      a "msg_aux_data_t" union.
+ *
+ * All of these declarations have names based on the message name.
+ *
+ * Later, when you say DECLARE_PUBLISH() or DECLARE_SUBSCRIBE(), we use the
+ * elements defined by DECLARE_MESSAGE() to make sure that the publish
+ * function takes the correct argument type, and that the subscription hook is
+ * declared with the right argument type.
+ **/
+
+#ifndef TOR_DISPATCH_MSG_H
+#define TOR_DISPATCH_MSG_H
+
+#include "lib/cc/compat_compiler.h"
+#include "lib/dispatch/dispatch_naming.h"
+#include "lib/pubsub/pub_binding_st.h"
+#include "lib/pubsub/pubsub_connect.h"
+#include "lib/pubsub/pubsub_flags.h"
+#include "lib/pubsub/pubsub_publish.h"
+
+/* Implemenation notes:
+ *
+ * For a messagename "foo", the DECLARE_MESSAGE*() macros must declare:
+ *
+ * msg_arg_type__foo -- a typedef for the argument type of the foo message.
+ * msg_arg_consttype__foo -- a typedef for the const argument type of the
+ *     foo message.
+ * msg_arg_name__foo[] -- a static string constant holding the unique
+ *      identifier for the type of the foo message.
+ * msg_arg_get__foo() -- an inline function taking a msg_aux_data_t and
+ *      returning the C data type.
+ * msg_arg_set__foo() -- an inline function taking a msg_aux_data_t and
+ *      the C type, setting the msg_aux_data_t to hold the C type.
+ *
+ * For a messagename "foo", the DECLARE_PUBLISH() macro must declare:
+ *
+ * pub_binding__foo -- A static pub_binding_t object used to send messages
+ *      from this module.
+ * publish_fn__foo -- A function taking an argument of the appropriate
+ *      C type, to be invoked by PUBLISH().
+ *
+ * For a messagename "foo", the DECLARE_SUBSCRIBE() macro must declare:
+ *
+ * hookfn -- A user-provided function name, with the correct signature.
+ * recv_fn__foo -- A wrapper callback that takes a msg_t *, and calls
+ *      hookfn with the appropriate arguments.
+ */
+
+/* Macro to declare common elements shared by DECLARE_MESSAGE and
+ * DECLARE_MESSAGE_INT.  Don't call this directly.
+ */
+#define DECLARE_MESSAGE_COMMON__(messagename, typename, c_type)         \
+  typedef c_type msg_arg_type__ ##messagename;                          \
+  typedef const c_type msg_arg_consttype__ ##messagename;               \
+  ATTR_UNUSED static const char msg_arg_name__ ##messagename[] = # typename;
+
+/**
+ * Use this macro in a header to declare the existence of a given message,
+ * taking a pointer as auxiliary data.
+ *
+ * "messagename" is a unique identifier for the message.
+ *
+ * "typename" is a unique identifier for the type of the auxiliary data.
+ *
+ * "c_type" is a C pointer type (like "char *" or "struct foo *").
+ */
+#define DECLARE_MESSAGE(messagename, typename, c_type)                  \
+  DECLARE_MESSAGE_COMMON__(messagename, typename, c_type)               \
+  ATTR_UNUSED static inline c_type                                      \
+  msg_arg_get__ ##messagename(msg_aux_data_t m)                         \
+  {                                                                     \
+    return m.ptr;                                                       \
+  }                                                                     \
+  ATTR_UNUSED static inline void                                        \
+  msg_arg_set__ ##messagename(msg_aux_data_t *m, c_type v)              \
+  {                                                                     \
+    m->ptr = v;                                                         \
+  }                                                                     \
+  EAT_SEMICOLON
+
+/**
+ * Use this macro in a header to declare the existence of a given message,
+ * taking an integer as auxiliary data.
+ *
+ * "messagename" is a unique identifier for the message.
+ *
+ * "typename" is a unique identifier for the type of the auxiliary data.
+ *
+ * "c_type" is a C integer type, like "int" or "bool".  It needs to fit inside
+ * a uint64_t.
+ */
+#define DECLARE_MESSAGE_INT(messagename, typename, c_type)              \
+  DECLARE_MESSAGE_COMMON__(messagename, typename, c_type)               \
+  ATTR_UNUSED static inline c_type                                      \
+  msg_arg_get__ ##messagename(msg_aux_data_t m)                         \
+  {                                                                     \
+    return (c_type)m.u64;                                               \
+  }                                                                     \
+  ATTR_UNUSED static inline void                                        \
+  msg_arg_set__ ##messagename(msg_aux_data_t *m, c_type v)              \
+  {                                                                     \
+    m->u64 = (uint64_t)v;                                               \
+  }                                                                     \
+  EAT_SEMICOLON
+
+/**
+ * Use this macro inside a C module declare that we'll be publishing a given
+ * message type from within this module.
+ *
+ * It creates necessary functions and wrappers to publish a message whose
+ * unique identifier is "messagename".
+ *
+ * Before you use this, you need to include the header where DECLARE_MESSAGE*()
+ * was used for this message.
+ */
+#define DECLARE_PUBLISH(messagename)                                    \
+  static pub_binding_t pub_binding__ ##messagename;                     \
+  static void                                                           \
+  publish_fn__ ##messagename(msg_arg_type__ ##messagename arg)          \
+  {                                                                     \
+    msg_aux_data_t data;                                                \
+    msg_arg_set__ ##messagename(&data, arg);                            \
+    pubsub_pub_(&pub_binding__ ##messagename, data);                    \
+  }                                                                     \
+  EAT_SEMICOLON
+
+/**
+ * Use this macro inside a C file to declare that we're subscribing to a
+ * given message and associating it with a given "hook function".  It
+ * declares the hook function static, and helps with strong typing.
+ *
+ * Before you use this, you need to include the header where
+ * DECLARE_MESSAGE*() was used for the message whose unique identifier is
+ * "messagename".
+ *
+ * You will need to define a function with the name that you provide for
+ * "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.
+ */
+#define DECLARE_SUBSCRIBE(messagename, hookfn) \
+  static void hookfn(const msg_t *,                             \
+                     const msg_arg_consttype__ ##messagename);  \
+  static void recv_fn__ ## messagename(const msg_t *m)          \
+  {                                                             \
+    msg_arg_type__ ## messagename arg;                          \
+    arg = msg_arg_get__ ##messagename(m->aux_data__);           \
+    hookfn(m, arg);                                             \
+  }                                                             \
+  EAT_SEMICOLON
+
+/*
+ * This macro is for internal use.  It backs DISPATCH_ADD_PUB*()
+ */
+#define DISPATCH_ADD_PUB_(connector, channel, messagename, flags)       \
+  (                                                                     \
+    ((void)publish_fn__ ##messagename),                                 \
+    pubsub_add_pub_((connector),                                        \
+                      &pub_binding__ ##messagename,                     \
+                      get_channel_id(# channel),                        \
+                    get_message_id(# messagename),                      \
+                      get_msg_type_id(msg_arg_name__ ## messagename),   \
+                      (flags),                                          \
+                      __FILE__,                                         \
+                      __LINE__)                                         \
+    )
+
+/**
+ * Use a given connector and channel name to declare that this subsystem will
+ * publish a given message type.
+ *
+ * Call this macro from within the add_subscriptions() function of a module.
+ */
+#define DISPATCH_ADD_PUB(connector, channel, messagename)       \
+    DISPATCH_ADD_PUB_(connector, channel, messagename, 0)
+
+/**
+ * Use a given connector and channel name to declare that this subsystem will
+ * publish a given message type, and that no other subsystem is allowed to.
+ *
+ * Call this macro from within the add_subscriptions() function of a module.
+ */
+#define DISPATCH_ADD_PUB_EXCL(connector, channel, messagename)  \
+    DISPATCH_ADD_PUB_(connector, channel, messagename, DISP_FLAG_EXCL)
+
+/*
+ * This macro is for internal use. It backs DISPATCH_ADD_SUB*()
+ */
+#define DISPATCH_ADD_SUB_(connector, channel, messagename, flags)       \
+  pubsub_add_sub_((connector),                                          \
+                    recv_fn__ ##messagename,                            \
+                    get_channel_id(#channel),                           \
+                    get_message_id(# messagename),                      \
+                    get_msg_type_id(msg_arg_name__ ##messagename),      \
+                    (flags),                                            \
+                    __FILE__,                                           \
+                    __LINE__)
+/*
+ * Use a given connector and channel name to declare that this subsystem will
+ * receive a given message type.
+ *
+ * Call this macro from within the add_subscriptions() function of a module.
+ */
+#define DISPATCH_ADD_SUB(connector, channel, messagename)       \
+    DISPATCH_ADD_SUB_(connector, channel, messagename, 0)
+/**
+ * Use a given connector and channel name to declare that this subsystem will
+ * receive a given message type, and that no other subsystem is allowed to do
+ * so.
+ *
+ * Call this macro from within the add_subscriptions() function of a module.
+ */
+#define DISPATCH_ADD_SUB_EXCL(connector, channel, messagename)  \
+    DISPATCH_ADD_SUB_(connector, channel, messagename, DISP_FLAG_EXCL)
+
+/**
+ * Publish a given message with a given argument.
+ */
+#define PUBLISH(messagename, arg)               \
+  publish_fn__ ##messagename(arg)
+
+/**
+ * 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),                          \
+                                  get_msg_type_id(#type),       \
+                                  (fns),                        \
+                                  __FILE__,                     \
+                                  __LINE__)
+
+#endif
diff --git a/src/test/include.am b/src/test/include.am
index 7734b73de..2f3bcd59c 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -165,6 +165,7 @@ src_test_test_SOURCES += \
 	src/test/test_proto_misc.c \
 	src/test/test_protover.c \
 	src/test/test_pt.c \
+	src/test/test_pubsub_build.c \
 	src/test/test_pubsub_msg.c \
 	src/test/test_relay.c \
 	src/test/test_relaycell.c \
diff --git a/src/test/test.c b/src/test/test.c
index da0be4133..2309da208 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -910,6 +910,7 @@ struct testgroup_t testgroups[] = {
   { "proto/misc/", proto_misc_tests },
   { "protover/", protover_tests },
   { "pt/", pt_tests },
+  { "pubsub/build/", pubsub_build_tests },
   { "pubsub/msg/", pubsub_msg_tests },
   { "relay/" , relay_tests },
   { "relaycell/", relaycell_tests },
diff --git a/src/test/test.h b/src/test/test.h
index fb417124c..96b22a9a0 100644
--- a/src/test/test.h
+++ b/src/test/test.h
@@ -253,6 +253,7 @@ extern struct testcase_t proto_http_tests[];
 extern struct testcase_t proto_misc_tests[];
 extern struct testcase_t protover_tests[];
 extern struct testcase_t pt_tests[];
+extern struct testcase_t pubsub_build_tests[];
 extern struct testcase_t pubsub_msg_tests[];
 extern struct testcase_t relay_tests[];
 extern struct testcase_t relaycell_tests[];
diff --git a/src/test/test_pubsub_build.c b/src/test/test_pubsub_build.c
new file mode 100644
index 000000000..86b5f763a
--- /dev/null
+++ b/src/test/test_pubsub_build.c
@@ -0,0 +1,596 @@
+/* Copyright (c) 2018, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#define DISPATCH_PRIVATE
+
+#include "test/test.h"
+
+#include "lib/cc/torint.h"
+#include "lib/dispatch/dispatch.h"
+#include "lib/dispatch/dispatch_naming.h"
+#include "lib/dispatch/dispatch_st.h"
+#include "lib/dispatch/msgtypes.h"
+#include "lib/pubsub/pubsub_macros.h"
+#include "lib/pubsub/pubsub_build.h"
+#include "lib/pubsub/pubsub_builder_st.h"
+
+#include "lib/log/escape.h"
+#include "lib/malloc/malloc.h"
+#include "lib/string/printf.h"
+
+#include "test/log_test_helpers.h"
+
+#include <stdio.h>
+#include <string.h>
+
+static char *
+ex_int_fmt(msg_aux_data_t aux)
+{
+  int val = (int) aux.u64;
+  char *r=NULL;
+  tor_asprintf(&r, "%d", val);
+  return r;
+}
+
+static char *
+ex_str_fmt(msg_aux_data_t aux)
+{
+  return esc_for_log(aux.ptr);
+}
+
+static void
+ex_str_free(msg_aux_data_t aux)
+{
+  tor_free_(aux.ptr);
+}
+
+static dispatch_typefns_t intfns = {
+  .fmt_fn = ex_int_fmt
+};
+
+static dispatch_typefns_t stringfns = {
+  .free_fn = ex_str_free,
+  .fmt_fn = ex_str_fmt
+};
+
+DECLARE_MESSAGE_INT(bunch_of_coconuts, int, int);
+DECLARE_PUBLISH(bunch_of_coconuts);
+DECLARE_SUBSCRIBE(bunch_of_coconuts, coconut_recipient_cb);
+
+DECLARE_MESSAGE(yes_we_have_no, string, char *);
+DECLARE_PUBLISH(yes_we_have_no);
+DECLARE_SUBSCRIBE(yes_we_have_no, absent_item_cb);
+
+static void
+coconut_recipient_cb(const msg_t *m, int n_coconuts)
+{
+  (void)m;
+  (void)n_coconuts;
+}
+
+static void
+absent_item_cb(const msg_t *m, const char *fruitname)
+{
+  (void)m;
+  (void)fruitname;
+}
+
+#define FLAG_SKIP 99999
+
+static void
+seed_dispatch_builder(pubsub_builder_t *b,
+                      unsigned fl1, unsigned fl2, unsigned fl3, unsigned fl4)
+{
+  pubsub_connector_t *c = NULL;
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys1"));
+    DISPATCH_DEFINE_TYPE(c, int, &intfns);
+    if (fl1 != FLAG_SKIP)
+      DISPATCH_ADD_PUB_(c, main, bunch_of_coconuts, fl1);
+    if (fl2 != FLAG_SKIP)
+      DISPATCH_ADD_SUB_(c, main, yes_we_have_no, fl2);
+    pubsub_connector_free(c);
+  }
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys2"));
+    DISPATCH_DEFINE_TYPE(c, string, &stringfns);
+    if (fl3 != FLAG_SKIP)
+      DISPATCH_ADD_PUB_(c, main, yes_we_have_no, fl3);
+    if (fl4 != FLAG_SKIP)
+      DISPATCH_ADD_SUB_(c, main, bunch_of_coconuts, fl4);
+    pubsub_connector_free(c);
+  }
+}
+
+static void
+seed_pubsub_builder_basic(pubsub_builder_t *b)
+{
+  seed_dispatch_builder(b, 0, 0, 0, 0);
+}
+
+/* Regular builder with valid types and messages.
+ */
+static void
+test_pubsub_build_types_ok(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+
+  tt_int_op(dispatcher->n_types, OP_GE, 2);
+  tt_assert(dispatcher->typefns);
+
+  tt_assert(dispatcher->typefns[get_msg_type_id("int")].fmt_fn == ex_int_fmt);
+  tt_assert(dispatcher->typefns[get_msg_type_id("string")].fmt_fn ==
+            ex_str_fmt);
+
+ done:
+  pubsub_connector_free(c);
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+}
+
+/* We fail if the same type is defined in two places with different functions.
+ */
+static void
+test_pubsub_build_types_decls_conflict(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+  {
+    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);
+    pubsub_connector_free(c);
+  }
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+  // expect_log_msg_containing("(int) declared twice"); // XXXX
+
+ done:
+  pubsub_connector_free(c);
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* If a message ID exists but nobody is publishing or subscribing to it,
+ * that's okay. */
+static void
+test_pubsub_build_unused_message(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+
+  // This message isn't actually generated by anyone, but that will be fine:
+  // we just log it at info.
+  get_message_id("unused");
+  setup_capture_of_logs(LOG_INFO);
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+  expect_log_msg_containing(
+     "Nobody is publishing or subscribing to message");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* Publishing or subscribing to a message with no subscribers / publishers
+ * should fail and warn. */
+static void
+test_pubsub_build_missing_pubsub(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+
+  b = pubsub_builder_new();
+  seed_dispatch_builder(b, 0, 0, FLAG_SKIP, FLAG_SKIP);
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+
+  expect_log_msg_containing(
+       "Message 0 (bunch_of_coconuts) has publishers, but no subscribers.");
+  expect_log_msg_containing(
+       "Message 1 (yes_we_have_no) has subscribers, but no publishers.");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* Make sure that a stub publisher or subscriber prevents an error from
+ * happening even if there are no other publishers/subscribers for a message
+ */
+static void
+test_pubsub_build_stub_pubsub(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+
+  b = pubsub_builder_new();
+  seed_dispatch_builder(b, 0, 0, DISP_FLAG_STUB, DISP_FLAG_STUB);
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+
+  // 1 subscriber.
+  tt_int_op(1, OP_EQ,
+            dispatcher->table[get_message_id("yes_we_have_no")]->n_enabled);
+  // no subscribers
+  tt_ptr_op(NULL, OP_EQ,
+            dispatcher->table[get_message_id("bunch_of_coconuts")]);
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+}
+
+/* Only one channel per msg id. */
+static void
+test_pubsub_build_channels_conflict(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+  pub_binding_t btmp;
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("problems"));
+    /* Usually the DISPATCH_ADD_PUB macro would keep us from using
+     * the wrong channel */
+    pubsub_add_pub_(c, &btmp, get_channel_id("hithere"),
+                    get_message_id("bunch_of_coconuts"),
+                    get_msg_type_id("int"),
+                    0 /* flags */,
+                    "somewhere.c", 22);
+    pubsub_connector_free(c);
+  };
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+
+  expect_log_msg_containing("Message 0 (bunch_of_coconuts) is associated "
+                            "with multiple inconsistent channels.");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* Only one type per msg id. */
+static void
+test_pubsub_build_types_conflict(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+  pub_binding_t btmp;
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("problems"));
+    /* Usually the DISPATCH_ADD_PUB macro would keep us from using
+     * the wrong channel */
+    pubsub_add_pub_(c, &btmp, get_channel_id("hithere"),
+                    get_message_id("bunch_of_coconuts"),
+                    get_msg_type_id("string"),
+                    0 /* flags */,
+                    "somewhere.c", 22);
+    pubsub_connector_free(c);
+  };
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+
+  expect_log_msg_containing("Message 0 (bunch_of_coconuts) is associated "
+                            "with multiple inconsistent message types.");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* The same module can't publish and subscribe the same message */
+static void
+test_pubsub_build_pubsub_same(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys1"));
+    // already publishing this.
+    DISPATCH_ADD_SUB(c, main, bunch_of_coconuts);
+    pubsub_connector_free(c);
+  };
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  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)");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* More than one subsystem may publish or subscribe, and that's okay. */
+static void
+test_pubsub_build_pubsub_multi(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+  pub_binding_t btmp;
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys3"));
+    DISPATCH_ADD_SUB(c, main, bunch_of_coconuts);
+    pubsub_add_pub_(c, &btmp, get_channel_id("main"),
+                    get_message_id("yes_we_have_no"),
+                    get_msg_type_id("string"),
+                    0 /* flags */,
+                    "somewhere.c", 22);
+    pubsub_connector_free(c);
+  };
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+
+  // 1 subscribers
+  tt_int_op(1, OP_EQ,
+            dispatcher->table[get_message_id("yes_we_have_no")]->n_enabled);
+  // 2 subscribers.
+  dtbl_entry_t *ent =
+    dispatcher->table[get_message_id("bunch_of_coconuts")];
+  tt_int_op(2, OP_EQ, ent->n_enabled);
+  tt_int_op(2, OP_EQ, ent->n_fns);
+  tt_ptr_op(ent->rcv[0].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+  tt_ptr_op(ent->rcv[1].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+}
+
+static void
+some_other_coconut_hook(const msg_t *m)
+{
+  (void)m;
+}
+
+/* Subscribe hooks should be build correctly when there are a bunch of
+ * them. */
+static void
+test_pubsub_build_sub_many(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+  char *sysname = NULL;
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+
+  int i;
+  for (i = 1; i < 100; ++i) {
+    tor_asprintf(&sysname, "system%d",i);
+    c = pubsub_connector_for_subsystem(b, get_subsys_id(sysname));
+    if (i % 7) {
+      DISPATCH_ADD_SUB(c, main, bunch_of_coconuts);
+    } else {
+      pubsub_add_sub_(c, some_other_coconut_hook,
+                      get_channel_id("main"),
+                      get_message_id("bunch_of_coconuts"),
+                      get_msg_type_id("int"),
+                      0 /* flags */,
+                      "somewhere.c", 22);
+    }
+    pubsub_connector_free(c);
+    tor_free(sysname);
+  };
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+
+  dtbl_entry_t *ent =
+    dispatcher->table[get_message_id("bunch_of_coconuts")];
+  tt_int_op(100, OP_EQ, ent->n_enabled);
+  tt_int_op(100, OP_EQ, ent->n_fns);
+  tt_ptr_op(ent->rcv[0].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+  tt_ptr_op(ent->rcv[1].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+  tt_ptr_op(ent->rcv[76].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+  tt_ptr_op(ent->rcv[77].fn, OP_EQ, some_other_coconut_hook);
+  tt_ptr_op(ent->rcv[78].fn, OP_EQ, recv_fn__bunch_of_coconuts);
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  tor_free(sysname);
+}
+
+/* The same subsystem can only declare one publish or subscribe. */
+static void
+test_pubsub_build_pubsub_redundant(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_pubsub_builder_basic(b);
+  pub_binding_t btmp;
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys2"));
+    DISPATCH_ADD_SUB(c, main, bunch_of_coconuts);
+    pubsub_add_pub_(c, &btmp, get_channel_id("main"),
+                    get_message_id("yes_we_have_no"),
+                    get_msg_type_id("string"),
+                    0 /* flags */,
+                    "somewhere.c", 22);
+    pubsub_connector_free(c);
+  };
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+
+  expect_log_msg_containing(
+    "is configured to be published by subsystem 1 (sys2) more than once");
+  expect_log_msg_containing(
+    "is configured to be subscribed by subsystem 1 (sys2) more than once");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+/* It's fine to declare the excl flag. */
+static void
+test_pubsub_build_excl_ok(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+
+  b = pubsub_builder_new();
+  // Try one excl/excl pair and one excl/non pair.
+  seed_dispatch_builder(b, DISP_FLAG_EXCL, 0,
+                        DISP_FLAG_EXCL, DISP_FLAG_EXCL);
+
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher);
+
+  // 1 subscribers
+  tt_int_op(1, OP_EQ,
+            dispatcher->table[get_message_id("yes_we_have_no")]->n_enabled);
+  // 1 subscriber.
+  tt_int_op(1, OP_EQ,
+            dispatcher->table[get_message_id("bunch_of_coconuts")]->n_enabled);
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+}
+
+/* but if you declare the excl flag, you need to mean it. */
+static void
+test_pubsub_build_excl_bad(void *arg)
+{
+  (void)arg;
+  pubsub_builder_t *b = NULL;
+  dispatch_t *dispatcher = NULL;
+  pubsub_connector_t *c = NULL;
+
+  b = pubsub_builder_new();
+  seed_dispatch_builder(b, DISP_FLAG_EXCL, DISP_FLAG_EXCL,
+                        0, 0);
+
+  {
+    c = pubsub_connector_for_subsystem(b, get_subsys_id("sys3"));
+    DISPATCH_ADD_PUB_(c, main, bunch_of_coconuts, 0);
+    DISPATCH_ADD_SUB_(c, main, yes_we_have_no, 0);
+    pubsub_connector_free(c);
+  };
+
+  setup_full_capture_of_logs(LOG_WARN);
+  dispatcher = pubsub_builder_finalize(b, NULL);
+  b = NULL;
+  tt_assert(dispatcher == NULL);
+
+  expect_log_msg_containing("has multiple publishers, but at least one is "
+                            "marked as exclusive.");
+  expect_log_msg_containing("has multiple subscribers, but at least one is "
+                            "marked as exclusive.");
+
+ done:
+  pubsub_builder_free(b);
+  dispatch_free(dispatcher);
+  teardown_capture_of_logs();
+}
+
+#define T(name, flags)                                          \
+  { #name, test_pubsub_build_ ## name , (flags), NULL, NULL }
+
+struct testcase_t pubsub_build_tests[] = {
+  T(types_ok, TT_FORK),
+  T(types_decls_conflict, TT_FORK),
+  T(unused_message, TT_FORK),
+  T(missing_pubsub, TT_FORK),
+  T(stub_pubsub, TT_FORK),
+  T(channels_conflict, TT_FORK),
+  T(types_conflict, TT_FORK),
+  T(pubsub_same, TT_FORK),
+  T(pubsub_multi, TT_FORK),
+  T(sub_many, TT_FORK),
+  T(pubsub_redundant, TT_FORK),
+  T(excl_ok, TT_FORK),
+  T(excl_bad, TT_FORK),
+  END_OF_TESTCASES
+};





More information about the tor-commits mailing list