Refactor config free logic to use a single path.

The right way to free a config object is now to wrap config_free(),
always.  Instead of creating an alternative free function, objects
should provide an alternative clear callback to free any fields that
the configuration manager doesn't manage.

This lets us simplify our code a little, and lets us extend the
confparse.c code to manage additional fields in config_free.

4 files changed, 23 insertions, 31 deletions

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 7aa253a6bf..cdf5fa266c 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -843,9 +843,9 @@ 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(const config_mgr_t *, 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
@@ -862,8 +862,8 @@ static const config_format_t options_format = {
   option_deprecation_notes_,
   option_vars_,
   options_validate_cb,
-  options_free_cb,
-  NULL
+  options_clear_cb,
+  NULL,
 };
 
 /*
@@ -1011,11 +1011,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) {
@@ -1038,6 +1038,13 @@ or_options_free_(or_options_t *options)
   tor_free(options->command_arg);
   tor_free(options->master_key_fname);
   config_free_lines(options->MyFamily);
+}
+
+/** Release all memory allocated in options
+ */
+STATIC void
+or_options_free_(or_options_t *options)
+{
   config_free(get_options_mgr(), options);
 }
 
@@ -3164,14 +3171,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(const config_mgr_t *mgr, void *options)
-{
-  (void)mgr;
-  or_options_free_(options);
-}
-
 #define REJECT(arg) \
   STMT_BEGIN *msg = tor_strdup(arg); return -1; STMT_END
 #if defined(__GNUC__) && __GNUC__ <= 3
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 998c0e6ef2..8e3ad7fafd 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -789,6 +789,10 @@ config_free_(const config_mgr_t *mgr, void *options)
 
   tor_assert(fmt);
 
+  if (mgr->toplevel->clear_fn) {
+    mgr->toplevel->clear_fn(mgr, options);
+  }
+
   SMARTLIST_FOREACH_BEGIN(mgr->all_vars, const managed_var_t *, mv) {
     config_clear(mgr, options, mv);
   } SMARTLIST_FOREACH_END(mv);
@@ -977,9 +981,7 @@ config_dump(const config_mgr_t *mgr, const void *default_options,
   result = smartlist_join_strings(elements, "", 0, NULL);
   SMARTLIST_FOREACH(elements, char *, cp, tor_free(cp));
   smartlist_free(elements);
-  if (defaults_tmp) {
-    fmt->free_fn(mgr, defaults_tmp);
-  }
+  config_free(mgr, defaults_tmp);
   return result;
 }
 
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index 83e886696e..2e1a4b4f57 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -41,8 +41,8 @@ typedef int (*validate_fn_t)(void*,void*,void*,int,char**);
 
 struct config_mgr_t;
 
-/** Callback to free a configuration object. */
-typedef void (*free_cfg_fn_t)(const struct config_mgr_t *mgr, void*);
+/** Callback to clear all non-managed fields of a configuration object. */
+typedef void (*clear_cfg_fn_t)(const struct config_mgr_t *mgr, void*);
 
 /** Information on the keys, value types, key-to-struct-member mappings,
  * variable descriptions, validation functions, and abbreviations for a
@@ -57,7 +57,7 @@ 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;
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index 54ee398a7f..ede35e6ced 100644
--- a/src/app/config/statefile.c
+++ b/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(const config_mgr_t *mgr, void *state);
-
 /** Magic value for or_state_t. */
 #define OR_STATE_MAGIC 0x57A73f57
 
@@ -170,7 +168,7 @@ static const config_format_t state_format = {
   NULL,
   state_vars_,
   or_state_validate_cb,
-  or_state_free_cb,
+  NULL,
   &state_extra_var,
 };
 
@@ -282,13 +280,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(const config_mgr_t *mgr, void *state)
-{
-  (void)mgr;
-  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