Move the responsibility for listing periodic events to periodic.c

The end goal here is to move the periodic callback to their
respective modules, so that mainloop.c doesn't have to include so
many other things.

This patch doesn't actually move any of the callbacks out of
mainloop.c yet.

src/core/mainloop/mainloop.c

@@ -1387,7 +1387,7 @@ CALLBACK(second_elapsed);
   PERIODIC_EVENT(name, PERIODIC_EVENT_ROLE_ ## r, f)
 #define FL(name) (PERIODIC_EVENT_FLAG_ ## name)
 
-STATIC periodic_event_item_t periodic_events[] = {
+STATIC periodic_event_item_t mainloop_periodic_events[] = {
 
   /* Everyone needs to run these. They need to have very long timeouts for
    * that to be safe. */
@@ -1485,24 +1485,7 @@ static periodic_event_item_t *prune_old_routers_event=NULL;
 void
 reset_all_main_loop_timers(void)
 {
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_reschedule(&periodic_events[i]);
-  }
-}
-
-/** Return the member of periodic_events[] whose name is <b>name</b>.
- * Return NULL if no such event is found.
- */
-static periodic_event_item_t *
-find_periodic_event(const char *name)
-{
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    if (strcmp(name, periodic_events[i].name) == 0)
-      return &periodic_events[i];
-  }
-  return NULL;
+  periodic_events_reset_all();
 }
 
 /** Return a bitmask of the roles this tor instance is configured for using
@@ -1565,8 +1548,8 @@ initialize_periodic_events_cb(evutil_socket_t fd, short events, void *data)
   rescan_periodic_events(get_options());
 }
 
-/** Set up all the members of periodic_events[], and configure them all to be
- * launched from a callback. */
+/** Set up all the members of mainloop_periodic_events[], and configure them
+ * all to be launched from a callback. */
 STATIC void
 initialize_periodic_events(void)
 {
@@ -1575,14 +1558,15 @@ initialize_periodic_events(void)
 
   periodic_events_initialized = 1;
 
-  /* Set up all periodic events. We'll launch them by roles. */
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_setup(&periodic_events[i]);
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_events_add(&mainloop_periodic_events[i]);
   }
 
+  /* Set up all periodic events. We'll launch them by roles. */
+  periodic_events_setup_all();
+
 #define NAMED_CALLBACK(name) \
-  STMT_BEGIN name ## _event = find_periodic_event( #name ); STMT_END
+  STMT_BEGIN name ## _event = periodic_events_find( #name ); STMT_END
 
   NAMED_CALLBACK(check_descriptor);
   NAMED_CALLBACK(prune_old_routers);
@@ -1602,10 +1586,7 @@ initialize_periodic_events(void)
 STATIC void
 teardown_periodic_events(void)
 {
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_destroy(&periodic_events[i]);
-  }
+  periodic_events_destroy_all();
   periodic_events_initialized = 0;
 }
 
@@ -1647,33 +1628,7 @@ rescan_periodic_events(const or_options_t *options)
     return;
   }
 
-  int roles = get_my_roles(options);
-
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
-
-    int enable = !!(item->roles & roles);
-
-    /* Handle the event flags. */
-    if (net_is_disabled() &&
-        (item->flags & PERIODIC_EVENT_FLAG_NEED_NET)) {
-      enable = 0;
-    }
-
-    /* Enable the event if needed. It is safe to enable an event that was
-     * already enabled. Same goes for disabling it. */
-    if (enable) {
-      log_debug(LD_GENERAL, "Launching periodic event %s", item->name);
-      periodic_event_enable(item);
-    } else {
-      log_debug(LD_GENERAL, "Disabling periodic event %s", item->name);
-      if (item->flags & PERIODIC_EVENT_FLAG_RUN_ON_DISABLE) {
-        periodic_event_schedule_and_disable(item);
-      } else {
-        periodic_event_disable(item);
-      }
-    }
-  }
+  periodic_events_rescan_by_roles(get_my_roles(options), net_is_disabled());
 }
 
 /* We just got new options globally set, see if we need to enabled or disable

src/core/mainloop/mainloop.h

@@ -113,7 +113,7 @@ extern smartlist_t *connection_array;
 
 /* We need the periodic_event_item_t definition. */
 #include "core/mainloop/periodic.h"
