Make linelists always overridden by the command line

This starts an effort to refactor torrc handling code to make it easier
to live with.  It makes it possible to override exit policies from the
command line, and possible to override (rather than append to) socksport
lists from the command line.

It'll be necessary to make a "base" torrc implementation work at all.

changes/config

@@ -0,0 +1,13 @@
+  o Minor features
+    - Slightly change behavior of "list" options (that is, options that
+      can appear more than once) when they appear both in torrc and on
+      the command line. Previously, the command-line options would be
+      appended to the ones from torrc. Now, the command-line options
+      override the torrc options entirely. This new behavior allows
+      the user to override list options (like exit policies and
+      ports to listen on) from the command line, rather than simply
+      appending to the list.
+
+  o Minor bugfixes:
+    - Restore behavior of overriding SocksPort, ORPort, and similar
+      options from the command line. Bugfix on 0.2.3.3-alpha.

src/or/config.c

@@ -1783,7 +1783,7 @@ config_line_append(config_line_t **lst,
 {
   config_line_t *newline;
 
-  newline = tor_malloc(sizeof(config_line_t));
+  newline = tor_malloc_zero(sizeof(config_line_t));
   newline->key = tor_strdup(key);
   newline->value = tor_strdup(val);
   newline->next = NULL;
@@ -1817,7 +1817,7 @@ config_get_lines(const char *string, config_line_t **result)
       /* This list can get long, so we keep a pointer to the end of it
        * rather than using config_line_append over and over and getting
        * n^2 performance. */
-      *next = tor_malloc(sizeof(config_line_t));
+      *next = tor_malloc_zero(sizeof(config_line_t));
       (*next)->key = k;
       (*next)->value = v;
       (*next)->next = NULL;
@@ -2048,7 +2048,19 @@ config_assign_value(const config_format_t *fmt, or_options_t *options,
 
   case CONFIG_TYPE_LINELIST:
   case CONFIG_TYPE_LINELIST_S:
+    {
+      config_line_t *lastval = *(config_line_t**)lvalue;
+      if (lastval && lastval->fragile) {
+        if (c->command != CONFIG_LINE_APPEND) {
+          config_free_lines(lastval);
+          *(config_line_t**)lvalue = NULL;
+        } else {
+          lastval->fragile = 0;
+        }
+      }
+
       config_line_append((config_line_t**)lvalue, c->key, c->value);
+    }
     break;
   case CONFIG_TYPE_OBSOLETE:
     log_warn(LD_CONFIG, "Skipping obsolete configuration option '%s'", c->key);
@@ -2064,6 +2076,28 @@ config_assign_value(const config_format_t *fmt, or_options_t *options,
   return 0;
 }
 
+/** Mark every linelist in <b>options<b> "fragile", so that fresh assignments
+ * to it will replace old ones. */
+static void
+config_mark_lists_fragile(const config_format_t *fmt, or_options_t *options)
+{
+  int i;
+  tor_assert(fmt);
+  tor_assert(options);
+
+  for (i = 0; fmt->vars[i].name; ++i) {
+    const config_var_t *var = &fmt->vars[i];
+    config_line_t *list;
+    if (var->type != CONFIG_TYPE_LINELIST &&
+        var->type != CONFIG_TYPE_LINELIST_V)
+      continue;
+
+    list = *(config_line_t **)STRUCT_VAR_P(options, var->var_offset);
+    if (list)
+      list->fragile = 1;
+  }
+}
+
 /** If <b>c</b> is a syntactically valid configuration line, update
  * <b>options</b> with its value and return 0.  Otherwise return -1 for bad
  * key, -2 for bad value.
@@ -2211,7 +2245,7 @@ config_lines_dup(const config_line_t *inp)
   config_line_t *result = NULL;
   config_line_t **next_out = &result;
   while (inp) {
-    *next_out = tor_malloc(sizeof(config_line_t));
+    *next_out = tor_malloc_zero(sizeof(config_line_t));
     (*next_out)->key = tor_strdup(inp->key);
     (*next_out)->value = tor_strdup(inp->value);
     inp = inp->next;
@@ -2448,6 +2482,12 @@ config_assign(const config_format_t *fmt, void *options, config_line_t *list,
     list = list->next;
   }
   bitarray_free(options_seen);
+
+  /** Now we're done assigning a group of options to the configuration.
+   * Subsequent group assignments should _replace_ linelists, not extend
+   * them. */
+  config_mark_lists_fragile(fmt, options);
+
   return 0;
 }
 

src/or/or.h

@@ -2836,11 +2836,23 @@ typedef struct port_cfg_t {
   char unix_addr[FLEXIBLE_ARRAY_MEMBER];
 } port_cfg_t;
 
+/** Ordinary configuration line. */
+#define CONFIG_LINE_NORMAL 0
+/** Appends to previous configuration for the same option, even if we
+ * would ordinary replace it. */
+#define CONFIG_LINE_APPEND 1
+
 /** A linked list of lines in a config file. */
 typedef struct config_line_t {
   char *key;
   char *value;
   struct config_line_t *next;
+  /** What special treatment (if any) does this line require? */
+  unsigned int command:1;
+  /** If true, subsequent assignments to this linelist should replace
+   * it, not extend it.  Set only on the first item in a linelist in an
+   * or_options_t. */
+  unsigned int fragile:1;
 } config_line_t;
 
 typedef struct routerset_t routerset_t;