Merge remote-tracking branch 'teor/ticket15431-event-mask-tests'

changes/ticket15431

@@ -0,0 +1,6 @@
+  o Minor features (testing):
+    - Add unit tests for control_event_is_interesting().
+      Add a compile-time check that the number of events doesn't exceed
+      the capacity of control_event_t.event_mask.
+      Closes ticket 15431, checks for bugs similar to 13085.
+      Patch by "teor".

src/or/control.c

@@ -72,7 +72,7 @@ static int disable_log_messages = 0;
 /** Macro: true if any control connection is interested in events of type
  * <b>e</b>. */
 #define EVENT_IS_INTERESTING(e) \
-  (!! (global_event_mask & (((uint64_t)1)<<(e))))
+  (!! (global_event_mask & EVENT_MASK_(e)))
 
 /** If we're using cookie-type authentication, how long should our cookies be?
  */

src/or/control.h

@@ -123,6 +123,7 @@ void control_free_all(void);
  * because it is used both as a list of v0 event types, and as indices
  * into the bitfield to determine which controllers want which events.
  */
+/* This bitfield has no event zero    0x0000 */
 #define EVENT_MIN_                    0x0001
 #define EVENT_CIRCUIT_STATUS          0x0001
 #define EVENT_STREAM_STATUS           0x0002
@@ -158,9 +159,29 @@ void control_free_all(void);
 #define EVENT_TRANSPORT_LAUNCHED      0x0020
 #define EVENT_HS_DESC                 0x0021
 #define EVENT_MAX_                    0x0021
-/* If EVENT_MAX_ ever hits 0x003F, we need to make the mask into a
+
+/* sizeof(control_connection_t.event_mask) in bits, currently a uint64_t */
+#define EVENT_CAPACITY_               0x0040
+
+/* If EVENT_MAX_ ever hits 0x0040, we need to make the mask into a
  * different structure, as it can only handle a maximum left shift of 1<<63. */
 
+#if EVENT_MAX_ >= EVENT_CAPACITY_
+#error control_connection_t.event_mask has an event greater than its capacity
+#endif
+
+#define EVENT_MASK_(e)               (((uint64_t)1)<<(e))
+
+#define EVENT_MASK_NONE_             ((uint64_t)0x0)
+
+#define EVENT_MASK_ABOVE_MIN_        ((~((uint64_t)0x0)) << EVENT_MIN_)
+#define EVENT_MASK_BELOW_MAX_        ((~((uint64_t)0x0)) \
+                                      >> (EVENT_CAPACITY_ - EVENT_MAX_ \
+                                          - EVENT_MIN_))
+
+#define EVENT_MASK_ALL_              (EVENT_MASK_ABOVE_MIN_ \
+                                      & EVENT_MASK_BELOW_MAX_)
+
 /* Used only by control.c and test.c */
 STATIC size_t write_escaped_data(const char *data, size_t len, char **out);
 STATIC size_t read_escaped_data(const char *data, size_t len, char **out);

src/test/test_controller_events.c

@@ -293,6 +293,104 @@ test_cntev_format_cell_stats(void *arg)
   tor_free(n_chan);
 }
 