-extern periodic_event_item_t periodic_events[];
+extern periodic_event_item_t mainloop_periodic_events[];
 #endif
 #endif /* defined(MAIN_PRIVATE) */
 

src/core/mainloop/periodic.c

@@ -6,9 +6,17 @@
  *
  * \brief Generic backend for handling periodic events.
  *
- * The events in this module are used by main.c to track items that need
+ * The events in this module are used to track items that need
  * to fire once every N seconds, possibly picking a new interval each time
- * that they fire.  See periodic_events[] in main.c for examples.
+ * that they fire.  See periodic_events[] in mainloop.c for examples.
+ *
+ * This module manages a global list of periodic_event_item_t objects,
+ * each corresponding to a single event.  To register an event, pass it to
+ * periodic_events_add() when initializing your subsystem.
+ *
+ * We expect that periodic_event_item_t objects will be statically allocated;
+ * we set them up and tear them down here, but we don't take ownership of
+ * them.
  */
 
 #include "core/or/or.h"
@@ -24,6 +32,11 @@
  */
 static const int MAX_INTERVAL = 10 * 365 * 86400;
 
+/**
+ *
+ **/
+static smartlist_t *the_periodic_events = NULL;
+
 /** Set the event <b>event</b> to run in <b>next_interval</b> seconds from
  * now. */
 static void
@@ -184,3 +197,123 @@ periodic_event_schedule_and_disable(periodic_event_item_t *event)
 
   mainloop_event_activate(event->ev);
 }
+
+/**
+ * Add <b>item</b> to the list of periodic events.
+ *
+ * Note that <b>item</b> should be statically allocated: we do not
+ * take ownership of it.
+ **/
+void
+periodic_events_add(periodic_event_item_t *item)
+{
+  if (!the_periodic_events)
+    the_periodic_events = smartlist_new();
+
+  if (BUG(smartlist_contains(the_periodic_events, item)))
+    return;
+
+  smartlist_add(the_periodic_events, item);
+}
+
+/** Set up all not-previously setup periodic events. */
+void
+periodic_events_setup_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (item->ev)
+      continue;
+    periodic_event_setup(item);
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/** Reset all the registered periodic events so we'll do all our actions again
+ * as if we just started up.
+ *
+ * Useful if our clock just moved back a long time from the future,
+ * so we don't wait until that future arrives again before acting.
+ */
+void
+periodic_events_reset_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    periodic_event_reschedule(item);
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/**
+ * Return the registered periodic event whose name is <b>name</b>.
+ * Return NULL if no such event is found.
+ */
+periodic_event_item_t *
+periodic_events_find(const char *name)
+{
+  if (! the_periodic_events)
+    return NULL;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (strcmp(name, item->name) == 0)
+      return item;
+  } SMARTLIST_FOREACH_END(item);
+  return NULL;
+}
+
+/**
+ * Start or stop registered periodic events, depending on our current set of
+ * roles.
+ *
+ * Invoked when our list of roles, or the net_disabled flag has changed.
+ **/
+void
+periodic_events_rescan_by_roles(int roles, bool net_disabled)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    int enable = !!(item->roles & roles);
+
+    /* Handle the event flags. */
+    if (net_disabled &&
+        (item->flags & PERIODIC_EVENT_FLAG_NEED_NET)) {
+      enable = 0;
+    }
+
+    /* Enable the event if needed. It is safe to enable an event that was
+     * already enabled. Same goes for disabling it. */
+    if (enable) {
+      log_debug(LD_GENERAL, "Launching periodic event %s", item->name);
+      periodic_event_enable(item);
+    } else {
+      log_debug(LD_GENERAL, "Disabling periodic event %s", item->name);
+      if (item->flags & PERIODIC_EVENT_FLAG_RUN_ON_DISABLE) {
+        periodic_event_schedule_and_disable(item);
+      } else {
+        periodic_event_disable(item);
+      }
+    }
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/** Invoked at shutdown: free resources used in this module.
+ *
+ * Does not free the periodic_event_item_t object themselves, because we do
+ * not own them. */
+void
+periodic_events_destroy_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    periodic_event_destroy(item);
+  } SMARTLIST_FOREACH_END(item);
+
+  smartlist_free(the_periodic_events);
+}

