Browse Source

Merge branch 'tor-github/pr/1290'

David Goulet 4 years ago
parent
commit
34f3fcef40

+ 5 - 0
changes/ticket31240

@@ -0,0 +1,5 @@
+  o Minor features (configuration):
+    - The configuration code has been extended to allow splitting
+      configuration data across multiple objects. Previously, all
+      configuration data needed to be kept in a single object, which
+      tended to become bloated.  Closes ticket 31240.

+ 75 - 82
src/app/config/config.c

@@ -843,15 +843,15 @@ static void config_maybe_load_geoip_files_(const or_options_t *options,
 static int options_validate_cb(void *old_options, void *options,
                                void *default_options,
                                int from_setconf, char **msg);
-static void options_free_cb(void *options);
 static void cleanup_protocol_warning_severity_level(void);
 static void set_protocol_warning_severity_level(int warning_severity);
+static void options_clear_cb(const config_mgr_t *mgr, void *opts);
 
 /** Magic value for or_options_t. */
 #define OR_OPTIONS_MAGIC 9090909
 
 /** Configuration format for or_options_t. */
-STATIC const config_format_t options_format = {
+static const config_format_t options_format = {
   sizeof(or_options_t),
   {
    "or_options_t",
@@ -862,8 +862,9 @@ STATIC const config_format_t options_format = {
   option_deprecation_notes_,
   option_vars_,
   options_validate_cb,
-  options_free_cb,
-  NULL
+  options_clear_cb,
+  NULL,
+  offsetof(or_options_t, subconfigs_),
 };
 
 /*
@@ -895,6 +896,20 @@ static int in_option_validation = 0;
 /* True iff we've initialized libevent */
 static int libevent_initialized = 0;
 
+/* A global configuration manager to handle all configuration objects. */
+static config_mgr_t *options_mgr = NULL;
+
+/** Return the global configuration manager object for torrc options. */
+STATIC const config_mgr_t *
+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;
+}
+
 /** Return the contents of our frontpage string, or NULL if not configured. */
 MOCK_IMPL(const char*,
 get_dirportfrontpage, (void))
@@ -951,9 +966,6 @@ get_options_defaults(void)
 int
 set_options(or_options_t *new_val, char **msg)
 {
-  int i;
-  smartlist_t *elements;
-  config_line_t *line;
   or_options_t *old_options = global_options;
   global_options = new_val;
   /* Note that we pass the *old* options below, for comparison. It
@@ -975,35 +987,16 @@ set_options(or_options_t *new_val, char **msg)
   /* Issues a CONF_CHANGED event to notify controller of the change. If Tor is
    * just starting up then the old_options will be undefined. */
   if (old_options && old_options != global_options) {
-    elements = smartlist_new();
-    for (i=0; options_format.vars[i].member.name; ++i) {
-      const config_var_t *var = &options_format.vars[i];
-      const char *var_name = var->member.name;
-      if (config_var_is_contained(var)) {
-        /* something else will check this var, or it doesn't need checking */
-        continue;
-      }
-      if (!config_is_same(&options_format, new_val, old_options, var_name)) {
-        line = config_get_assigned_option(&options_format, new_val,
-                                          var_name, 1);
-
-        if (line) {
-          config_line_t *next;
-          for (; line; line = next) {
-            next = line->next;
-            smartlist_add(elements, line->key);
-            smartlist_add(elements, line->value);
-            tor_free(line);
-          }
-        } else {
-          smartlist_add_strdup(elements, options_format.vars[i].member.name);
-          smartlist_add(elements, NULL);
-        }
-      }
+    smartlist_t *elements = smartlist_new();
+    config_line_t *changes =
+      config_get_changes(get_options_mgr(), old_options, new_val);
+    for (config_line_t *line = changes; line; line = line->next) {
+      smartlist_add(elements, line->key);
+      smartlist_add(elements, line->value);
     }
     control_event_conf_changed(elements);
-    SMARTLIST_FOREACH(elements, char *, cp, tor_free(cp));
     smartlist_free(elements);
+    config_free_lines(changes);
   }
 
   if (old_options != global_options) {
@@ -1019,11 +1012,11 @@ set_options(or_options_t *new_val, char **msg)
 
 /** Release additional memory allocated in options
  */
-STATIC void
-or_options_free_(or_options_t *options)
+static void
+options_clear_cb(const config_mgr_t *mgr, void *opts)
 {
-  if (!options)
-    return;
+  (void)mgr;
+  or_options_t *options = opts;
 
   routerset_free(options->ExcludeExitNodesUnion_);
   if (options->NodeFamilySets) {
@@ -1046,7 +1039,14 @@ or_options_free_(or_options_t *options)
   tor_free(options->command_arg);
   tor_free(options->master_key_fname);
   config_free_lines(options->MyFamily);
-  config_free(&options_format, options);
+}
+
+/** Release all memory allocated in options
+ */
+STATIC void
+or_options_free_(or_options_t *options)
+{
+  config_free(get_options_mgr(), options);
 }
 
 /** Release all memory and resources held by global configuration structures.
@@ -1080,6 +1080,8 @@ config_free_all(void)
 
   have_parsed_cmdline = 0;
   libevent_initialized = 0;
+
+  config_mgr_free(options_mgr);
 }
 
 /** Make <b>address</b> -- a piece of information related to our operation as
@@ -2547,7 +2549,7 @@ config_parse_commandline(int argc, char **argv, int ignore_errors,
 
     param = tor_malloc_zero(sizeof(config_line_t));
     param->key = is_cmdline ? tor_strdup(argv[i]) :
-                   tor_strdup(config_expand_abbrev(&options_format, s, 1, 1));
+                 tor_strdup(config_expand_abbrev(get_options_mgr(), s, 1, 1));
     param->value = arg;
     param->command = command;
     param->next = NULL;
@@ -2573,8 +2575,7 @@ config_parse_commandline(int argc, char **argv, int ignore_errors,
 int
 option_is_recognized(const char *key)
 {
-  const config_var_t *var = config_find_option(&options_format, key);
-  return (var != NULL);
+  return config_find_option_name(get_options_mgr(), key) != NULL;
 }
 
 /** Return the canonical name of a configuration option, or NULL
@@ -2582,8 +2583,7 @@ option_is_recognized(const char *key)
 const char *
 option_get_canonical_name(const char *key)
 {
-  const config_var_t *var = config_find_option(&options_format, key);
-  return var ? var->member.name : NULL;
+  return config_find_option_name(get_options_mgr(), key);
 }
 
 /** Return a canonical list of the options assigned for key.
@@ -2591,7 +2591,7 @@ option_get_canonical_name(const char *key)
 config_line_t *
 option_get_assignment(const or_options_t *options, const char *key)
 {
-  return config_get_assigned_option(&options_format, options, key, 1);
+  return config_get_assigned_option(get_options_mgr(), options, key, 1);
 }
 
 /** Try assigning <b>list</b> to the global options. You do this by duping
@@ -2607,9 +2607,9 @@ setopt_err_t
 options_trial_assign(config_line_t *list, unsigned flags, char **msg)
 {
   int r;
-  or_options_t *trial_options = config_dup(&options_format, get_options());
+  or_options_t *trial_options = config_dup(get_options_mgr(), get_options());
 
-  if ((r=config_assign(&options_format, trial_options,
+  if ((r=config_assign(get_options_mgr(), trial_options,
                        list, flags, msg)) < 0) {
     or_options_free(trial_options);
     return r;
@@ -2664,25 +2664,25 @@ print_usage(void)
 static void
 list_torrc_options(void)
 {
-  int i;
-  for (i = 0; option_vars_[i].member.name; ++i) {
-    const config_var_t *var = &option_vars_[i];
+  smartlist_t *vars = config_mgr_list_vars(get_options_mgr());
+  SMARTLIST_FOREACH_BEGIN(vars, const config_var_t *, var) {
     if (! config_var_is_settable(var)) {
       /* This variable cannot be set, or cannot be set by this name. */
       continue;
     }
     printf("%s\n", var->member.name);
-  }
+  } SMARTLIST_FOREACH_END(var);
+  smartlist_free(vars);
 }
 
 /** Print all deprecated but non-obsolete torrc options. */
 static void
 list_deprecated_options(void)
 {
-  const config_deprecation_t *d;
-  for (d = option_deprecation_notes_; d->name; ++d) {
-    printf("%s\n", d->name);
-  }
+  smartlist_t *deps = config_mgr_list_deprecated_vars(get_options_mgr());
+  SMARTLIST_FOREACH(deps, const char *, name,
+                    printf("%s\n", name));
+  smartlist_free(deps);
 }
 
 /** Print all compile-time modules and their enabled/disabled status. */
@@ -2992,7 +2992,7 @@ is_local_addr, (const tor_addr_t *addr))
 or_options_t *
 options_new(void)
 {
-  return config_new(&options_format);
+  return config_new(get_options_mgr());
 }
 
 /** Set <b>options</b> to hold reasonable defaults for most options.
@@ -3000,10 +3000,10 @@ options_new(void)
 void
 options_init(or_options_t *options)
 {
-  config_init(&options_format, options);
+  config_init(get_options_mgr(), options);
   config_line_t *dflts = get_options_defaults();
   char *msg=NULL;
-  if (config_assign(&options_format, options, dflts,
+  if (config_assign(get_options_mgr(), options, dflts,
                     CAL_WARN_DEPRECATIONS, &msg)<0) {
     log_err(LD_BUG, "Unable to set default options: %s", msg);
     tor_free(msg);
@@ -3040,7 +3040,7 @@ options_dump(const or_options_t *options, int how_to_dump)
       return NULL;
   }
 
-  return config_dump(&options_format, use_defaults, options, minimal, 0);
+  return config_dump(get_options_mgr(), use_defaults, options, minimal, 0);
 }
 
 /** Return 0 if every element of sl is a string holding a decimal
@@ -3172,13 +3172,6 @@ options_validate_cb(void *old_options, void *options, void *default_options,
   return rv;
 }
 
-/** Callback to free an or_options_t */
-static void
-options_free_cb(void *options)
-{
-  or_options_free_(options);
-}
-
 #define REJECT(arg) \
   STMT_BEGIN *msg = tor_strdup(arg); return -1; STMT_END
 #if defined(__GNUC__) && __GNUC__ <= 3
@@ -4435,7 +4428,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
   STMT_BEGIN                                                            \
     if (!options->TestingTorNetwork &&                                  \
         !options->UsingTestNetworkDefaults_ &&                          \
-        !config_is_same(&options_format,options,                        \
+        !config_is_same(get_options_mgr(),options,                        \
                         default_options,#arg)) {                        \
       REJECT(#arg " may only be changed in testing Tor "                \
              "networks!");                                              \
@@ -5411,8 +5404,7 @@ options_init_from_string(const char *cf_defaults, const char *cf,
   oldoptions = global_options; /* get_options unfortunately asserts if
                                   this is the first time we run*/
 
-  newoptions = tor_malloc_zero(sizeof(or_options_t));
-  newoptions->magic_ = OR_OPTIONS_MAGIC;
+  newoptions = options_new();
   options_init(newoptions);
   newoptions->command = command;
   newoptions->command_arg = command_arg ? tor_strdup(command_arg) : NULL;
@@ -5431,7 +5423,7 @@ options_init_from_string(const char *cf_defaults, const char *cf,
       err = SETOPT_ERR_PARSE;
       goto err;
     }
-    retval = config_assign(&options_format, newoptions, cl,
+    retval = config_assign(get_options_mgr(), newoptions, cl,
                            CAL_WARN_DEPRECATIONS, msg);
     config_free_lines(cl);
     if (retval < 0) {
@@ -5439,15 +5431,15 @@ options_init_from_string(const char *cf_defaults, const char *cf,
       goto err;
     }
     if (i==0)
-      newdefaultoptions = config_dup(&options_format, newoptions);
+      newdefaultoptions = config_dup(get_options_mgr(), newoptions);
   }
 
   if (newdefaultoptions == NULL) {
-    newdefaultoptions = config_dup(&options_format, global_default_options);
+    newdefaultoptions = config_dup(get_options_mgr(), global_default_options);
   }
 
   /* Go through command-line variables too */
-  retval = config_assign(&options_format, newoptions,
+  retval = config_assign(get_options_mgr(), newoptions,
                          global_cmdline_options, CAL_WARN_DEPRECATIONS, msg);
   if (retval < 0) {
     err = SETOPT_ERR_PARSE;
@@ -8133,9 +8125,8 @@ getinfo_helper_config(control_connection_t *conn,
   (void) errmsg;
   if (!strcmp(question, "config/names")) {
     smartlist_t *sl = smartlist_new();
-    int i;
-    for (i = 0; option_vars_[i].member.name; ++i) {
-      const config_var_t *var = &option_vars_[i];
+    smartlist_t *vars = config_mgr_list_vars(get_options_mgr());
+    SMARTLIST_FOREACH_BEGIN(vars, const config_var_t *, var) {
       /* don't tell controller about invisible options */
       if (config_var_is_invisible(var))
         continue;
@@ -8143,26 +8134,27 @@ getinfo_helper_config(control_connection_t *conn,
       if (!type)
         continue;
       smartlist_add_asprintf(sl, "%s %s\n",var->member.name,type);
-    }
+    } SMARTLIST_FOREACH_END(var);
     *answer = smartlist_join_strings(sl, "", 0, NULL);
     SMARTLIST_FOREACH(sl, char *, c, tor_free(c));
     smartlist_free(sl);
+    smartlist_free(vars);
   } else if (!strcmp(question, "config/defaults")) {
     smartlist_t *sl = smartlist_new();
     int dirauth_lines_seen = 0, fallback_lines_seen = 0;
-    for (int i = 0; option_vars_[i].member.name; ++i) {
-      const config_var_t *var = &option_vars_[i];
+    smartlist_t *vars = config_mgr_list_vars(get_options_mgr());
+    SMARTLIST_FOREACH_BEGIN(vars, const config_var_t *, var) {
       if (var->initvalue != NULL) {
-        if (strcmp(option_vars_[i].member.name, "DirAuthority") == 0) {
+        if (strcmp(var->member.name, "DirAuthority") == 0) {
           /*
            * Count dirauth lines we have a default for; we'll use the
            * count later to decide whether to add the defaults manually
            */
           ++dirauth_lines_seen;
         }
-        if (strcmp(option_vars_[i].member.name, "FallbackDir") == 0) {
+        if (strcmp(var->member.name, "FallbackDir") == 0) {
           /*
-           * Similarly count fallback lines, so that we can decided later
+           * Similarly count fallback lines, so that we can decide later
            * to add the defaults manually.
            */
           ++fallback_lines_seen;
@@ -8171,7 +8163,8 @@ getinfo_helper_config(control_connection_t *conn,
         smartlist_add_asprintf(sl, "%s %s\n",var->member.name,val);
         tor_free(val);
       }
-    }
+    } SMARTLIST_FOREACH_END(var);
+    smartlist_free(vars);
 
     if (dirauth_lines_seen == 0) {
       /*

+ 2 - 3
src/app/config/config.h

@@ -247,9 +247,8 @@ int options_any_client_port_set(const or_options_t *options);
 #define CL_PORT_DFLT_GROUP_WRITABLE (1u<<7)
 
 STATIC int options_act(const or_options_t *old_options);
-#ifdef TOR_UNIT_TESTS
-extern const struct config_format_t options_format;
-#endif
+struct config_mgr_t;
+STATIC const struct config_mgr_t *get_options_mgr(void);
 
 STATIC port_cfg_t *port_cfg_new(size_t namelen);
 #define port_cfg_free(port) \

File diff suppressed because it is too large
+ 573 - 141
src/app/config/confparse.c


+ 60 - 26
src/app/config/confparse.h

@@ -39,8 +39,19 @@ typedef struct config_deprecation_t {
  * of arguments. */
 typedef int (*validate_fn_t)(void*,void*,void*,int,char**);
 
-/** Callback to free a configuration object. */
-typedef void (*free_cfg_fn_t)(void*);
+struct config_mgr_t;
+
+/**
+ * Callback to clear all non-managed fields of a configuration object.
+ *
+ * <b>obj</b> is the configuration object whose non-managed fields should be
+ * cleared.
+ *
+ * (Regular fields get cleared by config_reset(), but you might have fields
+ * in the object that do not correspond to configuration variables.  If those
+ * fields need to be cleared or freed, this is where to do it.)
+ */
+typedef void (*clear_cfg_fn_t)(const struct config_mgr_t *mgr, void *obj);
 
 /** Information on the keys, value types, key-to-struct-member mappings,
  * variable descriptions, validation functions, and abbreviations for a
@@ -55,51 +66,70 @@ typedef struct config_format_t {
                              * values, and where we stick them in the
                              * structure. */
   validate_fn_t validate_fn; /**< Function to validate config. */
-  free_cfg_fn_t free_fn; /**< Function to free the configuration. */
+  clear_cfg_fn_t clear_fn; /**< Function to clear the configuration. */
   /** If present, extra denotes a LINELIST variable for unrecognized
    * lines.  Otherwise, unrecognized lines are an error. */
   const struct_member_t *extra;
+  /** The position of a config_suite_t pointer within the toplevel object,
+   * or -1 if there is no such pointer. */
+  int config_suite_offset;
 } config_format_t;
 
-/** Macro: assert that <b>cfg</b> has the right magic field for format
- * <b>fmt</b>. */
-#define CONFIG_CHECK(fmt, cfg) STMT_BEGIN                               \
-    tor_assert(fmt);                                                    \
-    struct_check_magic((cfg), &fmt->magic);                             \
-  STMT_END
+/**
+ * A collection of config_format_t objects to describe several objects
+ * that are all configured with the same configuration file.
+ *
+ * (NOTE: for now, this only handles a single config_format_t.)
+ **/
+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);
+int config_mgr_add_format(config_mgr_t *mgr,
+                          const config_format_t *fmt);
+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);
+struct smartlist_t *config_mgr_list_deprecated_vars(const config_mgr_t *mgr);
+
+/** A collection of managed configuration objects. */
+typedef struct config_suite_t config_suite_t;
 
 #define CAL_USE_DEFAULTS      (1u<<0)
 #define CAL_CLEAR_FIRST       (1u<<1)
 #define CAL_WARN_DEPRECATIONS (1u<<2)
 
-void *config_new(const config_format_t *fmt);
-void config_free_(const config_format_t *fmt, void *options);
-#define config_free(fmt, options) do {                \
-    config_free_((fmt), (options));                   \
+void *config_new(const config_mgr_t *fmt);
+void config_free_(const config_mgr_t *fmt, void *options);
+#define config_free(mgr, options) do {                \
+    config_free_((mgr), (options));                   \
     (options) = NULL;                                 \
   } while (0)
 
-struct config_line_t *config_get_assigned_option(const config_format_t *fmt,
+struct config_line_t *config_get_assigned_option(const config_mgr_t *mgr,
                                           const void *options, const char *key,
                                           int escape_val);
-int config_is_same(const config_format_t *fmt,
+int config_is_same(const config_mgr_t *fmt,
                    const void *o1, const void *o2,
                    const char *name);
-void config_init(const config_format_t *fmt, void *options);
-void *config_dup(const config_format_t *fmt, const void *old);
-char *config_dump(const config_format_t *fmt, const void *default_options,
+struct config_line_t *config_get_changes(const config_mgr_t *mgr,
+                                  const void *options1, const void *options2);
+void config_init(const config_mgr_t *mgr, void *options);
+void *config_dup(const config_mgr_t *mgr, const void *old);
+char *config_dump(const config_mgr_t *mgr, const void *default_options,
                   const void *options, int minimal,
                   int comment_defaults);
-bool config_check_ok(const config_format_t *fmt, const void *options,
+bool config_check_ok(const config_mgr_t *mgr, const void *options,
                      int severity);
-int config_assign(const config_format_t *fmt, void *options,
+int config_assign(const config_mgr_t *mgr, void *options,
                   struct config_line_t *list,
                   unsigned flags, char **msg);
-const char *config_find_deprecation(const config_format_t *fmt,
-                                     const char *key);
-const config_var_t *config_find_option(const config_format_t *fmt,
-                                       const char *key);
-const char *config_expand_abbrev(const config_format_t *fmt,
+const char *config_find_deprecation(const config_mgr_t *mgr,
+                                    const char *key);
+const char *config_find_option_name(const config_mgr_t *mgr,
+                                    const char *key);
+const char *config_expand_abbrev(const config_mgr_t *mgr,
                                  const char *option,
                                  int command_line, int warn_obsolete);
 void warn_deprecated_option(const char *what, const char *why);
@@ -119,8 +149,12 @@ bool config_var_is_dumpable(const config_var_t *var);
 #define CFG_EQ_ROUTERSET(a,b,opt) routerset_equal((a)->opt, (b)->opt)
 
 #ifdef CONFPARSE_PRIVATE
-STATIC void config_reset_line(const config_format_t *fmt, void *options,
+STATIC void config_reset_line(const config_mgr_t *mgr, void *options,
                               const char *key, int use_defaults);
+STATIC void *config_mgr_get_obj_mutable(const config_mgr_t *mgr,
+                                        void *toplevel, int idx);
+STATIC const void *config_mgr_get_obj(const config_mgr_t *mgr,
+                                       const void *toplevel, int idx);
 #endif
 
 #endif /* !defined(TOR_CONFPARSE_H) */

+ 9 - 0
src/app/config/or_options_st.h

@@ -18,6 +18,7 @@
 
 struct smartlist_t;
 struct config_line_t;
+struct config_suite_t;
 
 /** Enumeration of outbound address configuration types:
  * Exit-only, OR-only, or both */
@@ -1107,6 +1108,14 @@ struct or_options_t {
    * a possible previous dormant state.
    **/
   int DormantCanceledByStartup;
+
+  /**
+   * Configuration objects for individual modules.
+   *
+   * Never access this field or its members directly: instead, use the module
+   * in question to get its relevant configuration object.
+   */
+  struct config_suite_t *subconfigs_;
 };
 
 #endif /* !defined(TOR_OR_OPTIONS_ST_H) */

+ 9 - 0
src/app/config/or_state_st.h

@@ -15,6 +15,7 @@
 
 #include "lib/cc/torint.h"
 struct smartlist_t;
+struct config_suite_t;
 
 /** Persistent state for an onion router, as saved to disk. */
 struct or_state_t {
@@ -94,6 +95,14 @@ struct or_state_t {
   /** True if we were dormant when we last wrote the file; false if we
    * weren't.  "auto" on initial startup. */
   int Dormant;
+
+  /**
+   * State objects for individual modules.
+   *
+   * Never access this field or its members directly: instead, use the module
+   * in question to get its relevant state object if you must.
+   */
+  struct config_suite_t *substates_;
 };
 
 #endif /* !defined(TOR_OR_STATE_ST_H) */

+ 25 - 18
src/app/config/statefile.c

@@ -145,8 +145,6 @@ static int or_state_validate_cb(void *old_options, void *options,
                                 void *default_options,
                                 int from_setconf, char **msg);
 
-static void or_state_free_cb(void *state);
-
 /** Magic value for or_state_t. */
 #define OR_STATE_MAGIC 0x57A73f57
 
@@ -170,10 +168,25 @@ static const config_format_t state_format = {
   NULL,
   state_vars_,
   or_state_validate_cb,
-  or_state_free_cb,
+  NULL,
   &state_extra_var,
+  offsetof(or_state_t, substates_),
 };
 
+/* A global configuration manager for state-file objects */
+static config_mgr_t *state_mgr = NULL;
+
+/** Return the configuration manager for state-file objects. */
+static const config_mgr_t *
+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;
+}
+
 /** Persistent serialized state. */
 static or_state_t *global_state = NULL;
 
@@ -268,12 +281,6 @@ or_state_validate_cb(void *old_state, void *state, void *default_state,
   return or_state_validate(state, msg);
 }
 
-static void
-or_state_free_cb(void *state)
-{
-  or_state_free_(state);
-}
-
 /** Return 0 if every setting in <b>state</b> is reasonable, and a
  * permissible transition from <b>old_state</b>.  Else warn and return -1.
  * Should have no side effects, except for normalizing the contents of
@@ -298,7 +305,7 @@ or_state_set(or_state_t *new_state)
   char *err = NULL;
   int ret = 0;
   tor_assert(new_state);
-  config_free(&state_format, global_state);
+  config_free(get_state_mgr(), global_state);
   global_state = new_state;
   if (entry_guards_parse_state(global_state, 1, &err)<0) {
     log_warn(LD_GENERAL,"%s",err);
@@ -361,9 +368,8 @@ or_state_save_broken(char *fname)
 STATIC or_state_t *
 or_state_new(void)
 {
-  or_state_t *new_state = tor_malloc_zero(sizeof(or_state_t));
-  new_state->magic_ = OR_STATE_MAGIC;
-  config_init(&state_format, new_state);
+  or_state_t *new_state = config_new(get_state_mgr());
+  config_init(get_state_mgr(), new_state);
 
   return new_state;
 }
@@ -404,7 +410,7 @@ or_state_load(void)
     int assign_retval;
     if (config_get_lines(contents, &lines, 0)<0)
       goto done;
-    assign_retval = config_assign(&state_format, new_state,
+    assign_retval = config_assign(get_state_mgr(), new_state,
                                   lines, 0, &errmsg);
     config_free_lines(lines);
     if (assign_retval<0)
@@ -431,7 +437,7 @@ or_state_load(void)
     or_state_save_broken(fname);
 
     tor_free(contents);
-    config_free(&state_format, new_state);
+    config_free(get_state_mgr(), new_state);
 
     new_state = or_state_new();
   } else if (contents) {
@@ -464,7 +470,7 @@ or_state_load(void)
   tor_free(fname);
   tor_free(contents);
   if (new_state)
-    config_free(&state_format, new_state);
+    config_free(get_state_mgr(), new_state);
 
   return r;
 }
@@ -517,7 +523,7 @@ or_state_save(time_t now)
   tor_free(global_state->TorVersion);
   tor_asprintf(&global_state->TorVersion, "Tor %s", get_version());
 
-  state = config_dump(&state_format, NULL, global_state, 1, 0);
+  state = config_dump(get_state_mgr(), NULL, global_state, 1, 0);
   format_local_iso_time(tbuf, now);
   tor_asprintf(&contents,
                "# Tor state file last generated on %s local time\n"
@@ -727,7 +733,7 @@ or_state_free_(or_state_t *state)
   if (!state)
     return;
 
-  config_free(&state_format, state);
+  config_free(get_state_mgr(), state);
 }
 
 void
@@ -735,4 +741,5 @@ or_state_free_all(void)
 {
   or_state_free(global_state);
   global_state = NULL;
+  config_mgr_free(state_mgr);
 }

+ 26 - 17
src/feature/dirauth/shared_random_state.c

@@ -62,7 +62,6 @@ DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
 static int
 disk_state_validate_cb(void *old_state, void *state, void *default_state,
                        int from_setconf, char **msg);
-static void disk_state_free_cb(void *);
 
 /* Array of variables that are saved to disk as a persistent state. */
 static const config_var_t state_vars[] = {
@@ -99,10 +98,25 @@ static const config_format_t state_format = {
   NULL,
   state_vars,
   disk_state_validate_cb,
-  disk_state_free_cb,
+  NULL,
   &state_extra_var,
+  -1,
 };
 
+/* Global configuration manager for the shared-random state file */
+static config_mgr_t *shared_random_state_mgr = NULL;
+
+/** Return the configuration manager for the shared-random state file. */
+static const config_mgr_t *
+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;
+}
+
 static void state_query_del_(sr_state_object_t obj_type, void *data);
 
 /* Return a string representation of a protocol phase. */
@@ -264,23 +278,22 @@ disk_state_free_(sr_disk_state_t *state)
   if (state == NULL) {
     return;
   }
-  config_free(&state_format, state);
+  config_free(get_srs_mgr(), state);
 }
 
 /* Allocate a new disk state, initialize it and return it. */
 static sr_disk_state_t *
 disk_state_new(time_t now)
 {
-  sr_disk_state_t *new_state = tor_malloc_zero(sizeof(*new_state));
+  sr_disk_state_t *new_state = config_new(get_srs_mgr());
 
-  new_state->magic_ = SR_DISK_STATE_MAGIC;
   new_state->Version = SR_PROTO_VERSION;
   new_state->TorVersion = tor_strdup(get_version());
   new_state->ValidUntil = get_state_valid_until_time(now);
   new_state->ValidAfter = now;
 
   /* Init config format. */
-  config_init(&state_format, new_state);
+  config_init(get_srs_mgr(), new_state);
   return new_state;
 }
 
@@ -348,12 +361,6 @@ disk_state_validate_cb(void *old_state, void *state, void *default_state,
   return 0;
 }
 
-static void
-disk_state_free_cb(void *state)
-{
-  disk_state_free_(state);
-}
-
 /* Parse the Commit line(s) in the disk state and translate them to the
  * the memory state. Return 0 on success else -1 on error. */
 static int
@@ -584,11 +591,12 @@ disk_state_reset(void)
   config_free_lines(sr_disk_state->ExtraLines);
   tor_free(sr_disk_state->TorVersion);
 
-  /* Clean up the struct */
-  memset(sr_disk_state, 0, sizeof(*sr_disk_state));
+  /* Clear other fields. */
+  sr_disk_state->ValidAfter = 0;
+  sr_disk_state->ValidUntil = 0;
+  sr_disk_state->Version = 0;
 
   /* Reset it with useful data */
-  sr_disk_state->magic_ = SR_DISK_STATE_MAGIC;
   sr_disk_state->TorVersion = tor_strdup(get_version());
 }
 
@@ -683,7 +691,7 @@ disk_state_load_from_disk_impl(const char *fname)
     }
 
     disk_state = disk_state_new(time(NULL));
-    config_assign(&state_format, disk_state, lines, 0, &errmsg);
+    config_assign(get_srs_mgr(), disk_state, lines, 0, &errmsg);
     config_free_lines(lines);
     if (errmsg) {
       log_warn(LD_DIR, "SR: Reading state error: %s", errmsg);
@@ -736,7 +744,7 @@ disk_state_save_to_disk(void)
   /* Make sure that our disk state is up to date with our memory state
    * before saving it to disk. */
   disk_state_update();
-  state = config_dump(&state_format, NULL, sr_disk_state, 0, 0);
+  state = config_dump(get_srs_mgr(), NULL, sr_disk_state, 0, 0);
   format_local_iso_time(tbuf, now);
   tor_asprintf(&content,
                "# Tor shared random state file last generated on %s "
@@ -1278,6 +1286,7 @@ sr_state_free_all(void)
   /* Nullify our global state. */
   sr_state = NULL;
   sr_disk_state = NULL;
+  config_mgr_free(shared_random_state_mgr);
 }
 
 /* Save our current state in memory to disk. */

+ 3 - 2
src/test/fuzz/fuzzing_common.c

@@ -1,6 +1,7 @@
 /* Copyright (c) 2016-2019, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 #define CRYPTO_ED25519_PRIVATE
+#define CONFIG_PRIVATE
 #include "orconfig.h"
 #include "core/or/or.h"
 #include "app/main/subsysmgr.h"
@@ -111,7 +112,7 @@ global_init(void)
   }
 
   /* set up the options. */
-  mock_options = tor_malloc_zero(sizeof(or_options_t));
+  mock_options = options_new();
   MOCK(get_options, mock_get_options);
 
   /* Make BUG() and nonfatal asserts crash */
@@ -189,7 +190,7 @@ main(int argc, char **argv)
   if (fuzz_cleanup() < 0)
     abort();
 
-  tor_free(mock_options);
+  or_options_free(mock_options);
   UNMOCK(get_options);
   return 0;
 }

+ 1 - 0
src/test/include.am

@@ -124,6 +124,7 @@ src_test_test_SOURCES += \
 	src/test/test_circuitstats.c \
 	src/test/test_compat_libevent.c \
 	src/test/test_config.c \
+	src/test/test_confmgr.c \
 	src/test/test_confparse.c \
 	src/test/test_connection.c \
 	src/test/test_conscache.c \

+ 1 - 0
src/test/test.c

@@ -840,6 +840,7 @@ struct testgroup_t testgroups[] = {
   { "circuituse/", circuituse_tests },
   { "compat/libevent/", compat_libevent_tests },
   { "config/", config_tests },
+  { "config/mgr/", confmgr_tests },
   { "config/parse/", confparse_tests },
   { "connection/", connection_tests },
   { "conscache/", conscache_tests },

+ 1 - 0
src/test/test.h

@@ -197,6 +197,7 @@ extern struct testcase_t circuitstats_tests[];
 extern struct testcase_t circuituse_tests[];
 extern struct testcase_t compat_libevent_tests[];
 extern struct testcase_t config_tests[];
+extern struct testcase_t confmgr_tests[];
 extern struct testcase_t confparse_tests[];
 extern struct testcase_t connection_tests[];
 extern struct testcase_t conscache_tests[];

+ 65 - 27
src/test/test_config.c

@@ -1755,6 +1755,18 @@ add_default_fallback_dir_servers_known_default(void)
   n_add_default_fallback_dir_servers_known_default++;
 }
 
+/* Helper for test_config_adding_dir_servers(), which should be
+ * refactored: clear the fields in the options which the options object
+ * does not really own. */
+static void
+ads_clear_helper(or_options_t *options)
+{
+  options->DirAuthorities = NULL;
+  options->AlternateBridgeAuthority = NULL;
+  options->AlternateDirAuthority = NULL;
+  options->FallbackDir = NULL;
+}
+
 /* Test all the different combinations of adding dir servers */
 static void
 test_config_adding_dir_servers(void *arg)
@@ -1762,7 +1774,7 @@ test_config_adding_dir_servers(void *arg)
   (void)arg;
 
   /* allocate options */
-  or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
+  or_options_t *options = options_new();
 
   /* Allocate and populate configuration lines:
    *
@@ -1885,7 +1897,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -1967,7 +1981,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2108,7 +2124,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2249,7 +2267,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2391,7 +2411,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2543,7 +2565,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2697,7 +2721,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -2860,7 +2886,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -3017,7 +3045,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -3183,7 +3213,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -3346,7 +3378,9 @@ test_config_adding_dir_servers(void *arg)
     n_add_default_fallback_dir_servers_known_default = 0;
 
     /* clear options*/
-    memset(options, 0, sizeof(or_options_t));
+    ads_clear_helper(options);
+    or_options_free(options);
+    options = options_new();
 
     /* clear any previous dir servers:
      consider_adding_dir_servers() should do this anyway */
@@ -3515,10 +3549,7 @@ test_config_adding_dir_servers(void *arg)
   tor_free(test_fallback_directory->value);
   tor_free(test_fallback_directory);
 
-  options->DirAuthorities = NULL;
-  options->AlternateBridgeAuthority = NULL;
-  options->AlternateDirAuthority = NULL;
-  options->FallbackDir = NULL;
+  ads_clear_helper(options);
   or_options_free(options);
 
   UNMOCK(add_default_fallback_dir_servers);
@@ -3533,7 +3564,7 @@ test_config_default_dir_servers(void *arg)
   int fallback_count = 0;
 
   /* new set of options should stop fallback parsing */
-  opts = tor_malloc_zero(sizeof(or_options_t));
+  opts = options_new();
   opts->UseDefaultFallbackDirs = 0;
   /* set old_options to NULL to force dir update */
   consider_adding_dir_servers(opts, NULL);
@@ -3547,7 +3578,7 @@ test_config_default_dir_servers(void *arg)
   /* if we disable the default fallbacks, there must not be any extra */
   tt_assert(fallback_count == trusted_count);
 
-  opts = tor_malloc_zero(sizeof(or_options_t));
+  opts = options_new();
   opts->UseDefaultFallbackDirs = 1;
   consider_adding_dir_servers(opts, opts);
   trusted_count = smartlist_len(router_get_trusted_dir_servers());
@@ -3607,7 +3638,7 @@ test_config_directory_fetch(void *arg)
   (void)arg;
 
   /* Test Setup */
-  or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
+  or_options_t *options = options_new();
   routerinfo_t routerinfo;
   memset(&routerinfo, 0, sizeof(routerinfo));
   mock_router_pick_published_address_result = -1;
@@ -3619,9 +3650,10 @@ test_config_directory_fetch(void *arg)
        mock_router_my_exit_policy_is_reject_star);
   MOCK(advertised_server_mode, mock_advertised_server_mode);
   MOCK(router_get_my_routerinfo, mock_router_get_my_routerinfo);
+  or_options_free(options);
+  options = options_new();
 
   /* Clients can use multiple directory mirrors for bootstrap */
-  memset(options, 0, sizeof(or_options_t));
   options->ClientOnly = 1;
   tt_assert(server_mode(options) == 0);
   tt_assert(public_server_mode(options) == 0);
@@ -3630,7 +3662,8 @@ test_config_directory_fetch(void *arg)
             OP_EQ, 1);
 
   /* Bridge Clients can use multiple directory mirrors for bootstrap */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->UseBridges = 1;
   tt_assert(server_mode(options) == 0);
   tt_assert(public_server_mode(options) == 0);
@@ -3640,7 +3673,8 @@ test_config_directory_fetch(void *arg)
 
   /* Bridge Relays (Bridges) must act like clients, and use multiple
    * directory mirrors for bootstrap */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->BridgeRelay = 1;
   options->ORPort_set = 1;
   tt_assert(server_mode(options) == 1);
@@ -3651,7 +3685,8 @@ test_config_directory_fetch(void *arg)
 
   /* Clients set to FetchDirInfoEarly must fetch it from the authorities,
    * but can use multiple authorities for bootstrap */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->FetchDirInfoEarly = 1;
   tt_assert(server_mode(options) == 0);
   tt_assert(public_server_mode(options) == 0);
@@ -3662,7 +3697,8 @@ test_config_directory_fetch(void *arg)
   /* OR servers only fetch the consensus from the authorities when they don't
    * know their own address, but never use multiple directories for bootstrap
    */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->ORPort_set = 1;
 
   mock_router_pick_published_address_result = -1;
@@ -3682,7 +3718,8 @@ test_config_directory_fetch(void *arg)
   /* Exit OR servers only fetch the consensus from the authorities when they
    * refuse unknown exits, but never use multiple directories for bootstrap
    */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->ORPort_set = 1;
   options->ExitRelay = 1;
   mock_router_pick_published_address_result = 0;
@@ -3712,7 +3749,8 @@ test_config_directory_fetch(void *arg)
    * advertising their dirport, and never use multiple directories for
    * bootstrap. This only applies if they are also OR servers.
    * (We don't care much about the behaviour of non-OR directory servers.) */
-  memset(options, 0, sizeof(or_options_t));
+  or_options_free(options);
+  options = options_new();
   options->DirPort_set = 1;
   options->ORPort_set = 1;
   options->DirCache = 1;
@@ -3766,7 +3804,7 @@ test_config_directory_fetch(void *arg)
             OP_EQ, 0);
 
  done:
-  tor_free(options);
+  or_options_free(options);
   UNMOCK(router_pick_published_address);
   UNMOCK(router_get_my_routerinfo);
   UNMOCK(advertised_server_mode);

+ 325 - 0
src/test/test_confmgr.c

@@ -0,0 +1,325 @@
+/* Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/*
+ * Tests for confparse.c's features that support multiple configuration
+ * formats and configuration objects.
+ */
+
+#define CONFPARSE_PRIVATE
+#include "orconfig.h"
+
+#include "core/or/or.h"
+#include "lib/encoding/confline.h"
+#include "app/config/confparse.h"
+#include "test/test.h"
+#include "test/log_test_helpers.h"
+
+/*
+ * Set up a few objects: a pasture_cfg is toplevel; it has a llama_cfg and an
+ * alpaca_cfg.
+ */
+
+typedef struct {
+  uint32_t magic;
+  char *address;
+  int opentopublic;
+  config_suite_t *subobjs;
+} pasture_cfg_t;
+
+typedef struct {
+  char *llamaname;
+  int cuteness;
+  uint32_t magic;
+  int eats_meat; /* deprecated; llamas are never carnivorous. */
+
+  char *description; // derived from other fields.
+} llama_cfg_t;
+
+typedef struct {
+  uint32_t magic;
+  int fuzziness;
+  char *alpacaname;
+  int n_wings; /* deprecated; alpacas don't have wings. */
+} alpaca_cfg_t;
+
+/*
+ * Make the above into configuration objects.
+ */
+
+static pasture_cfg_t pasture_cfg_t_dummy;
+static llama_cfg_t llama_cfg_t_dummy;
+static alpaca_cfg_t alpaca_cfg_t_dummy;
+
+#define PV(name, type, dflt) \
+  CONFIG_VAR_ETYPE(pasture_cfg_t, #name, type, name, 0, dflt)
+#define LV(name, type, dflt) \
+  CONFIG_VAR_ETYPE(llama_cfg_t, #name, type, name, 0, dflt)
+#define AV(name, type, dflt) \
+  CONFIG_VAR_ETYPE(alpaca_cfg_t, #name, type, name, 0, dflt)
+static const config_var_t pasture_vars[] = {
+  PV(address, STRING, NULL),
+  PV(opentopublic, BOOL, "1"),
+  END_OF_CONFIG_VARS
+};
+static const config_var_t llama_vars[] =
+{
+  LV(llamaname, STRING, NULL),
+  LV(eats_meat, BOOL, NULL),
+  LV(cuteness, POSINT, "100"),
+  END_OF_CONFIG_VARS
+};
+static const config_var_t alpaca_vars[] =
+{
+  AV(alpacaname, STRING, NULL),
+  AV(fuzziness, POSINT, "50"),
+  AV(n_wings, POSINT, "0"),
+  END_OF_CONFIG_VARS
+};
+
+static config_deprecation_t llama_deprecations[] = {
+  { "eats_meat", "Llamas are herbivores." },
+  {NULL,NULL}
+};
+
+static config_deprecation_t alpaca_deprecations[] = {
+  { "n_wings", "Alpacas are quadrupeds." },
+  {NULL,NULL}
+};
+
+static int clear_llama_cfg_called = 0;
+static void
+clear_llama_cfg(const config_mgr_t *mgr, void *llamacfg)
+{
+  (void)mgr;
+  llama_cfg_t *lc = llamacfg;
+  tor_free(lc->description);
+  ++clear_llama_cfg_called;
+}
+
+static config_abbrev_t llama_abbrevs[] = {
+  { "gracia", "cuteness", 0, 0 },
+  { "gentillesse", "cuteness", 0, 0 },
+  { NULL, NULL, 0, 0 },
+};
+
+static const config_format_t pasture_fmt = {
+  sizeof(pasture_cfg_t),
+  {
+    "pasture_cfg_t",
+    8989,
+    offsetof(pasture_cfg_t, magic)
+  },
+  .vars = pasture_vars,
+  .config_suite_offset = offsetof(pasture_cfg_t, subobjs),
+};
+
+static const config_format_t llama_fmt = {
+  sizeof(llama_cfg_t),
+  {
+    "llama_cfg_t",
+    0x11aa11,
+    offsetof(llama_cfg_t, magic)
+  },
+  .vars = llama_vars,
+  .config_suite_offset = -1,
+  .deprecations = llama_deprecations,
+  .abbrevs = llama_abbrevs,
+  .clear_fn = clear_llama_cfg,
+};
+
+static const config_format_t alpaca_fmt = {
+  sizeof(alpaca_cfg_t),
+  {
+    "alpaca_cfg_t",
+    0xa15aca,
+    offsetof(alpaca_cfg_t, magic)
+  },
+  .vars = alpaca_vars,
+  .config_suite_offset = -1,
+  .deprecations = alpaca_deprecations,
+};
+
+#define LLAMA_IDX 0
+#define ALPACA_IDX 1
+
+static config_mgr_t *
+get_mgr(bool freeze)
+{
+  config_mgr_t *mgr = config_mgr_new(&pasture_fmt);
+  tt_int_op(LLAMA_IDX, OP_EQ, config_mgr_add_format(mgr, &llama_fmt));
+  tt_int_op(ALPACA_IDX, OP_EQ, config_mgr_add_format(mgr, &alpaca_fmt));
+  if (freeze)
+    config_mgr_freeze(mgr);
+  return mgr;
+
+ done:
+  config_mgr_free(mgr);
+  return NULL;
+}
+
+static void
+test_confmgr_init(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = get_mgr(true);
+  smartlist_t *vars = NULL;
+  tt_ptr_op(mgr, OP_NE, NULL);
+
+  vars = config_mgr_list_vars(mgr);
+  tt_int_op(smartlist_len(vars), OP_EQ, 8); // 8 vars total.
+
+  tt_str_op("cuteness", OP_EQ, config_find_option_name(mgr, "CUTENESS"));
+  tt_str_op("cuteness", OP_EQ, config_find_option_name(mgr, "GRACIA"));
+  smartlist_free(vars);
+
+  vars = config_mgr_list_deprecated_vars(mgr); // 2 deprecated vars.
+  tt_int_op(smartlist_len(vars), OP_EQ, 2);
+  tt_assert(smartlist_contains_string(vars, "eats_meat"));
+  tt_assert(smartlist_contains_string(vars, "n_wings"));
+
+  tt_str_op("Llamas are herbivores.", OP_EQ,
+            config_find_deprecation(mgr, "EATS_MEAT"));
+  tt_str_op("Alpacas are quadrupeds.", OP_EQ,
+            config_find_deprecation(mgr, "N_WINGS"));
+
+ done:
+  smartlist_free(vars);
+  config_mgr_free(mgr);
+}
+
+static void
+test_confmgr_magic(void *args)
+{
+  (void)args;
+  // Every time we build a manager, it is supposed to get a different magic
+  // number.  Let's test that.
+  config_mgr_t *mgr1 = get_mgr(true);
+  config_mgr_t *mgr2 = get_mgr(true);
+  config_mgr_t *mgr3 = get_mgr(true);
+
+  pasture_cfg_t *p1 = NULL, *p2 = NULL, *p3 = NULL;
+
+  tt_assert(mgr1);
+  tt_assert(mgr2);
+  tt_assert(mgr3);
+
+  p1 = config_new(mgr1);
+  p2 = config_new(mgr2);
+  p3 = config_new(mgr3);
+
+  tt_assert(p1);
+  tt_assert(p2);
+  tt_assert(p3);
+
+  // By chance, two managers get the same magic with P=2^-32.  Let's
+  // make sure that at least two of them are different, so that our
+  // odds of a false positive are 1/2^-64.
+  tt_assert((p1->magic != p2->magic) || (p2->magic != p3->magic));
+
+ done:
+  config_free(mgr1, p1);
+  config_free(mgr2, p2);
+  config_free(mgr3, p3);
+
+  config_mgr_free(mgr1);
+  config_mgr_free(mgr2);
+  config_mgr_free(mgr3);
+}
+
+static const char *simple_pasture =
+  "LLamaname hugo\n"
+  "Alpacaname daphne\n"
+  "gentillesse 42\n"
+  "address 123 Camelid ave\n";
+
+static void
+test_confmgr_parse(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = get_mgr(true);
+  pasture_cfg_t *p = config_new(mgr);
+  config_line_t *lines = NULL;
+  char *msg = NULL;
+
+  config_init(mgr, p); // set defaults.
+
+  int r = config_get_lines(simple_pasture, &lines, 0);
+  tt_int_op(r, OP_EQ, 0);
+  r = config_assign(mgr, p, lines, 0, &msg);
+  tt_int_op(r, OP_EQ, 0);
+
+  tt_int_op(p->opentopublic, OP_EQ, 1);
+  tt_str_op(p->address, OP_EQ, "123 Camelid ave");
+
+  // We are using this API directly; modules outside confparse will, in the
+  // future, not.
+  const alpaca_cfg_t *ac = config_mgr_get_obj(mgr, p, ALPACA_IDX);
+  const llama_cfg_t *lc = config_mgr_get_obj(mgr, p, LLAMA_IDX);
+  tt_str_op(lc->llamaname, OP_EQ, "hugo");
+  tt_str_op(ac->alpacaname, OP_EQ, "daphne");
+  tt_int_op(lc->cuteness, OP_EQ, 42);
+  tt_int_op(ac->fuzziness, OP_EQ, 50);
+
+  // We set the description for the llama here, so that the clear function
+  // can clear it.  (Later we can do this in a verification function.)
+  clear_llama_cfg_called = 0;
+  llama_cfg_t *mut_lc = config_mgr_get_obj_mutable(mgr, p, LLAMA_IDX);
+  mut_lc->description = tor_strdup("A llama named Hugo.");
+  config_free(mgr, p);
+  tt_int_op(clear_llama_cfg_called, OP_EQ, 1);
+
+ done:
+  config_free_lines(lines);
+  config_free(mgr, p);
+  config_mgr_free(mgr);
+  tor_free(msg);
+}
+
+static void
+test_confmgr_dump(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = get_mgr(true);
+  pasture_cfg_t *p = config_new(mgr);
+  pasture_cfg_t *defaults = config_new(mgr);
+  config_line_t *lines = NULL;
+  char *msg = NULL;
+  char *s = NULL;
+
+  config_init(mgr, p); // set defaults.
+  config_init(mgr, defaults); // set defaults.
+
+  int r = config_get_lines(simple_pasture, &lines, 0);
+  tt_int_op(r, OP_EQ, 0);
+  r = config_assign(mgr, p, lines, 0, &msg);
+  tt_int_op(r, OP_EQ, 0);
+
+  s = config_dump(mgr, defaults, p, 1, 0);
+  tt_str_op("address 123 Camelid ave\n"
+            "alpacaname daphne\n"
+            "cuteness 42\n"
+            "llamaname hugo\n", OP_EQ, s);
+
+ done:
+  config_free_lines(lines);
+  config_free(mgr, p);
+  config_free(mgr, defaults);
+  config_mgr_free(mgr);
+
+  tor_free(msg);
+  tor_free(s);
+}
+
+#define CONFMGR_TEST(name, flags)                       \
+  { #name, test_confmgr_ ## name, flags, NULL, NULL }
+
+struct testcase_t confmgr_tests[] = {
+  CONFMGR_TEST(init, 0),
+  CONFMGR_TEST(magic, 0),
+  CONFMGR_TEST(parse, 0),
+  CONFMGR_TEST(dump, 0),
+  END_OF_TESTCASES
+};

+ 256 - 127
src/test/test_confparse.c

@@ -119,8 +119,6 @@ test_validate_cb(void *old_options, void *options, void *default_options,
   return 0;
 }
 
-static void test_free_cb(void *options);
-
 #define TEST_MAGIC 0x1337
 
 static const config_format_t test_fmt = {
@@ -134,29 +132,22 @@ static const config_format_t test_fmt = {
   test_deprecation_notes,
   test_vars,
   test_validate_cb,
-  test_free_cb,
   NULL,
+  NULL,
+  -1,
 };
 
-static void
-test_free_cb(void *options)
-{
-  if (!options)
-    return;
-
-  config_free(&test_fmt, options);
-}
-
 /* Make sure that config_init sets everything to the right defaults. */
 static void
 test_confparse_init(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
-  config_init(&test_fmt, tst);
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
+  test_struct_t *tst = config_new(mgr);
+  config_init(mgr, tst);
 
   // Make sure that options are initialized right. */
-  tt_uint_op(tst->magic, OP_EQ, TEST_MAGIC);
   tt_str_op(tst->s, OP_EQ, "hello");
   tt_ptr_op(tst->fn, OP_EQ, NULL);
   tt_int_op(tst->pos, OP_EQ, 0);
@@ -178,7 +169,8 @@ test_confparse_init(void *arg)
   tt_int_op(tst->hidden_int, OP_EQ, 0);
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
+  config_mgr_free(mgr);
 }
 
 static const char simple_settings[] =
@@ -207,18 +199,18 @@ static const char simple_settings[] =
 
 /* Return a configuration object set up from simple_settings above. */
 static test_struct_t *
-get_simple_config(void)
+get_simple_config(const config_mgr_t *mgr)
 {
   test_struct_t *result = NULL;
-  test_struct_t *tst = config_new(&test_fmt);
+  test_struct_t *tst = config_new(mgr);
   config_line_t *lines = NULL;
   char *msg = NULL;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines(simple_settings, &lines, 0);
   tt_int_op(r, OP_EQ, 0);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
 
@@ -227,7 +219,7 @@ get_simple_config(void)
  done:
   tor_free(msg);
   config_free_lines(lines);
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   return result;
 }
 
@@ -236,7 +228,9 @@ static void
 test_confparse_assign_simple(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+  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");
   tt_str_op(tst->fn, OP_EQ, "/simple/test of the");
@@ -284,10 +278,11 @@ test_confparse_assign_simple(void *arg)
   tt_str_op(tst->mixed_hidden_lines->next->value, OP_EQ, "ABC");
   tt_assert(!tst->mixed_hidden_lines->next->next);
 
-  tt_assert(config_check_ok(&test_fmt, tst, LOG_ERR));
+  tt_assert(config_check_ok(mgr, tst, LOG_ERR));
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
+  config_mgr_free(mgr);
 }
 
 /* Try to assign to an obsolete option, and make sure we get a warning. */
@@ -295,26 +290,29 @@ static void
 test_confparse_assign_obsolete(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines("obsolete option here",
                            &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   expect_single_log_msg_containing("Skipping obsolete configuration option");
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Try to assign to an deprecated option, and make sure we get a warning
@@ -323,30 +321,33 @@ static void
 test_confparse_assign_deprecated(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines("deprecated_int 7",
                            &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, CAL_WARN_DEPRECATIONS, &msg);
+  r = config_assign(mgr, tst, lines, CAL_WARN_DEPRECATIONS, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   expect_single_log_msg_containing("This integer is deprecated.");
 
   tt_int_op(tst->deprecated_int, OP_EQ, 7);
 
-  tt_assert(config_check_ok(&test_fmt, tst, LOG_ERR));
+  tt_assert(config_check_ok(mgr, tst, LOG_ERR));
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Try to re-assign an option name that has been depreacted in favor of
@@ -355,16 +356,18 @@ static void
 test_confparse_assign_replaced(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines("float 1000\n", &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, CAL_WARN_DEPRECATIONS, &msg);
+  r = config_assign(mgr, tst, lines, CAL_WARN_DEPRECATIONS, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   expect_single_log_msg_containing("use 'dbl' instead.");
@@ -374,9 +377,10 @@ test_confparse_assign_replaced(void *arg)
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Try to set a linelist value with no option. */
@@ -384,25 +388,28 @@ static void
 test_confparse_assign_emptystring(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines("lines\n", &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   expect_single_log_msg_containing("has no value");
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Try to set a the same option twice; make sure we get a warning. */
@@ -410,26 +417,29 @@ static void
 test_confparse_assign_twice(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines("pos 10\n"
                            "pos 99\n", &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   expect_single_log_msg_containing("used more than once");
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 typedef struct badval_test_t {
@@ -443,16 +453,18 @@ static void
 test_confparse_assign_badval(void *arg)
 {
   const badval_test_t *bt = arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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;
 
-  config_init(&test_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines(bt->cfg, &lines, 0);
   tt_int_op(r, OP_EQ, 0);
   setup_capture_of_logs(LOG_WARN);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_LT, 0);
   tt_ptr_op(msg, OP_NE, NULL);
   if (! strstr(msg, bt->expect_msg)) {
@@ -461,9 +473,10 @@ test_confparse_assign_badval(void *arg)
 
  done:
   teardown_capture_of_logs();
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Various arguments for badval test.
@@ -495,88 +508,90 @@ static void
 test_confparse_dump(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
+  test_struct_t *tst = get_simple_config(mgr);
   char *dumped = NULL;
 
   /* Minimal version. */
-  dumped = config_dump(&test_fmt, NULL, tst, 1, 0);
+  dumped = config_dump(mgr, NULL, tst, 1, 0);
   tt_str_op(dumped, OP_EQ,
-            "s this is a\n"
-            "fn /simple/test of the\n"
-            "pos 77\n"
-            "i 3\n"
-            "u64 1000000000000\n"
-            "interval 300\n"
-            "msec_interval 300000\n"
-            "mem 10\n"
-            "dbl 6.060842\n"
-            "boolean 1\n"
             "autobool 0\n"
-            "time 2019-06-14 13:58:51\n"
+            "boolean 1\n"
             "csv configuration,parsing,system\n"
             "csv_interval 10\n"
+            "dbl 6.060842\n"
+            "fn /simple/test of the\n"
+            "i 3\n"
+            "interval 300\n"
             "lines hello\n"
             "lines world\n"
+            "mem 10\n"
+            "VisibleLineB ABC\n"
             "LineTypeA i d\n"
             "LineTypeB i c\n"
+            "msec_interval 300000\n"
+            "pos 77\n"
             "routerset $FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF\n"
-            "VisibleLineB ABC\n");
+            "s this is a\n"
+            "time 2019-06-14 13:58:51\n"
+            "u64 1000000000000\n");
 
-  /* Maximal */
   tor_free(dumped);
-  dumped = config_dump(&test_fmt, NULL, tst, 0, 0);
+  dumped = config_dump(mgr, NULL, tst, 0, 0);
   tt_str_op(dumped, OP_EQ,
-            "s this is a\n"
-            "fn /simple/test of the\n"
-            "pos 77\n"
-            "i 3\n"
-            "deprecated_int 3\n"
-            "u64 1000000000000\n"
-            "interval 300\n"
-            "msec_interval 300000\n"
-            "mem 10\n"
-            "dbl 6.060842\n"
-            "boolean 1\n"
             "autobool 0\n"
-            "time 2019-06-14 13:58:51\n"
+            "boolean 1\n"
             "csv configuration,parsing,system\n"
             "csv_interval 10\n"
+            "dbl 6.060842\n"
+            "deprecated_int 3\n"
+            "fn /simple/test of the\n"
+            "i 3\n"
+            "interval 300\n"
             "lines hello\n"
             "lines world\n"
+            "mem 10\n"
+            "VisibleLineB ABC\n"
             "LineTypeA i d\n"
             "LineTypeB i c\n"
+            "msec_interval 300000\n"
+            "pos 77\n"
             "routerset $FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF\n"
-            "VisibleLineB ABC\n");
+            "s this is a\n"
+            "time 2019-06-14 13:58:51\n"
+            "u64 1000000000000\n");
 
   /* commented */
   tor_free(dumped);
-  dumped = config_dump(&test_fmt, NULL, tst, 0, 1);
+  dumped = config_dump(mgr, NULL, tst, 0, 1);
   tt_str_op(dumped, OP_EQ,
-            "s this is a\n"
-            "fn /simple/test of the\n"
-            "pos 77\n"
-            "i 3\n"
-            "# deprecated_int 3\n"
-            "u64 1000000000000\n"
-            "interval 300\n"
-            "msec_interval 300000\n"
-            "mem 10\n"
-            "dbl 6.060842\n"
-            "boolean 1\n"
             "autobool 0\n"
-            "time 2019-06-14 13:58:51\n"
+            "boolean 1\n"
             "csv configuration,parsing,system\n"
             "csv_interval 10\n"
+            "dbl 6.060842\n"
+            "# deprecated_int 3\n"
+            "fn /simple/test of the\n"
+            "i 3\n"
+            "interval 300\n"
             "lines hello\n"
             "lines world\n"
+            "mem 10\n"
+            "VisibleLineB ABC\n"
             "LineTypeA i d\n"
             "LineTypeB i c\n"
+            "msec_interval 300000\n"
+            "pos 77\n"
             "routerset $FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF\n"
-                        "VisibleLineB ABC\n");
+            "s this is a\n"
+            "time 2019-06-14 13:58:51\n"
+            "u64 1000000000000\n");
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   tor_free(dumped);
+  config_mgr_free(mgr);
 }
 
 /* Try confparse_reset_line(), and make sure it behaves correctly */
@@ -584,20 +599,23 @@ static void
 test_confparse_reset(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  config_mgr_freeze(mgr);
+  test_struct_t *tst = get_simple_config(mgr);
 
-  config_reset_line(&test_fmt, tst, "interval", 0);
+  config_reset_line(mgr, tst, "interval", 0);
   tt_int_op(tst->interval, OP_EQ, 0);
 
-  config_reset_line(&test_fmt, tst, "interval", 1);
+  config_reset_line(mgr, tst, "interval", 1);
   tt_int_op(tst->interval, OP_EQ, 10);
 
   tt_ptr_op(tst->routerset, OP_NE, NULL);
-  config_reset_line(&test_fmt, tst, "routerset", 0);
+  config_reset_line(mgr, tst, "routerset", 0);
   tt_ptr_op(tst->routerset, OP_EQ, NULL);
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
+  config_mgr_free(mgr);
 }
 
 /* Try setting options a second time on a config object, and make sure
@@ -606,7 +624,9 @@ static void
 test_confparse_reassign(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+  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;
 
@@ -617,7 +637,7 @@ test_confparse_reassign(void *arg)
          "csv 14,15\n"
          "routerset 127.0.0.1\n",
          &lines, 0);
-  r = config_assign(&test_fmt, tst,lines, 0, &msg);
+  r = config_assign(mgr, tst,lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
 
@@ -637,7 +657,7 @@ test_confparse_reassign(void *arg)
   tt_str_op(rs, OP_EQ, "127.0.0.1");
 
   // Try again with the CLEAR_FIRST and USE_DEFAULTS flags
-  r = config_assign(&test_fmt, tst, lines,
+  r = config_assign(mgr, tst, lines,
                     CAL_CLEAR_FIRST|CAL_USE_DEFAULTS, &msg);
   tt_int_op(r, OP_EQ, 0);
 
@@ -648,10 +668,11 @@ test_confparse_reassign(void *arg)
   tt_int_op(tst->i, OP_EQ, 12);
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
   tor_free(rs);
+  config_mgr_free(mgr);
 }
 
 /* Try setting options a second time on a config object, using the +foo
@@ -660,7 +681,9 @@ static void
 test_confparse_reassign_extend(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+  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;
 
@@ -668,7 +691,7 @@ test_confparse_reassign_extend(void *arg)
          "+lines 13\n",
          &lines, 1); // allow extended format.
   tt_int_op(r, OP_EQ, 0);
-  r = config_assign(&test_fmt, tst,lines, 0, &msg);
+  r = config_assign(mgr, tst,lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
 
@@ -688,27 +711,28 @@ test_confparse_reassign_extend(void *arg)
          "/lines\n",
          &lines, 1); // allow extended format.
   tt_int_op(r, OP_EQ, 0);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   tt_assert(tst->lines == NULL);
   config_free_lines(lines);
 
-  config_free(&test_fmt, tst);
-  tst = get_simple_config();
+  config_free(mgr, tst);
+  tst = get_simple_config(mgr);
   r = config_get_lines(
          "/lines away!\n",
          &lines, 1); // allow extended format.
   tt_int_op(r, OP_EQ, 0);
-  r = config_assign(&test_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
   tt_assert(tst->lines == NULL);
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
   tor_free(msg);
+  config_mgr_free(mgr);
 }
 
 /* Test out confparse_get_assigned(). */
@@ -716,30 +740,33 @@ static void
 test_confparse_get_assigned(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = get_simple_config();
+
+  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;
 
-  lines = config_get_assigned_option(&test_fmt, tst, "I", 1);
+  lines = config_get_assigned_option(mgr, tst, "I", 1);
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "i");
   tt_str_op(lines->value, OP_EQ, "3");
   tt_assert(lines->next == NULL);
   config_free_lines(lines);
 
-  lines = config_get_assigned_option(&test_fmt, tst, "s", 1);
+  lines = config_get_assigned_option(mgr, tst, "s", 1);
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "s");
   tt_str_op(lines->value, OP_EQ, "this is a");
   tt_assert(lines->next == NULL);
   config_free_lines(lines);
 
-  lines = config_get_assigned_option(&test_fmt, tst, "obsolete", 1);
+  lines = config_get_assigned_option(mgr, tst, "obsolete", 1);
   tt_assert(!lines);
 
-  lines = config_get_assigned_option(&test_fmt, tst, "nonesuch", 1);
+  lines = config_get_assigned_option(mgr, tst, "nonesuch", 1);
   tt_assert(!lines);
 
-  lines = config_get_assigned_option(&test_fmt, tst, "mixedlines", 1);
+  lines = config_get_assigned_option(mgr, tst, "mixedlines", 1);
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "LineTypeA");
   tt_str_op(lines->value, OP_EQ, "i d");
@@ -749,7 +776,7 @@ test_confparse_get_assigned(void *arg)
   tt_assert(lines->next->next == NULL);
   config_free_lines(lines);
 
-  lines = config_get_assigned_option(&test_fmt, tst, "linetypeb", 1);
+  lines = config_get_assigned_option(mgr, tst, "linetypeb", 1);
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "LineTypeB");
   tt_str_op(lines->value, OP_EQ, "i c");
@@ -758,7 +785,7 @@ test_confparse_get_assigned(void *arg)
 
   tor_free(tst->s);
   tst->s = tor_strdup("Hello\nWorld");
-  lines = config_get_assigned_option(&test_fmt, tst, "s", 1);
+  lines = config_get_assigned_option(mgr, tst, "s", 1);
   tt_assert(lines);
   tt_str_op(lines->key, OP_EQ, "s");
   tt_str_op(lines->value, OP_EQ, "\"Hello\\nWorld\"");
@@ -766,8 +793,9 @@ test_confparse_get_assigned(void *arg)
   config_free_lines(lines);
 
  done:
-  config_free(&test_fmt, tst);
+  config_free(mgr, tst);
   config_free_lines(lines);
+  config_mgr_free(mgr);
 }
 
 /* Another variant, which accepts and stores unrecognized lines.*/
@@ -790,8 +818,9 @@ static config_format_t etest_fmt = {
   test_deprecation_notes,
   test_vars,
   test_validate_cb,
-  test_free_cb,
+  NULL,
   &extra,
+  -1,
 };
 
 /* Try out the feature where we can store unrecognized lines and dump them
@@ -800,24 +829,26 @@ static void
 test_confparse_extra_lines(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&etest_fmt);
+  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;
 
-  config_init(&etest_fmt, tst);
+  config_init(mgr, tst);
 
   int r = config_get_lines(
       "unknotty addita\n"
       "pos 99\n"
       "wombat knish\n", &lines, 0);
   tt_int_op(r, OP_EQ, 0);
-  r = config_assign(&etest_fmt, tst, lines, 0, &msg);
+  r = config_assign(mgr, tst, lines, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
   tt_ptr_op(msg, OP_EQ, NULL);
 
   tt_assert(tst->extra_lines);
 
-  dump = config_dump(&etest_fmt, NULL, tst, 1, 0);
+  dump = config_dump(mgr, NULL, tst, 1, 0);
   tt_str_op(dump, OP_EQ,
       "pos 99\n"
       "unknotty addita\n"
@@ -827,7 +858,8 @@ test_confparse_extra_lines(void *arg)
   tor_free(msg);
   tor_free(dump);
   config_free_lines(lines);
-  config_free(&etest_fmt, tst);
+  config_free(mgr, tst);
+  config_mgr_free(mgr);
 }
 
 static void
@@ -893,12 +925,106 @@ static void
 test_confparse_check_ok_fail(void *arg)
 {
   (void)arg;
-  test_struct_t *tst = config_new(&test_fmt);
+  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(&test_fmt, tst, LOG_INFO));
+  tt_assert(! config_check_ok(mgr, tst, LOG_INFO));
+
+ done:
+  config_free(mgr, tst);
+  config_mgr_free(mgr);
+}
+
+static void
+test_confparse_list_vars(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  smartlist_t *vars = config_mgr_list_vars(mgr);
+  smartlist_t *varnames = smartlist_new();
+  char *joined = NULL;
+
+  tt_assert(vars);
+  SMARTLIST_FOREACH(vars, config_var_t *, cv,
+                    smartlist_add(varnames, (void*)cv->member.name));
+  smartlist_sort_strings(varnames);
+  joined = smartlist_join_strings(varnames, "::", 0, NULL);
+  tt_str_op(joined, OP_EQ,
+            "LineTypeA::"
+            "LineTypeB::"
+            "MixedHiddenLines::"
+            "MixedLines::"
+            "VisibleLineB::"
+            "__HiddenInt::"
+            "__HiddenLineA::"
+            "autobool::"
+            "boolean::"
+            "csv::"
+            "csv_interval::"
+            "dbl::"
+            "deprecated_int::"
+            "fn::"
+            "i::"
+            "interval::"
+            "lines::"
+            "mem::"
+            "msec_interval::"
+            "obsolete::"
+            "pos::"
+            "routerset::"
+            "s::"
+            "time::"
+            "u64");
+
+ done:
+  tor_free(joined);
+  smartlist_free(varnames);
+  smartlist_free(vars);
+  config_mgr_free(mgr);
+}
+
+static void
+test_confparse_list_deprecated(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+  smartlist_t *vars = config_mgr_list_deprecated_vars(mgr);
+  char *joined = NULL;
+
+  tt_assert(vars);
+  smartlist_sort_strings(vars);
+  joined = smartlist_join_strings(vars, "::", 0, NULL);
+
+  tt_str_op(joined, OP_EQ, "deprecated_int");
+
+ done:
+  tor_free(joined);
+  smartlist_free(vars);
+  config_mgr_free(mgr);
+}
+
+static void
+test_confparse_find_option_name(void *arg)
+{
+  (void)arg;
+  config_mgr_t *mgr = config_mgr_new(&test_fmt);
+
+  // exact match
+  tt_str_op(config_find_option_name(mgr, "u64"), OP_EQ, "u64");
+  // case-insensitive match
+  tt_str_op(config_find_option_name(mgr, "S"), OP_EQ, "s");
+  tt_str_op(config_find_option_name(mgr, "linetypea"), OP_EQ, "LineTypeA");
+  // prefix match
+  tt_str_op(config_find_option_name(mgr, "deprec"), OP_EQ, "deprecated_int");
+  // explicit abbreviation
+  tt_str_op(config_find_option_name(mgr, "uint"), OP_EQ, "pos");
+  tt_str_op(config_find_option_name(mgr, "UINT"), OP_EQ, "pos");
+  // no match
+  tt_ptr_op(config_find_option_name(mgr, "absent"), OP_EQ, NULL);
 
  done:
-  config_free(&test_fmt, tst);
+  config_mgr_free(mgr);
 }
 
 #define CONFPARSE_TEST(name, flags)                          \
@@ -937,5 +1063,8 @@ struct testcase_t confparse_tests[] = {
   CONFPARSE_TEST(extra_lines, 0),
   CONFPARSE_TEST(unitparse, 0),
   CONFPARSE_TEST(check_ok_fail, 0),
+  CONFPARSE_TEST(list_vars, 0),
+  CONFPARSE_TEST(list_deprecated, 0),
+  CONFPARSE_TEST(find_option_name, 0),
   END_OF_TESTCASES
 };

+ 1 - 2
src/test/test_dir_handle_get.c

@@ -479,8 +479,7 @@ static or_options_t *mock_options = NULL;
 static void
 init_mock_options(void)
 {
-  mock_options = tor_malloc(sizeof(or_options_t));
-  memset(mock_options, 0, sizeof(or_options_t));
+  mock_options = options_new();
   mock_options->TestingTorNetwork = 1;
   mock_options->DataDirectory = tor_strdup(get_fname_rnd("datadir_tmp"));
   mock_options->CacheDirectory = tor_strdup(mock_options->DataDirectory);

+ 16 - 14
src/test/test_entrynodes.c

@@ -5,6 +5,7 @@
 
 #define CIRCUITLIST_PRIVATE
 #define CIRCUITBUILD_PRIVATE
+#define CONFIG_PRIVATE
 #define STATEFILE_PRIVATE
 #define ENTRYNODES_PRIVATE
 #define ROUTERLIST_PRIVATE
@@ -201,7 +202,7 @@ big_fake_network_setup(const struct testcase_t *testcase)
     smartlist_add(big_fake_net_nodes, n);
   }
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
   dummy_consensus = tor_malloc_zero(sizeof(networkstatus_t));
   if (reasonably_future_consensus) {
     /* Make the dummy consensus valid in 6 hours, and expiring in 7 hours. */
@@ -235,12 +236,12 @@ mock_randomize_time_no_randomization(time_t a, time_t b)
   return a;
 }
 
-static or_options_t mocked_options;
+static or_options_t *mocked_options;
 
 static const or_options_t *
 mock_get_options(void)
 {
-  return &mocked_options;
+  return mocked_options;
 }
 
 #define TEST_IPV4_ADDR "123.45.67.89"
@@ -259,7 +260,7 @@ test_node_preferred_orport(void *arg)
   tor_addr_port_t ap;
 
   /* Setup options */
-  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options = options_new();
   /* We don't test ClientPreferIPv6ORPort here, because it's used in
    * nodelist_set_consensus to setup node.ipv6_preferred, which we set
    * directly. */
@@ -282,8 +283,8 @@ test_node_preferred_orport(void *arg)
 
   /* Check the preferred address is IPv4 if we're only using IPv4, regardless
    * of whether we prefer it or not */
-  mocked_options.ClientUseIPv4 = 1;
-  mocked_options.ClientUseIPv6 = 0;
+  mocked_options->ClientUseIPv4 = 1;
+  mocked_options->ClientUseIPv6 = 0;
   node.ipv6_preferred = 0;
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
@@ -296,8 +297,8 @@ test_node_preferred_orport(void *arg)
 
   /* Check the preferred address is IPv4 if we're using IPv4 and IPv6, but
    * don't prefer the IPv6 address */
-  mocked_options.ClientUseIPv4 = 1;
-  mocked_options.ClientUseIPv6 = 1;
+  mocked_options->ClientUseIPv4 = 1;
+  mocked_options->ClientUseIPv6 = 1;
   node.ipv6_preferred = 0;
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
@@ -305,28 +306,29 @@ test_node_preferred_orport(void *arg)
 
   /* Check the preferred address is IPv6 if we prefer it and
    * ClientUseIPv6 is 1, regardless of ClientUseIPv4 */
-  mocked_options.ClientUseIPv4 = 1;
-  mocked_options.ClientUseIPv6 = 1;
+  mocked_options->ClientUseIPv4 = 1;
+  mocked_options->ClientUseIPv6 = 1;
   node.ipv6_preferred = 1;
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
   tt_assert(ap.port == ipv6_port);
 
-  mocked_options.ClientUseIPv4 = 0;
+  mocked_options->ClientUseIPv4 = 0;
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
   tt_assert(ap.port == ipv6_port);
 
   /* Check the preferred address is IPv6 if we don't prefer it, but
    * ClientUseIPv4 is 0 */
-  mocked_options.ClientUseIPv4 = 0;
-  mocked_options.ClientUseIPv6 = 1;
-  node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(&mocked_options);
+  mocked_options->ClientUseIPv4 = 0;
+  mocked_options->ClientUseIPv6 = 1;
+  node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(mocked_options);
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
   tt_assert(ap.port == ipv6_port);
 
  done:
+  or_options_free(mocked_options);
   UNMOCK(get_options);
 }
 

+ 1 - 1
src/test/test_helpers.c

@@ -295,7 +295,7 @@ helper_parse_options(const char *conf)
   if (ret != 0) {
     goto done;
   }
-  ret = config_assign(&options_format, opt, line, 0, &msg);
+  ret = config_assign(get_options_mgr(), opt, line, 0, &msg);
   if (ret != 0) {
     goto done;
   }

+ 5 - 5
src/test/test_hs_service.c

@@ -1178,7 +1178,7 @@ test_introduce2(void *arg)
   MOCK(get_or_state,
        get_or_state_replacement);
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_S_INTRO, flags);
   tt_assert(circ);
@@ -1343,7 +1343,7 @@ test_rotate_descriptors(void *arg)
 
   (void) arg;
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   hs_init();
   MOCK(get_or_state, get_or_state_replacement);
@@ -1460,7 +1460,7 @@ test_build_update_descriptors(void *arg)
   MOCK(networkstatus_get_live_consensus,
        mock_networkstatus_get_live_consensus);
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 03:00:00 UTC",
                            &mock_ns.valid_after);
@@ -1691,7 +1691,7 @@ test_build_descriptors(void *arg)
   MOCK(networkstatus_get_live_consensus,
        mock_networkstatus_get_live_consensus);
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 03:00:00 UTC",
                            &mock_ns.valid_after);
@@ -1792,7 +1792,7 @@ test_upload_descriptors(void *arg)
   MOCK(networkstatus_get_live_consensus,
        mock_networkstatus_get_live_consensus);
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
                            &mock_ns.valid_after);

+ 7 - 7
src/test/test_options.c

@@ -96,7 +96,7 @@ clear_log_messages(void)
     opt->command = CMD_RUN_TOR;              \
     options_init(opt);                       \
                                              \
-    dflt = config_dup(&options_format, opt); \
+    dflt = config_dup(get_options_mgr(), opt); \
     clear_log_messages();                    \
   } while (0)
 
@@ -196,7 +196,7 @@ test_options_validate_impl(const char *configuration,
   if (r)
     goto done;
 
-  r = config_assign(&options_format, opt, cl, 0, &msg);
+  r = config_assign(get_options_mgr(), opt, cl, 0, &msg);
   if (phase == PH_ASSIGN) {
     if (test_options_checkmsgs(configuration, expect_errmsg,
                                expect_log_severity,
@@ -304,7 +304,7 @@ test_have_enough_mem_for_dircache(void *arg)
   r = config_get_lines(configuration, &cl, 1);
   tt_int_op(r, OP_EQ, 0);
 
-  r = config_assign(&options_format, opt, cl, 0, &msg);
+  r = config_assign(get_options_mgr(), opt, cl, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
 
   /* 300 MB RAM available, DirCache enabled */
@@ -327,7 +327,7 @@ test_have_enough_mem_for_dircache(void *arg)
   r = config_get_lines(configuration, &cl, 1);
   tt_int_op(r, OP_EQ, 0);
 
-  r = config_assign(&options_format, opt, cl, 0, &msg);
+  r = config_assign(get_options_mgr(), opt, cl, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
 
   /* 300 MB RAM available, DirCache enabled, Bridge */
@@ -350,7 +350,7 @@ test_have_enough_mem_for_dircache(void *arg)
   r = config_get_lines(configuration, &cl, 1);
   tt_int_op(r, OP_EQ, 0);
 
-  r = config_assign(&options_format, opt, cl, 0, &msg);
+  r = config_assign(get_options_mgr(), opt, cl, 0, &msg);
   tt_int_op(r, OP_EQ, 0);
 
   /* 200 MB RAM available, DirCache disabled */
@@ -438,7 +438,7 @@ get_options_test_data(const char *conf)
 
   rv = config_get_lines(conf, &cl, 1);
   tt_int_op(rv, OP_EQ, 0);
-  rv = config_assign(&options_format, result->opt, cl, 0, &msg);
+  rv = config_assign(get_options_mgr(), result->opt, cl, 0, &msg);
   if (msg) {
     /* Display the parse error message by comparing it with an empty string */
     tt_str_op(msg, OP_EQ, "");
@@ -449,7 +449,7 @@ get_options_test_data(const char *conf)
   result->opt->TokenBucketRefillInterval = 1;
   rv = config_get_lines(TEST_OPTIONS_OLD_VALUES, &cl, 1);
   tt_int_op(rv, OP_EQ, 0);
-  rv = config_assign(&options_format, result->def_opt, cl, 0, &msg);
+  rv = config_assign(get_options_mgr(), result->def_opt, cl, 0, &msg);
   if (msg) {
     /* Display the parse error message by comparing it with an empty string */
     tt_str_op(msg, OP_EQ, "");

+ 1 - 1
src/test/test_pt.c

@@ -352,7 +352,7 @@ test_pt_configure_proxy(void *arg)
   managed_proxy_t *mp = NULL;
   (void) arg;
 
-  dummy_state = tor_malloc_zero(sizeof(or_state_t));
+  dummy_state = or_state_new();
 
   MOCK(process_read_stdout, process_read_stdout_replacement);
   MOCK(get_or_state,

Some files were not shown because too many files changed in this diff