config.c: extract validate, check-transition, and set into a function

This eliminates duplicated code.  The options_validate() function
itself is now tests-only.

src/app/config/config.c

@@ -858,6 +858,9 @@ static int options_validate_cb(const void *old_options, 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);
+static setopt_err_t options_validate_and_set(const or_options_t *old_options,
+                                             or_options_t *new_options,
+                                             char **msg_out);
 
 /** Magic value for or_options_t. */
 #define OR_OPTIONS_MAGIC 9090909
@@ -2684,37 +2687,9 @@ options_trial_assign(config_line_t *list, unsigned flags, char **msg)
     or_options_free(trial_options);
     return r;
   }
+  const or_options_t *cur_options = get_options();
 
-  setopt_err_t rv;
+  return options_validate_and_set(cur_options, trial_options, msg);
-  or_options_t *cur_options = get_options_mutable();
-
-  in_option_validation = 1;
-
-  if (options_validate(cur_options, trial_options,
-                       msg) < 0) {
-    or_options_free(trial_options);
-    rv = SETOPT_ERR_PARSE; /*XXX make this a separate return value. */
-    goto done;
-  }
-
-  if (options_transition_allowed(cur_options, trial_options, msg) < 0) {
-    or_options_free(trial_options);
-    rv = SETOPT_ERR_TRANSITION;
-    goto done;
-  }
-  in_option_validation = 0;
-
-  if (set_options(trial_options, msg)<0) {
-    or_options_free(trial_options);
-    rv = SETOPT_ERR_SETTING;
-    goto done;
-  }
-
-  /* we liked it. put it in place. */
-  rv = SETOPT_OK;
- done:
-  in_option_validation = 0;
-  return rv;
 }
 
 /** Print a usage message for tor. */
@@ -3239,6 +3214,54 @@ compute_publishserverdescriptor(or_options_t *options)
  * */
 #define RECOMMENDED_MIN_CIRCUIT_BUILD_TIMEOUT (10)
 
+/**
+ * Validate <b>new_options</b>.  If it is valid, and it is a reasonable
+ * replacement for <b>old_options</b>, replace the previous value of the
+ * global options, and return return SETOPT_OK.
+ *
+ * If it is not valid, then free <b>new_options</b>, set *<b>msg_out</b> to a
+ * newly allocated error message, and return an error code.
+ */
+static setopt_err_t
+options_validate_and_set(const or_options_t *old_options,
+                         or_options_t *new_options,
+                         char **msg_out)
+{
+  setopt_err_t rv;
+  validation_status_t vs;
+
+  in_option_validation = 1;
+  vs = config_validate(get_options_mgr(), old_options, new_options, msg_out);
+
+  if (vs == VSTAT_TRANSITION_ERR) {
+    rv = SETOPT_ERR_TRANSITION;
+    goto err;
+  } else if (vs < 0) {
+    rv = SETOPT_ERR_PARSE;
+    goto err;
+  }
+
+  if (options_transition_allowed(old_options, new_options, msg_out) < 0) {
+    rv = SETOPT_ERR_TRANSITION;
+    goto err;
+  }
+  in_option_validation = 0;
+
+  if (set_options(new_options, msg_out)) {
+    rv = SETOPT_ERR_SETTING;
+    goto err;
+  }
+
+  rv = SETOPT_OK;
+  new_options = NULL; /* prevent free */
+ err:
+  in_option_validation = 0;
+  tor_assert(new_options == NULL || rv != SETOPT_OK);
+  or_options_free(new_options);
+  return rv;
+}
+
+#ifdef TOR_UNIT_TESTS
 /**
  * Return 0 if every setting in <b>options</b> is reasonable, is a
  * permissible transition from <b>old_options</b>, and none of the
@@ -3248,7 +3271,7 @@ compute_publishserverdescriptor(or_options_t *options)
  *
  * On error, tor_strdup an error explanation into *<b>msg</b>.
  */
-STATIC int
+int
 options_validate(const or_options_t *old_options, or_options_t *options,
                  char **msg)
 {
@@ -3256,6 +3279,7 @@ options_validate(const or_options_t *old_options, or_options_t *options,
   vs = config_validate(get_options_mgr(), old_options, options, msg);
   return vs < 0 ? -1 : 0;
 }
+#endif
 
 #define REJECT(arg) \
   STMT_BEGIN *msg = tor_strdup(arg); return -1; STMT_END
@@ -5529,25 +5553,12 @@ options_init_from_string(const char *cf_defaults, const char *cf,
   }
 
   newoptions->IncludeUsed = cf_has_include;
-  in_option_validation = 1;
   newoptions->FilesOpenedByIncludes = opened_files;
+  opened_files = NULL; // prevent double-free.
 
-  /* Validate newoptions */
+  err = options_validate_and_set(oldoptions, newoptions, msg);
-  if (options_validate(oldoptions, newoptions, msg) < 0) {
+  if (err < 0)
-    err = SETOPT_ERR_PARSE; /*XXX make this a separate return value.*/
     goto err;
-  }
-
-  if (options_transition_allowed(oldoptions, newoptions, msg) < 0) {
-    err = SETOPT_ERR_TRANSITION;
-    goto err;
-  }
-  in_option_validation = 0;
-
-  if (set_options(newoptions, msg)) {
-    err = SETOPT_ERR_SETTING;
-    goto err; /* frees and replaces old options */
-  }
 
   or_options_free(global_default_options);
   global_default_options = newdefaultoptions;
@@ -5560,9 +5571,6 @@ options_init_from_string(const char *cf_defaults, const char *cf,
     SMARTLIST_FOREACH(opened_files, char *, f, tor_free(f));
     smartlist_free(opened_files);
   }
-  // may have been set to opened_files, avoid double free
-  newoptions->FilesOpenedByIncludes = NULL;
-  or_options_free(newoptions);
   or_options_free(newdefaultoptions);
   if (*msg) {
     char *old_msg = *msg;

src/app/config/config.h

@@ -277,9 +277,6 @@ STATIC void port_cfg_free_(port_cfg_t *port);
 STATIC void or_options_free_(or_options_t *options);
 STATIC int options_validate_single_onion(or_options_t *options,
                                          char **msg);
-STATIC int options_validate(const or_options_t *old_options,
-                            or_options_t *options,
-                            char **msg);
 STATIC int parse_transport_line(const or_options_t *options,
                                 const char *line, int validate_only,
                                 int server);
@@ -311,6 +308,12 @@ STATIC int open_and_add_file_log(const log_severity_list_t *severity,
 STATIC int options_init_logs(const or_options_t *old_options,
                              or_options_t *options, int validate_only);
 
+#ifdef TOR_UNIT_TESTS
+int options_validate(const or_options_t *old_options,
+                     or_options_t *options,
+                     char **msg);
+#endif
+
 #endif /* defined(CONFIG_PRIVATE) */
 
 #endif /* !defined(TOR_CONFIG_H) */