Add the ability to append and clear linelist options from cmdline

This will be important for getting stuff to work right across zones.

6 files changed, 65 insertions, 20 deletions

diff --git a/changes/config b/changes/config
index 5e4039e761..08874eb8b6 100644
--- a/changes/config
+++ b/changes/config
@@ -7,6 +7,12 @@
       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.
+    - You can get the old (appending) command-line behavior for "list"
+      "list" options, by prefixing the option name with a "+".
+    - You can remove all the values for a "list" option from the command
+      line without adding any new ones by prefixing the option name
+      with a "/".
+
 
   o Minor bugfixes:
     - Restore behavior of overriding SocksPort, ORPort, and similar
diff --git a/src/or/config.c b/src/or/config.c
index f4cf3d010b..fa44793eea 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1728,6 +1728,9 @@ config_get_commandlines(int argc, char **argv, config_line_t **result)
   int i = 1;
 
   while (i < argc) {
+    unsigned command = CONFIG_LINE_NORMAL;
+    int want_arg = 1;
+
     if (!strcmp(argv[i],"-f") ||
         !strcmp(argv[i],"--hash-password")) {
       i += 2; /* command-line option with argument. ignore them. */
@@ -1745,13 +1748,6 @@ config_get_commandlines(int argc, char **argv, config_line_t **result)
       continue;
     }
 
-    if (i == argc-1) {
-      log_warn(LD_CONFIG,"Command-line option '%s' with no value. Failing.",
-               argv[i]);
-      config_free_lines(front);
-      return -1;
-    }
-
     *new = tor_malloc_zero(sizeof(config_line_t));
     s = argv[i];
 
@@ -1760,15 +1756,33 @@ config_get_commandlines(int argc, char **argv, config_line_t **result)
       s++;
     if (*s == '-')
       s++;
+    /* Figure out the command, if any. */
+    if (*s == '+') {
+      s++;
+      command = CONFIG_LINE_APPEND;
+    } else if (*s == '/') {
+      s++;
+      command = CONFIG_LINE_CLEAR;
+      /* A 'clear' command has no argument. */
+      want_arg = 0;
+    }
+
+    if (want_arg && i == argc-1) {
+      log_warn(LD_CONFIG,"Command-line option '%s' with no value. Failing.",
+               argv[i]);
+      config_free_lines(front);
+      return -1;
+    }
 
     (*new)->key = tor_strdup(expand_abbrev(&options_format, s, 1, 1));
-    (*new)->value = tor_strdup(argv[i+1]);
+    (*new)->value = want_arg ? tor_strdup(argv[i+1]) : tor_strdup("");
+    (*new)->command = command;
     (*new)->next = NULL;
     log(LOG_DEBUG, LD_CONFIG, "command line: parsed keyword '%s', value '%s'",
         (*new)->key, (*new)->value);
 
     new = &((*new)->next);
-    i += 2;
+    i += want_arg ? 2 : 1;
   }
   *result = front;
   return 0;