+static void
+test_cntev_event_mask(void *arg)
+{
+  unsigned int test_event, selected_event;
+  (void)arg;
+
+  /* Check that nothing is interesting when no events are set */
+  control_testing_set_global_event_mask(EVENT_MASK_NONE_);
+
+  /* Check that nothing is interesting between EVENT_MIN_ and EVENT_MAX_ */
+  for (test_event = EVENT_MIN_; test_event <= EVENT_MAX_; test_event++)
+    tt_assert(!control_event_is_interesting(test_event));
+
+  /* Check that nothing is interesting outside EVENT_MIN_ to EVENT_MAX_
+   * This will break if control_event_is_interesting() checks its arguments */
+  for (test_event = 0; test_event < EVENT_MIN_; test_event++)
+    tt_assert(!control_event_is_interesting(test_event));
+  for (test_event = EVENT_MAX_ + 1;
+       test_event < EVENT_CAPACITY_;
+       test_event++)
+    tt_assert(!control_event_is_interesting(test_event));
+
+  /* Check that all valid events are interesting when all events are set */
+  control_testing_set_global_event_mask(EVENT_MASK_ALL_);
+
+  /* Check that everything is interesting between EVENT_MIN_ and EVENT_MAX_ */
+  for (test_event = EVENT_MIN_; test_event <= EVENT_MAX_; test_event++)
+    tt_assert(control_event_is_interesting(test_event));
+
+  /* Check that nothing is interesting outside EVENT_MIN_ to EVENT_MAX_
+   * This will break if control_event_is_interesting() checks its arguments */
+  for (test_event = 0; test_event < EVENT_MIN_; test_event++)
+    tt_assert(!control_event_is_interesting(test_event));
+  for (test_event = EVENT_MAX_ + 1;
+       test_event < EVENT_CAPACITY_;
+       test_event++)
+    tt_assert(!control_event_is_interesting(test_event));
+
+  /* Check that only that event is interesting when a single event is set */
+  for (selected_event = EVENT_MIN_;
+       selected_event <= EVENT_MAX_;
+       selected_event++) {
+    control_testing_set_global_event_mask(EVENT_MASK_(selected_event));
+
+    /* Check that only this event is interesting
+     * between EVENT_MIN_ and EVENT_MAX_ */
+    for (test_event = EVENT_MIN_; test_event <= EVENT_MAX_; test_event++) {
+      if (test_event == selected_event) {
+        tt_assert(control_event_is_interesting(test_event));
+      } else {
+        tt_assert(!control_event_is_interesting(test_event));
+      }
+    }
+
+    /* Check that nothing is interesting outside EVENT_MIN_ to EVENT_MAX_
+     * This will break if control_event_is_interesting checks its arguments */
+    for (test_event = 0; test_event < EVENT_MIN_; test_event++)
+      tt_assert(!control_event_is_interesting(test_event));
+    for (test_event = EVENT_MAX_ + 1;
+         test_event < EVENT_CAPACITY_;
+         test_event++)
+      tt_assert(!control_event_is_interesting(test_event));
+  }
+
+  /* Check that only that event is not-interesting
+   * when a single event is un-set */
+  for (selected_event = EVENT_MIN_;
+       selected_event <= EVENT_MAX_;
+       selected_event++) {
+    control_testing_set_global_event_mask(
+                                          EVENT_MASK_ALL_
+                                          & ~(EVENT_MASK_(selected_event))
+                                          );
+
+    /* Check that only this event is not-interesting
+     * between EVENT_MIN_ and EVENT_MAX_ */
+    for (test_event = EVENT_MIN_; test_event <= EVENT_MAX_; test_event++) {
+      if (test_event == selected_event) {
+        tt_assert(!control_event_is_interesting(test_event));
+      } else {
+        tt_assert(control_event_is_interesting(test_event));
+      }
+    }
+
+    /* Check that nothing is interesting outside EVENT_MIN_ to EVENT_MAX_
+     * This will break if control_event_is_interesting checks its arguments */
+    for (test_event = 0; test_event < EVENT_MIN_; test_event++)
+      tt_assert(!control_event_is_interesting(test_event));
+    for (test_event = EVENT_MAX_ + 1;
+         test_event < EVENT_CAPACITY_;
+         test_event++)
+      tt_assert(!control_event_is_interesting(test_event));
+  }
+
+ done:
+  ;
+}
+
 #define TEST(name, flags)                                               \
   { #name, test_cntev_ ## name, flags, 0, NULL }
 
@@ -302,6 +400,7 @@ struct testcase_t controller_event_tests[] = {
   TEST(sum_up_cell_stats, 0),
   TEST(append_cell_stats, 0),
   TEST(format_cell_stats, 0),
+  TEST(event_mask, 0),
   END_OF_TESTCASES
 };