When the controller's *setconf commands fail, collect an error message

in a string and hand it back. This starts to resolve bug 275.


svn:r6241

src/or/circuitbuild.c

@@ -2184,17 +2184,17 @@ choose_random_entry(cpath_build_state_t *state)
 /** Parse <b>state</b> and learn about the entry guards it describes.
  * If <b>set</b> is true, and there are no errors, replace the global
  * entry_list with what we find.
- * On success, return 0. On failure, set *<b>err</b> to a string
+ * On success, return 0. On failure, alloc into *<b>msg</b> a string
  * describing the error, and return -1.
  */
 int
-entry_guards_parse_state(or_state_t *state, int set, const char **err)
+entry_guards_parse_state(or_state_t *state, int set, char **msg)
 {
   entry_guard_t *node = NULL;
   smartlist_t *new_entry_guards = smartlist_create();
   config_line_t *line;
 
-  *err = NULL;
+  *msg = NULL;
   for (line = state->EntryGuards; line; line = line->next) {
     if (!strcasecmp(line->key, "EntryGuard")) {
       smartlist_t *args = smartlist_create();
@@ -2205,28 +2205,33 @@ entry_guards_parse_state(or_state_t *state, int set, const char **err)
       smartlist_split_string(args, line->value, " ",
                              SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
       if (smartlist_len(args)<2) {
-        *err = "Too few arguments to EntryGuard";
+        *msg = tor_strdup("Unable to parse entry nodes: "
+                          "Too few arguments to EntryGuard");
       } else if (!is_legal_nickname(smartlist_get(args,0))) {
-        *err = "Bad nickname for EntryGuard";
+        *msg = tor_strdup("Unable to parse entry nodes: "
+                          "Bad nickname for EntryGuard");
       } else {
         strlcpy(node->nickname, smartlist_get(args,0), MAX_NICKNAME_LEN+1);
         if (base16_decode(node->identity, DIGEST_LEN, smartlist_get(args,1),
                           strlen(smartlist_get(args,1)))<0) {
-          *err = "Bad hex digest for EntryGuard";
+          *msg = tor_strdup("Unable to parse entry nodes: "
+                            "Bad hex digest for EntryGuard");
         }
       }
       SMARTLIST_FOREACH(args, char*, cp, tor_free(cp));
       smartlist_free(args);
-      if (*err)
+      if (*msg)
         break;
     } else {
       time_t when;
       if (!node) {
-        *err = "EntryGuardDownSince/UnlistedSince without EntryGuard";
+        *msg = tor_strdup("Unable to parse entry nodes: "
+               "EntryGuardDownSince/UnlistedSince without EntryGuard");
         break;
       }
       if (parse_iso_time(line->value, &when)<0) {
-        *err = "Bad time in EntryGuardDownSince/UnlistedSince";
+        *msg = tor_strdup("Unable to parse entry nodes: "
+                          "Bad time in EntryGuardDownSince/UnlistedSince");
         break;
       }
       if (!strcasecmp(line->key, "EntryGuardDownSince"))
@@ -2236,7 +2241,7 @@ entry_guards_parse_state(or_state_t *state, int set, const char **err)
     }
   }
 
-  if (*err || !set) {
+  if (*msg || !set) {
     SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e, tor_free(e));
     smartlist_free(new_entry_guards);
   } else { /* !*err && set */
@@ -2247,7 +2252,7 @@ entry_guards_parse_state(or_state_t *state, int set, const char **err)
     entry_guards = new_entry_guards;
     entry_guards_dirty = 0;
   }
-  return *err ? -1 : 0;
+  return *msg ? -1 : 0;
 }
 
 /** Our list of entry guards has changed, or some element of one

src/or/control.c

@@ -663,6 +663,7 @@ control_setconf_helper(connection_t *conn, uint32_t len, char *body,
   int r;
   config_line_t *lines=NULL;
   char *start = body;
+  char *errstring = NULL;
   int v0 = STATE_IS_V0(conn->state);
 
   if (!v0) {
@@ -717,11 +718,13 @@ control_setconf_helper(connection_t *conn, uint32_t len, char *body,
     }
   }
 
-  if ((r=options_trial_assign(lines, use_defaults, clear_first)) < 0) {
+  if ((r=options_trial_assign(lines, use_defaults,
+                              clear_first, &errstring)) < 0) {
     int v0_err;
     const char *msg;
     log_warn(LD_CONTROL,
-             "Controller gave us config lines that didn't validate.");
+             "Controller gave us config lines that didn't validate: %s.",
+             errstring);
     switch (r) {
       case -1:
         v0_err = ERR_UNRECOGNIZED_CONFIG_KEY;
@@ -744,9 +747,10 @@ control_setconf_helper(connection_t *conn, uint32_t len, char *body,
     if (v0) {
       send_control0_error(conn, v0_err, msg);
     } else {
-      connection_printf_to_buf(conn, "%s\r\n", msg);
+      connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring);
     }
     config_free_lines(lines);
+    tor_free(errstring);
     return 0;
   }
   config_free_lines(lines);

src/or/or.h

@@ -1527,7 +1527,7 @@ int entry_guard_set_status(const char *digest, int succeeded);
 void entry_nodes_should_be_added(void);
 void entry_guards_prepend_from_config(void);
 void entry_guards_update_state(or_state_t *state);
-int entry_guards_parse_state(or_state_t *state, int set, const char **err);
+int entry_guards_parse_state(or_state_t *state, int set, char **msg);
 int entry_guards_getinfo(const char *question, char **answer);
 void entry_guards_free_all(void);
 
@@ -1607,7 +1607,7 @@ extern uint64_t stats_n_destroy_cells_processed;
 /********************************* config.c ***************************/
 
 or_options_t *get_options(void);
-int set_options(or_options_t *new_val);
+int set_options(or_options_t *new_val, char **msg);
 void config_free_all(void);
 const char *safe_str(const char *address);
 const char *escaped_safe_str(const char *address);
@@ -1615,7 +1615,7 @@ const char *escaped_safe_str(const char *address);
 int config_get_lines(char *string, config_line_t **result);
 void config_free_lines(config_line_t *front);
 int options_trial_assign(config_line_t *list, int use_defaults,
-                         int clear_first);
+                         int clear_first, char **msg);
 int resolve_my_address(or_options_t *options, uint32_t *addr,
                        char **hostname_out);
 void options_init(or_options_t *options);
