[tor/master] Add a "freeze" function for config_mgr_t objects. - tor-commits

4 Sep 2019

commit dde091ebc76d8ae5d93c1b3e1febfa2c9d1459fa
Author: Nick Mathewson nickm@torproject.org
Date:   Tue Jul 23 09:08:31 2019 -0400
Add a "freeze" function for config_mgr_t objects.
It's important to make sure that we don't change a config_mgr_t
    after we start using it to make objects, or we could get into
    inconsistent states.  This feature is the start of a safety
    mechanism to prevent this problem.
---
 src/app/config/config.c                   |  1 +
 src/app/config/confparse.c                | 20 ++++++++++++++++++++
 src/app/config/confparse.h                |  1 +
 src/app/config/statefile.c                |  1 +
 src/feature/dirauth/shared_random_state.c |  1 +
 src/test/test_confparse.c                 | 15 +++++++++++++++
 6 files changed, 39 insertions(+)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 246f52a64..d8b33aee7 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -904,6 +904,7 @@ get_options_mgr(void)
 {
   if (PREDICT_UNLIKELY(options_mgr == NULL)) {
     options_mgr = config_mgr_new(&options_format);
+    config_mgr_freeze(options_mgr);
   }
   return options_mgr;
 }
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 9d4226307..b086f8c94 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -85,6 +85,10 @@ struct config_mgr_t {
   smartlist_t *all_abbrevs;
   /** A smartlist of config_deprecation_t for all configuration formats. */
   smartlist_t *all_deprecations;
+  /** True if this manager has been frozen and cannot have any more formats
+   * added to it. A manager must be frozen before it can be used to construct
+   * or manipulate objects. */
+  bool frozen;
 };
#define IDX_TOPLEVEL (-1)
@@ -112,6 +116,10 @@ config_mgr_register_fmt(config_mgr_t *mgr,
 {
   int i;
+  tor_assertf(!mgr->frozen,
+              "Tried to add a format to a configuration manager after "
+              "it had been frozen.");
+
   /* register variables */
   for (i = 0; fmt->vars[i].member.name; ++i) {
     managed_var_t *mv = tor_malloc_zero(sizeof(managed_var_t));
@@ -161,6 +169,16 @@ config_mgr_get_obj(const config_mgr_t *mgr, const void *toplevel, int idx)
   return config_mgr_get_obj_mutable(mgr, (void*)toplevel, idx);
 }
+/**
+ * Mark a configuration manager as "frozen", so that no more formats can be
+ * added, and so that it can be used for manipulating configuration objects.
+ **/
+void
+config_mgr_freeze(config_mgr_t *mgr)
+{
+  mgr->frozen = true;
+}
+
 /** Release all storage held in <b>mgr</b> */
 void
 config_mgr_free_(config_mgr_t *mgr)
@@ -208,6 +226,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr,
 {
   tor_assert(mgr);
   tor_assert(options);
+  tor_assert(mgr->frozen);
   struct_check_magic(options, &mgr->toplevel->magic);
 }
@@ -221,6 +240,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr,
 void *
 config_new(const config_mgr_t *mgr)
 {
+  tor_assert(mgr->frozen);
   const config_format_t *fmt = mgr->toplevel;
   void *opts = tor_malloc_zero(fmt->size);
   struct_set_magic(opts, &fmt->magic);
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index edc7fa377..83e886696 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -73,6 +73,7 @@ typedef struct config_mgr_t config_mgr_t;
config_mgr_t *config_mgr_new(const config_format_t *toplevel_fmt);
 void config_mgr_free_(config_mgr_t *mgr);
+void config_mgr_freeze(config_mgr_t *mgr);
 #define config_mgr_free(mgr) \
   FREE_AND_NULL(config_mgr_t, config_mgr_free_, (mgr))
 struct smartlist_t *config_mgr_list_vars(const config_mgr_t *mgr);
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index a44bcf6fb..93e35659d 100644
--- a/src/app/config/statefile.c
+++ b/src/app/config/statefile.c
@@ -183,6 +183,7 @@ get_state_mgr(void)
 {
   if (PREDICT_UNLIKELY(state_mgr == NULL)) {
     state_mgr = config_mgr_new(&state_format);
+    config_mgr_freeze(state_mgr);
   }
   return state_mgr;
 }
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c
index a552e621c..12ae79ce3 100644
--- a/src/feature/dirauth/shared_random_state.c
+++ b/src/feature/dirauth/shared_random_state.c
@@ -112,6 +112,7 @@ get_srs_mgr(void)
 {
   if (PREDICT_UNLIKELY(shared_random_state_mgr == NULL)) {
     shared_random_state_mgr = config_mgr_new(&state_format);
+    config_mgr_freeze(shared_random_state_mgr);
   }
   return shared_random_state_mgr;
 }
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index 8a921531f..716384e2f 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -153,6 +153,7 @@ test_confparse_init(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = config_new(mgr);
   config_init(mgr, tst);
@@ -239,6 +240,7 @@ test_confparse_assign_simple(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
tt_str_op(tst->s, OP_EQ, "this is a");
@@ -300,6 +302,7 @@ test_confparse_assign_obsolete(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -330,6 +333,7 @@ test_confparse_assign_deprecated(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -364,6 +368,7 @@ test_confparse_assign_replaced(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -395,6 +400,7 @@ test_confparse_assign_emptystring(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -423,6 +429,7 @@ test_confparse_assign_twice(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -458,6 +465,7 @@ test_confparse_assign_badval(void *arg)
 {
   const badval_test_t *bt = arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -512,6 +520,7 @@ test_confparse_dump(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   char *dumped = NULL;
@@ -603,6 +612,7 @@ test_confparse_reset(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
config_reset_line(mgr, tst, "interval", 0);
@@ -623,6 +633,7 @@ test_confparse_reassign(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL, *rs = NULL;
@@ -679,6 +690,7 @@ test_confparse_reassign_extend(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
@@ -738,6 +750,7 @@ test_confparse_get_assigned(void *arg)
   (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = get_simple_config(mgr);
   config_line_t *lines = NULL;
@@ -824,6 +837,7 @@ test_confparse_extra_lines(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&etest_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = config_new(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL, *dump = NULL;
@@ -919,6 +933,7 @@ test_confparse_check_ok_fail(void *arg)
 {
   (void)arg;
   config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
   test_struct_t *tst = config_new(mgr);
   tst->pos = -10;
   tt_assert(! config_check_ok(mgr, tst, LOG_INFO));

    