[or-cvs] [tor/master 1/3] Warn when encounter the same (non-list) option twice in the same place

[nickm at torproject.org]

Author: Nick Mathewson <nickm at torproject.org>
Date: Sat, 31 Jul 2010 13:16:48 -0400
Subject: Warn when encounter the same (non-list) option twice in the same place
Commit: 39378bf1829dc9f5c9a1172442dcb8b9864ec25c

It's okay to get (say) a SocksPort line in the torrc, and then a
SocksPort on the command line to override it, and then a SocksPort via
a controller to override *that*.  But if there are two occurrences of
SocksPort in the torrc, or on the command line, or in a single SETCONF
command, then the user is likely confused.  Our old code would not
help unconfuse the user, but would instead silently ignore all but
the last occurrence.

This patch changes the behavior so that if the some option is passed
more than once to any torrc, command line, or SETCONF (each of which
coincidentally corresponds to a call to config_assign()), and the
option is not a type that allows multiple occurrences (LINELIST or
LINELIST_X), then we can warn the user.

This closes trac entry 1384.
---
 changes/bug1384 |    5 +++++
 src/or/config.c |   33 +++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 changes/bug1384

diff --git a/changes/bug1384 b/changes/bug1384
new file mode 100644
index 0000000..d3da65d
--- /dev/null
+++ b/changes/bug1384
@@ -0,0 +1,5 @@
+  o Minor features:
+    - Warn when the same option is provided more then once in a torrc file,
+      on the command line, or in a single SETCONF statement, and option
+      is one that only accepts a single value.
+
diff --git a/src/or/config.c b/src/or/config.c
index a4e4f89..34bad4c 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1560,6 +1560,16 @@ config_find_option(config_format_t *fmt, const char *key)
   return NULL;
 }
 
+/** Return the number of option entries in <b>fmt</b>. */
+static int
+config_count_options(config_format_t *fmt)
+{
+  int i;
+  for (i=0; fmt->vars[i].name; ++i)
+    ;
+  return i;
+}
+
 /*
  * Functions to assign config options.
  */
@@ -1704,7 +1714,7 @@ config_assign_value(config_format_t *fmt, or_options_t *options,
 static int
 config_assign_line(config_format_t *fmt, or_options_t *options,
                    config_line_t *c, int use_defaults,
-                   int clear_first, char **msg)
+                   int clear_first, bitarray_t *options_seen, char **msg)
 {
   config_var_t *var;
 
@@ -1724,6 +1734,7 @@ config_assign_line(config_format_t *fmt, or_options_t *options,
       return -1;
     }
   }
+
   /* Put keyword into canonical case. */
   if (strcmp(var->name, c->key)) {
     tor_free(c->key);
@@ -1746,6 +1757,18 @@ config_assign_line(config_format_t *fmt, or_options_t *options,
     return 0;
   }
 
+  if (options_seen && (var->type != CONFIG_TYPE_LINELIST &&
+                       var->type != CONFIG_TYPE_LINELIST_S)) {
+    /* We're tracking which options we've seen, and this option is not
+     * supposed to occur more than once. */
+    int var_index = var - fmt->vars;
+    if (bitarray_is_set(options_seen, var_index)) {
+      log_warn(LD_CONFIG, "Option '%s' used more than once; all but the last "
+               "value will be ignored.", var->name);
+    }
+    bitarray_set(options_seen, var_index);
+  }
+
   if (config_assign_value(fmt, options, c, msg) < 0)
     return -2;
   return 0;
@@ -2014,6 +2037,8 @@ config_assign(config_format_t *fmt, void *options, config_line_t *list,
               int use_defaults, int clear_first, char **msg)
 {
   config_line_t *p;
+  bitarray_t *options_seen;
+  const int n_options = config_count_options(fmt);
 
   CHECK(fmt, options);
 
@@ -2033,14 +2058,18 @@ config_assign(config_format_t *fmt, void *options, config_line_t *list,
       config_reset_line(fmt, options, p->key, use_defaults);
   }
 
+  options_seen = bitarray_init_zero(n_options);
   /* pass 3: assign. */
   while (list) {
     int r;
     if ((r=config_assign_line(fmt, options, list, use_defaults,
-                              clear_first, msg)))
+                              clear_first, options_seen, msg))) {
+      bitarray_free(options_seen);
       return r;
+    }
     list = list->next;
   }
+  bitarray_free(options_seen);
   return 0;
 }
 
-- 
1.7.1