@@ -2097,7 +2097,7 @@ int rep_hist_get_predicted_internal(time_t now, int *need_uptime,
                                     int *need_capacity);
 
 void rep_hist_update_state(or_state_t *state);
-int rep_hist_load_state(or_state_t *state, const char **err);
+int rep_hist_load_state(or_state_t *state, char **err);
 
 void rep_hist_free_all(void);
 

src/or/rephist.c

@@ -694,7 +694,7 @@ rep_hist_update_state(or_state_t *state)
 /** Set bandwidth history from our saved state.
  */
 int
-rep_hist_load_state(or_state_t *state, const char **err)
+rep_hist_load_state(or_state_t *state, char **err)
 {
   time_t s_begins, start;
   time_t now = time(NULL);
@@ -742,8 +742,7 @@ rep_hist_load_state(or_state_t *state, const char **err)
   }
 
   if (!all_ok) {
-    if (err)
-      *err = "Parsing of bandwidth history values failed";
+    *err = tor_strdup("Parsing of bandwidth history values failed");
     /* and create fresh arrays */
     tor_free(read_array);
     tor_free(write_array);

src/or/test.c

@@ -1614,12 +1614,17 @@ int
 main(int c, char**v)
 {
   or_options_t *options = options_new();
+  char *errmsg = NULL;
   options->command = CMD_RUN_UNITTESTS;
   network_init();
   setup_directory();
   options_init(options);
   options->DataDirectory = tor_strdup(temp_dir);
-  set_options(options);
+  if (set_options(options, &errmsg) < 0) {
+    printf("Failed to set initial options: %s\n", errmsg);
+    tor_free(errmsg);
+    return 1;
+  }
 
   crypto_seed_rng();