@@ -1796,9 +1810,12 @@ config_line_append(config_line_t **lst,
 /** Helper: parse the config string and strdup into key/value
  * strings. Set *result to the list, or NULL if parsing the string
  * failed.  Return 0 on success, -1 on failure. Warn and ignore any
- * misformatted lines. */
+ * misformatted lines.
+ *
+ * If <b>extended</b> is set, then treat keys beginning with / and with + as
+ * indicating "clear" and "append" respectively. */
 int
-config_get_lines(const char *string, config_line_t **result)
+config_get_lines(const char *string, config_line_t **result, int extended)
 {
   config_line_t *list = NULL, **next;
   char *k, *v;
@@ -1814,6 +1831,22 @@ config_get_lines(const char *string, config_line_t **result)
       return -1;
     }
     if (k && v) {
+      unsigned command = CONFIG_LINE_NORMAL;
+      if (extended) {
+        if (k[0] == '+') {
+          char *k_new = tor_strdup(k+1);
+          tor_free(k);
+          k = k_new;
+          command = CONFIG_LINE_APPEND;
+        } else if (k[0] == '/') {
+          char *k_new = tor_strdup(k+1);
+          tor_free(k);
+          k = k_new;
+          tor_free(v);
+          v = tor_strdup("");
+          command = CONFIG_LINE_CLEAR;
+        }
+      }
       /* 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. */
@@ -1821,6 +1854,7 @@ config_get_lines(const char *string, config_line_t **result)
       (*next)->key = k;
       (*next)->value = v;
       (*next)->next = NULL;
+      (*next)->command = command;
       next = &((*next)->next);
     } else {
       tor_free(k);
@@ -2140,8 +2174,9 @@ config_assign_line(const config_format_t *fmt, or_options_t *options,
   if (!strlen(c->value)) {
     /* reset or clear it, then return */
     if (!clear_first) {
-      if (var->type == CONFIG_TYPE_LINELIST ||
-          var->type == CONFIG_TYPE_LINELIST_S) {
+      if ((var->type == CONFIG_TYPE_LINELIST ||
+           var->type == CONFIG_TYPE_LINELIST_S) &&
+          c->command != CONFIG_LINE_CLEAR) {
         /* We got an empty linelist from the torrc or command line.
            As a special case, call this an error. Warn and ignore. */
         log_warn(LD_CONFIG,
@@ -2151,6 +2186,8 @@ config_assign_line(const config_format_t *fmt, or_options_t *options,
       }
     }
     return 0;
+  } else if (c->command == CONFIG_LINE_CLEAR && !clear_first) {
+    option_reset(fmt, options, var, use_defaults);
   }
 
   if (options_seen && (var->type != CONFIG_TYPE_LINELIST &&
@@ -4454,7 +4491,7 @@ options_init_from_string(const char *cf,
   newoptions->command_arg = command_arg;
 
   /* get config lines, assign them */
-  retval = config_get_lines(cf, &cl);
+  retval = config_get_lines(cf, &cl, 1);
   if (retval < 0) {
     err = SETOPT_ERR_PARSE;
     goto err;
@@ -4505,7 +4542,7 @@ options_init_from_string(const char *cf,
     newoptions->command_arg = command_arg;
 
     /* Assign all options a second time. */
-    retval = config_get_lines(cf, &cl);
+    retval = config_get_lines(cf, &cl, 1);
     if (retval < 0) {
       err = SETOPT_ERR_PARSE;
       goto err;
@@ -6190,7 +6227,7 @@ or_state_load(void)
   if (contents) {
     config_line_t *lines=NULL;
     int assign_retval;
-    if (config_get_lines(contents, &lines)<0)
+    if (config_get_lines(contents, &lines, 0)<0)
       goto done;
     assign_retval = config_assign(&state_format, new_state,
                                   lines, 0, 0, &errmsg);
diff --git a/src/or/config.h b/src/or/config.h
index 1e4d5678b6..73095de11e 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -23,7 +23,7 @@ const char *escaped_safe_str_client(const char *address);
 const char *escaped_safe_str(const char *address);
 const char *get_version(void);
 
-int config_get_lines(const char *string, config_line_t **result);
+int config_get_lines(const char *string, config_line_t **result, int extended);
 void config_free_lines(config_line_t *front);
 setopt_err_t options_trial_assign(config_line_t *list, int use_defaults,
                                   int clear_first, char **msg);
diff --git a/src/or/control.c b/src/or/control.c
index 109eb8857b..19904ddeb9 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -737,7 +737,7 @@ control_setconf_helper(control_connection_t *conn, uint32_t len, char *body,
   SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp));
   smartlist_free(entries);
 
-  if (config_get_lines(config, &lines) < 0) {
+  if (config_get_lines(config, &lines, 0) < 0) {
     log_warn(LD_CONTROL,"Controller gave us config lines we can't parse.");
     connection_write_str_to_buf("551 Couldn't parse configuration\r\n",
                                 conn);
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index be62459b16..8fe1b18a35 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -232,7 +232,7 @@ dirserv_load_fingerprint_file(void)
   }
   tor_free(fname);
 
-  result = config_get_lines(cf, &front);
+  result = config_get_lines(cf, &front, 0);
   tor_free(cf);
   if (result < 0) {
     log_warn(LD_CONFIG, "Error reading from fingerprint file");
diff --git a/src/or/or.h b/src/or/or.h
index b3fd082cb6..c0963b0eec 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2841,6 +2841,8 @@ typedef struct port_cfg_t {
 /** Appends to previous configuration for the same option, even if we
  * would ordinary replace it. */
 #define CONFIG_LINE_APPEND 1
+/* Removes all previous configuration for an option. */
+#define CONFIG_LINE_CLEAR 2
 
 /** A linked list of lines in a config file. */
 typedef struct config_line_t {
@@ -2848,7 +2850,7 @@ typedef struct config_line_t {
   char *value;
   struct config_line_t *next;
   /** What special treatment (if any) does this line require? */
-  unsigned int command:1;
+  unsigned int command:2;
   /** 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. */