src/core/mainloop/periodic.h

@@ -90,4 +90,11 @@ void periodic_event_enable(periodic_event_item_t *event);
 void periodic_event_disable(periodic_event_item_t *event);
 void periodic_event_schedule_and_disable(periodic_event_item_t *event);
 
+void periodic_events_add(periodic_event_item_t *item);
+void periodic_events_setup_all(void);
+void periodic_events_reset_all(void);
+periodic_event_item_t *periodic_events_find(const char *name);
+void periodic_events_rescan_by_roles(int roles, bool net_disabled);
+void periodic_events_destroy_all(void);
+
 #endif /* !defined(TOR_PERIODIC_H) */

src/test/test_periodic_event.c

@@ -55,8 +55,8 @@ test_pe_initialize(void *arg)
   rescan_periodic_events(get_options());
 
   /* Validate that all events have been set up. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     tt_assert(item->ev);
     tt_assert(item->fn);
     tt_u64_op(item->last_action_time, OP_EQ, 0);
@@ -89,8 +89,8 @@ test_pe_launch(void *arg)
   /* Hack: We'll set a dumb fn() of each events so they don't get called when
    * dispatching them. We just want to test the state of the callbacks, not
    * the whole code path. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     item->fn = dumb_event_fn;
   }
 
@@ -116,8 +116,8 @@ test_pe_launch(void *arg)
 
   int mask = PERIODIC_EVENT_ROLE_CLIENT|PERIODIC_EVENT_ROLE_ALL|
     PERIODIC_EVENT_ROLE_NET_PARTICIPANT;
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     int should_be_enabled = !!(item->roles & mask);
     tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled);
     // enabled or not, the event has not yet been run.
@@ -134,8 +134,8 @@ test_pe_launch(void *arg)
              PERIODIC_EVENT_ROLE_RELAY|PERIODIC_EVENT_ROLE_DIRSERVER|
              PERIODIC_EVENT_ROLE_ALL|PERIODIC_EVENT_ROLE_NET_PARTICIPANT);
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     /* Only Client role should be disabled. */
     if (item->roles == PERIODIC_EVENT_ROLE_CLIENT) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
@@ -156,8 +156,8 @@ test_pe_launch(void *arg)
   set_network_participation(false);
   periodic_events_on_new_options(options);
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     int should_be_enabled = (item->roles & PERIODIC_EVENT_ROLE_ALL) &&
       !(item->flags & PERIODIC_EVENT_FLAG_NEED_NET);
     tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled);
@@ -177,8 +177,8 @@ test_pe_launch(void *arg)
    * trigger a rescan of the event disabling the HS service event. */
   to_remove = &service;
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     tt_int_op(periodic_event_is_enabled(item), OP_EQ,
               (item->roles != PERIODIC_EVENT_ROLE_CONTROLEV));
   }
@@ -304,8 +304,8 @@ test_pe_hs_service(void *arg)
   /* Hack: We'll set a dumb fn() of each events so they don't get called when
    * dispatching them. We just want to test the state of the callbacks, not
    * the whole code path. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     item->fn = dumb_event_fn;
   }
 
@@ -318,8 +318,8 @@ test_pe_hs_service(void *arg)
    * trigger a rescan of the event disabling the HS service event. */
   to_remove = &service;
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     if (item->roles & PERIODIC_EVENT_ROLE_HS_SERVICE) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
     }
@@ -329,8 +329,8 @@ test_pe_hs_service(void *arg)
   /* Remove the service from the global map, it should trigger a rescan and
    * disable the HS service events. */
   remove_service(get_hs_service_map(), &service);
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     if (item->roles & PERIODIC_EVENT_ROLE_HS_SERVICE) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
     }