From 39378bf1829dc9f5c9a1172442dcb8b9864ec25c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 31 Jul 2010 13:16:48 -0400 Subject: [PATCH 1/3] Warn when encounter the same (non-list) option twice in the same place 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 0000000000..d3da65d45c --- /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 a4e4f89c1a..34bad4c6b2 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 fmt. */ +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; } From 883af78a9d536562345d2713edebb13f8978a72a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 31 Jul 2010 13:23:41 -0400 Subject: [PATCH 2/3] Clarify that cmd-line options override torrc options --- doc/tor.1.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 222aaf103c..db623c8da9 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -64,7 +64,8 @@ OPTIONS Other options can be specified either on the command-line (--option value), or in the configuration file (option value or option "value"). Options are case-insensitive. C-style escaped characters are allowed inside - quoted values. + quoted values. Options on the command line take precedence over + options found in the configuration file. **BandwidthRate** __N__ **bytes**|**KB**|**MB**|**GB**:: A token bucket limits the average incoming bandwidth usage on this node to From a9d055c5c516924b2e87e416b154bf220b0a914e Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Mon, 2 Aug 2010 18:44:00 +0200 Subject: [PATCH 3/3] Fix a compile warning on OS X 10.6 Also update the changes file to contain a note on which bug was fixed by this. --- changes/bug1384 | 2 +- src/or/config.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changes/bug1384 b/changes/bug1384 index d3da65d45c..9c406d9e25 100644 --- a/changes/bug1384 +++ b/changes/bug1384 @@ -1,5 +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. + is one that only accepts a single value. Closes bug 1384. diff --git a/src/or/config.c b/src/or/config.c index 34bad4c6b2..0a26c84be7 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1761,7 +1761,7 @@ config_assign_line(config_format_t *fmt, or_options_t *options, 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; + int var_index = (int)(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);