Browse Source

Merge branch 'tor-github/pr/1254'

David Goulet 4 years ago
parent
commit
24bc2cd7b5

+ 6 - 0
changes/ticket30935

@@ -0,0 +1,6 @@
+  o Code simplification and refactoring:
+    - Numerous simplifications in configuration-handling logic:
+      remove duplicated macro definitions, replace magical names
+      with flags, and refactor "TestingTorNetwork" to use the
+      same default-option logic as the rest of Tor.
+      Closes ticket 30935.

+ 1 - 1
scripts/maint/practracker/exceptions.txt

@@ -30,7 +30,7 @@
 # Remember: It is better to fix the problem than to add a new exception!
 
 problem file-size /src/app/config/config.c 8518
-problem include-count /src/app/config/config.c 88
+problem include-count /src/app/config/config.c 89
 problem function-size /src/app/config/config.c:options_act_reversible() 296
 problem function-size /src/app/config/config.c:options_act() 589
 problem function-size /src/app/config/config.c:resolve_my_address() 190

+ 100 - 160
src/app/config/config.c

@@ -191,7 +191,7 @@ static const char unix_q_socket_prefix[] = "unix:\"";
 
 /** A list of abbreviations and aliases to map command-line options, obsolete
  * option names, or alternative option names, to their current values. */
-static config_abbrev_t option_abbrevs_[] = {
+static const config_abbrev_t option_abbrevs_[] = {
   PLURAL(AuthDirBadDirCC),
   PLURAL(AuthDirBadExitCC),
   PLURAL(AuthDirInvalidCC),
@@ -259,28 +259,19 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
  * or_options_t.<b>member</b>"
  */
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_ ## conftype,                               \
-        .offset = offsetof(or_options_t, member),                       \
-        },                                                              \
-      initvalue CONF_TEST_MEMBERS(or_options_t, conftype, member) }
-
-#ifdef TOR_UNIT_TESTS
-#define DUMMY_TEST_MEMBERS , {.INT=NULL}
-#else
-#define DUMMY_TEST_MEMBERS
-#endif
+  CONFIG_VAR_ETYPE(or_options_t, varname, conftype, member, 0, initvalue)
 
 /* As VAR, but uses a type definition in addition to a type enum. */
 #define VAR_D(varname,conftype,member,initvalue)                        \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_ ## conftype,                               \
-        .type_def = &conftype ## _type_defn,                            \
-        .offset = offsetof(or_options_t, member),                       \
-        },                                                              \
-      initvalue DUMMY_TEST_MEMBERS }
-
-/** As VAR, but the option name and member name are the same. */
+  CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, 0, initvalue)
+
+#define VAR_NODUMP(varname,conftype,member,initvalue)             \
+  CONFIG_VAR_ETYPE(or_options_t, varname, conftype, member,       \
+                   CVFLAG_NODUMP, initvalue)
+#define VAR_INVIS(varname,conftype,member,initvalue)              \
+  CONFIG_VAR_ETYPE(or_options_t, varname, conftype, member,       \
+                   CVFLAG_NODUMP|CVFLAG_INVISIBLE, initvalue)
+
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
@@ -289,9 +280,7 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
   VAR_D(#member, type, member, initvalue)
 
 /** An entry for config_vars: "The option <b>varname</b> is obsolete." */
-#define OBSOLETE(varname) \
-  { { .name = varname, .type = CONFIG_TYPE_OBSOLETE, }, NULL   \
-    DUMMY_TEST_MEMBERS }
+#define OBSOLETE(varname) CONFIG_VAR_OBSOLETE(varname)
 
 /**
  * Macro to declare *Port options.  Each one comes in three entries.
@@ -303,7 +292,7 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
 #define VPORT(member)                                           \
   VAR(#member "Lines", LINELIST_V, member ## _lines, NULL),     \
   VAR(#member, LINELIST_S, member ## _lines, NULL),             \
-  VAR("__" #member, LINELIST_S, member ## _lines, NULL)
+  VAR_NODUMP("__" #member, LINELIST_S, member ## _lines, NULL)
 
 /** UINT64_MAX as a decimal string */
 #define UINT64_MAX_STRING "18446744073709551615"
@@ -312,7 +301,7 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
  * abbreviations, order is significant, since the first matching option will
  * be chosen first.
  */
-static config_var_t option_vars_[] = {
+static const config_var_t option_vars_[] = {
   V(AccountingMax,               MEMUNIT,  "0 bytes"),
   VAR("AccountingRule",          STRING,   AccountingRule_option,  "max"),
   V(AccountingStart,             STRING,   NULL),
@@ -700,15 +689,17 @@ static config_var_t option_vars_[] = {
   V(WarnPlaintextPorts,          CSV,      "23,109,110,143"),
   OBSOLETE("UseFilteringSSLBufferevents"),
   OBSOLETE("__UseFilteringSSLBufferevents"),
-  VAR("__ReloadTorrcOnSIGHUP",   BOOL,  ReloadTorrcOnSIGHUP,      "1"),
-  VAR("__AllDirActionsPrivate",  BOOL,  AllDirActionsPrivate,     "0"),
-  VAR("__DisablePredictedCircuits",BOOL,DisablePredictedCircuits, "0"),
-  VAR("__DisableSignalHandlers", BOOL,  DisableSignalHandlers,    "0"),
-  VAR("__LeaveStreamsUnattached",BOOL,  LeaveStreamsUnattached,   "0"),
-  VAR("__HashedControlSessionPassword", LINELIST, HashedControlSessionPassword,
+  VAR_NODUMP("__ReloadTorrcOnSIGHUP",   BOOL,  ReloadTorrcOnSIGHUP,      "1"),
+  VAR_NODUMP("__AllDirActionsPrivate",  BOOL,  AllDirActionsPrivate,     "0"),
+  VAR_NODUMP("__DisablePredictedCircuits",BOOL,DisablePredictedCircuits, "0"),
+  VAR_NODUMP("__DisableSignalHandlers", BOOL,  DisableSignalHandlers,    "0"),
+  VAR_NODUMP("__LeaveStreamsUnattached",BOOL,  LeaveStreamsUnattached,   "0"),
+  VAR_NODUMP("__HashedControlSessionPassword", LINELIST,
+             HashedControlSessionPassword,
       NULL),
-  VAR("__OwningControllerProcess",STRING,OwningControllerProcess, NULL),
-  VAR("__OwningControllerFD", UINT64, OwningControllerFD, UINT64_MAX_STRING),
+  VAR_NODUMP("__OwningControllerProcess",STRING,OwningControllerProcess, NULL),
+  VAR_NODUMP("__OwningControllerFD", UINT64, OwningControllerFD,
+             UINT64_MAX_STRING),
   V(MinUptimeHidServDirectoryV2, INTERVAL, "96 hours"),
   V(TestingServerDownloadInitialDelay, CSV_INTERVAL, "0"),
   V(TestingClientDownloadInitialDelay, CSV_INTERVAL, "0"),
@@ -761,50 +752,34 @@ static config_var_t option_vars_[] = {
   V(TestingDirAuthVoteGuardIsStrict,  BOOL,     "0"),
   V_D(TestingDirAuthVoteHSDir, ROUTERSET, NULL),
   V(TestingDirAuthVoteHSDirIsStrict,  BOOL,     "0"),
-  VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "0"),
+  VAR_INVIS("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_,
+            "0"),
 
   END_OF_CONFIG_VARS
 };
 
+/** List of default directory authorities */
+static const char *default_authorities[] = {
+#include "auth_dirs.inc"
+  NULL
+};
+
+/** List of fallback directory authorities. The list is generated by opt-in of
+ * relays that meet certain stability criteria.
+ */
+static const char *default_fallbacks[] = {
+#include "fallback_dirs.inc"
+  NULL
+};
+
 /** Override default values with these if the user sets the TestingTorNetwork
  * option. */
-static const config_var_t testing_tor_network_defaults[] = {
-  V(DirAllowPrivateAddresses,    BOOL,     "1"),
-  V(EnforceDistinctSubnets,      BOOL,     "0"),
-  V(AssumeReachable,             BOOL,     "1"),
-  V(AuthDirMaxServersPerAddr,    POSINT,     "0"),
-  V(ClientBootstrapConsensusAuthorityDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(ClientBootstrapConsensusFallbackDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(ClientBootstrapConsensusAuthorityOnlyDownloadInitialDelay, CSV_INTERVAL,
-    "0"),
-  V(ClientDNSRejectInternalAddresses, BOOL,"0"),
-  V(ClientRejectInternalAddresses, BOOL,   "0"),
-  V(CountPrivateBandwidth,       BOOL,     "1"),
-  V(ExitPolicyRejectPrivate,     BOOL,     "0"),
-  V(ExtendAllowPrivateAddresses, BOOL,     "1"),
-  V(V3AuthVotingInterval,        INTERVAL, "5 minutes"),
-  V(V3AuthVoteDelay,             INTERVAL, "20 seconds"),
-  V(V3AuthDistDelay,             INTERVAL, "20 seconds"),
-  V(TestingV3AuthInitialVotingInterval, INTERVAL, "150 seconds"),
-  V(TestingV3AuthInitialVoteDelay, INTERVAL, "20 seconds"),
-  V(TestingV3AuthInitialDistDelay, INTERVAL, "20 seconds"),
-  V(TestingAuthDirTimeToLearnReachability, INTERVAL, "0 minutes"),
-  V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "0 minutes"),
-  V(MinUptimeHidServDirectoryV2, INTERVAL, "0 minutes"),
-  V(TestingServerDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(TestingClientDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(TestingServerConsensusDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(TestingClientConsensusDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(TestingBridgeDownloadInitialDelay, CSV_INTERVAL, "10"),
-  V(TestingBridgeBootstrapDownloadInitialDelay, CSV_INTERVAL, "0"),
-  V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "5 seconds"),
-  V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"),
-  V(TestingEnableConnBwEvent,    BOOL,     "1"),
-  V(TestingEnableCellStatsEvent, BOOL,     "1"),
-  VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "1"),
-  V(RendPostPeriod,              INTERVAL, "2 minutes"),
-
-  END_OF_CONFIG_VARS
+static const struct {
+  const char *k;
+  const char *v;
+} testing_tor_network_defaults[] = {
+#include "testnet.inc"
+  { NULL, NULL }
 };
 
 #undef VAR
@@ -876,7 +851,7 @@ static void set_protocol_warning_severity_level(int warning_severity);
 #define OR_OPTIONS_MAGIC 9090909
 
 /** Configuration format for or_options_t. */
-STATIC config_format_t options_format = {
+STATIC const config_format_t options_format = {
   sizeof(or_options_t),
   {
    "or_options_t",
@@ -943,6 +918,32 @@ get_options,(void))
   return get_options_mutable();
 }
 
+/**
+ * True iff we have noticed that this is a testing tor network, and we
+ * should use the corresponding defaults.
+ **/
+static bool testing_network_configured = false;
+
+/** Return a set of lines for any default options that we want to override
+ * from those set in our config_var_t values. */
+static config_line_t *
+get_options_defaults(void)
+{
+  int i;
+  config_line_t *result = NULL, **next = &result;
+
+  if (testing_network_configured) {
+    for (i = 0; testing_tor_network_defaults[i].k; ++i) {
+      config_line_append(next,
+                         testing_tor_network_defaults[i].k,
+                         testing_tor_network_defaults[i].v);
+      next = &(*next)->next;
+    }
+  }
+
+  return result;
+}
+
 /** Change the current global options to contain <b>new_val</b> instead of
  * their current value; take action based on the new value; free the old value
  * as necessary.  Returns 0 on success, -1 on failure.
@@ -978,8 +979,8 @@ set_options(or_options_t *new_val, char **msg)
     for (i=0; options_format.vars[i].member.name; ++i) {
       const config_var_t *var = &options_format.vars[i];
       const char *var_name = var->member.name;
-      if (var->member.type == CONFIG_TYPE_LINELIST_S ||
-          var->member.type == CONFIG_TYPE_OBSOLETE) {
+      if (config_var_is_contained(var)) {
+        /* something else will check this var, or it doesn't need checking */
         continue;
       }
       if (!config_is_same(&options_format, new_val, old_options, var_name)) {
@@ -1192,21 +1193,6 @@ cleanup_protocol_warning_severity_level(void)
    atomic_counter_destroy(&protocol_warning_severity_level);
 }
 
-/** List of default directory authorities */
-
-static const char *default_authorities[] = {
-#include "auth_dirs.inc"
-  NULL
-};
-
-/** List of fallback directory authorities. The list is generated by opt-in of
- * relays that meet certain stability criteria.
- */
-static const char *default_fallbacks[] = {
-#include "fallback_dirs.inc"
-  NULL
-};
-
 /** Add the default directory authorities directly into the trusted dir list,
  * but only add them insofar as they share bits with <b>type</b>.
  * Each authority's bits are restricted to the bits shared with <b>type</b>.
@@ -2681,9 +2667,10 @@ list_torrc_options(void)
   int i;
   for (i = 0; option_vars_[i].member.name; ++i) {
     const config_var_t *var = &option_vars_[i];
-    if (var->member.type == CONFIG_TYPE_OBSOLETE ||
-        var->member.type == CONFIG_TYPE_LINELIST_V)
+    if (! config_var_is_settable(var)) {
+      /* This variable cannot be set, or cannot be set by this name. */
       continue;
+    }
     printf("%s\n", var->member.name);
   }
 }
@@ -3014,6 +3001,16 @@ void
 options_init(or_options_t *options)
 {
   config_init(&options_format, options);
+  config_line_t *dflts = get_options_defaults();
+  char *msg=NULL;
+  if (config_assign(&options_format, options, dflts,
+                    CAL_WARN_DEPRECATIONS, &msg)<0) {
+    log_err(LD_BUG, "Unable to set default options: %s", msg);
+    tor_free(msg);
+    tor_assert_unreached();
+  }
+  config_free_lines(dflts);
+  tor_free(msg);
 }
 
 /** Return a string containing a possible configuration file that would give
@@ -5403,6 +5400,7 @@ options_init_from_string(const char *cf_defaults, const char *cf,
                          int command, const char *command_arg,
                          char **msg)
 {
+  bool retry = false;
   or_options_t *oldoptions, *newoptions, *newdefaultoptions=NULL;
   config_line_t *cl;
   int retval;
@@ -5460,73 +5458,12 @@ options_init_from_string(const char *cf_defaults, const char *cf,
   newoptions->FilesOpenedByIncludes = opened_files;
 
   /* If this is a testing network configuration, change defaults
-   * for a list of dependent config options, re-initialize newoptions
-   * with the new defaults, and assign all options to it second time. */
-  if (newoptions->TestingTorNetwork) {
-    /* XXXX this is a bit of a kludge.  perhaps there's a better way to do
-     * this?  We could, for example, make the parsing algorithm do two passes
-     * over the configuration.  If it finds any "suite" options like
-     * TestingTorNetwork, it could change the defaults before its second pass.
-     * Not urgent so long as this seems to work, but at any sign of trouble,
-     * let's clean it up.  -NM */
-
-    /* Change defaults. */
-    for (int i = 0; testing_tor_network_defaults[i].member.name; ++i) {
-      const config_var_t *new_var = &testing_tor_network_defaults[i];
-      config_var_t *old_var =
-        config_find_option_mutable(&options_format, new_var->member.name);
-      tor_assert(new_var);
-      tor_assert(old_var);
-      old_var->initvalue = new_var->initvalue;
-
-      if ((config_find_deprecation(&options_format, new_var->member.name))) {
-        log_warn(LD_GENERAL, "Testing options override the deprecated "
-                 "option %s. Is that intentional?",
-                 new_var->member.name);
-      }
-    }
-
-    /* Clear newoptions and re-initialize them with new defaults. */
-    or_options_free(newoptions);
-    or_options_free(newdefaultoptions);
-    newdefaultoptions = NULL;
-    newoptions = tor_malloc_zero(sizeof(or_options_t));
-    newoptions->magic_ = OR_OPTIONS_MAGIC;
-    options_init(newoptions);
-    newoptions->command = command;
-    newoptions->command_arg = command_arg ? tor_strdup(command_arg) : NULL;
-
-    /* Assign all options a second time. */
-    opened_files = smartlist_new();
-    for (int i = 0; i < 2; ++i) {
-      const char *body = i==0 ? cf_defaults : cf;
-      if (!body)
-        continue;
-
-      /* get config lines, assign them */
-      retval = config_get_lines_include(body, &cl, 1,
-                                        body == cf ? &cf_has_include : NULL,
-                                        opened_files);
-      if (retval < 0) {
-        err = SETOPT_ERR_PARSE;
-        goto err;
-      }
-      retval = config_assign(&options_format, newoptions, cl, 0, msg);
-      config_free_lines(cl);
-      if (retval < 0) {
-        err = SETOPT_ERR_PARSE;
-        goto err;
-      }
-      if (i==0)
-        newdefaultoptions = config_dup(&options_format, newoptions);
-    }
-    /* Assign command-line variables a second time too */
-    retval = config_assign(&options_format, newoptions,
-                           global_cmdline_options, 0, msg);
-    if (retval < 0) {
-      err = SETOPT_ERR_PARSE;
-      goto err;
-    }
+   * for a list of dependent config options, and try this function again. */
+  if (newoptions->TestingTorNetwork && ! testing_network_configured) {
+    // retry with the testing defaults.
+    testing_network_configured = true;
+    retry = true;
+    goto err;
   }
 
   newoptions->IncludeUsed = cf_has_include;
@@ -5571,6 +5508,9 @@ options_init_from_string(const char *cf_defaults, const char *cf,
     tor_asprintf(msg, "Failed to parse/validate config: %s", old_msg);
     tor_free(old_msg);
   }
+  if (retry)
+    return options_init_from_string(cf_defaults, cf, command, command_arg,
+                                    msg);
   return err;
 }
 
@@ -8196,8 +8136,8 @@ getinfo_helper_config(control_connection_t *conn,
     int i;
     for (i = 0; option_vars_[i].member.name; ++i) {
       const config_var_t *var = &option_vars_[i];
-      /* don't tell controller about triple-underscore options */
-      if (!strncmp(option_vars_[i].member.name, "___", 3))
+      /* don't tell controller about invisible options */
+      if (config_var_is_invisible(var))
         continue;
       const char *type = struct_var_get_typename(&var->member);
       if (!type)

+ 1 - 1
src/app/config/config.h

@@ -248,7 +248,7 @@ int options_any_client_port_set(const or_options_t *options);
 
 STATIC int options_act(const or_options_t *old_options);
 #ifdef TOR_UNIT_TESTS
-extern struct config_format_t options_format;
+extern const struct config_format_t options_format;
 #endif
 
 STATIC port_cfg_t *port_cfg_new(size_t namelen);

+ 54 - 36
src/app/config/confparse.c

@@ -22,14 +22,20 @@
  */
 
 #define CONFPARSE_PRIVATE
-#include "core/or/or.h"
+#include "orconfig.h"
 #include "app/config/confparse.h"
-#include "feature/nodelist/routerset.h"
 
+#include "lib/confmgt/structvar.h"
 #include "lib/confmgt/unitparse.h"
 #include "lib/container/bitarray.h"
+#include "lib/container/smartlist.h"
 #include "lib/encoding/confline.h"
-#include "lib/confmgt/structvar.h"
+#include "lib/log/escape.h"
+#include "lib/log/log.h"
+#include "lib/log/util_bug.h"
+#include "lib/string/compat_ctype.h"
+#include "lib/string/printf.h"
+#include "lib/string/util_string.h"
 
 static void config_reset(const config_format_t *fmt, void *options,
                          const config_var_t *var, int use_defaults);
@@ -100,9 +106,13 @@ config_find_deprecation(const config_format_t *fmt, const char *key)
   return NULL;
 }
 
-/** As config_find_option, but return a non-const pointer. */
-config_var_t *
-config_find_option_mutable(config_format_t *fmt, const char *key)
+/** If <b>key</b> is a configuration option, return the corresponding const
+ * config_var_t.  Otherwise, if <b>key</b> is a non-standard abbreviation,
+ * warn, and return the corresponding const config_var_t.  Otherwise return
+ * NULL.
+ */
+const config_var_t *
+config_find_option(const config_format_t *fmt, const char *key)
 {
   int i;
   size_t keylen = strlen(key);
@@ -127,17 +137,6 @@ config_find_option_mutable(config_format_t *fmt, const char *key)
   return NULL;
 }
 
-/** If <b>key</b> is a configuration option, return the corresponding const
- * config_var_t.  Otherwise, if <b>key</b> is a non-standard abbreviation,
- * warn, and return the corresponding const config_var_t.  Otherwise return
- * NULL.
- */
-const config_var_t *
-config_find_option(const config_format_t *fmt, const char *key)
-{
-  return config_find_option_mutable((config_format_t*)fmt, key);
-}
-
 /** Return the number of option entries in <b>fmt</b>. */
 static int
 config_count_options(const config_format_t *fmt)
@@ -148,6 +147,34 @@ config_count_options(const config_format_t *fmt)
   return i;
 }
 
+bool
+config_var_is_cumulative(const config_var_t *var)
+{
+  return struct_var_is_cumulative(&var->member);
+}
+bool
+config_var_is_settable(const config_var_t *var)
+{
+  if (var->flags & CVFLAG_OBSOLETE)
+    return false;
+  return struct_var_is_settable(&var->member);
+}
+bool
+config_var_is_contained(const config_var_t *var)
+{
+  return struct_var_is_contained(&var->member);
+}
+bool
+config_var_is_invisible(const config_var_t *var)
+{
+  return (var->flags & CVFLAG_INVISIBLE) != 0;
+}
+bool
+config_var_is_dumpable(const config_var_t *var)
+{
+  return (var->flags & CVFLAG_NODUMP) == 0;
+}
+
 /*
  * Functions to assign config options.
  */
@@ -183,14 +210,7 @@ config_mark_lists_fragile(const config_format_t *fmt, void *options)
 
   for (i = 0; fmt->vars[i].member.name; ++i) {
     const config_var_t *var = &fmt->vars[i];
-    config_line_t *list;
-    if (var->member.type != CONFIG_TYPE_LINELIST &&
-        var->member.type != CONFIG_TYPE_LINELIST_V)
-      continue;
-
-    list = *(config_line_t **)STRUCT_VAR_P(options, var->member.offset);
-    if (list)
-      list->fragile = 1;
+    struct_var_mark_fragile(options, &var->member);
   }
 }
 
@@ -255,9 +275,7 @@ config_assign_line(const config_format_t *fmt, void *options,
   if (!strlen(c->value)) {
     /* reset or clear it, then return */
     if (!clear_first) {
-      if ((var->member.type == CONFIG_TYPE_LINELIST ||
-           var->member.type == CONFIG_TYPE_LINELIST_S) &&
-          c->command != CONFIG_LINE_CLEAR) {
+      if (config_var_is_cumulative(var) && 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,
@@ -273,8 +291,7 @@ config_assign_line(const config_format_t *fmt, void *options,
     config_reset(fmt, options, var, use_defaults); // LCOV_EXCL_LINE
   }
 
-  if (options_seen && (var->member.type != CONFIG_TYPE_LINELIST &&
-                       var->member.type != CONFIG_TYPE_LINELIST_S)) {
+  if (options_seen && ! config_var_is_cumulative(var)) {
     /* We're tracking which options we've seen, and this option is not
      * supposed to occur more than once. */
     int var_index = (int)(var - fmt->vars);
@@ -562,10 +579,10 @@ config_dup(const config_format_t *fmt, const void *old)
 
   newopts = config_new(fmt);
   for (i=0; fmt->vars[i].member.name; ++i) {
-    if (fmt->vars[i].member.type == CONFIG_TYPE_LINELIST_S)
-      continue;
-    if (fmt->vars[i].member.type == CONFIG_TYPE_OBSOLETE)
+    if (config_var_is_contained(&fmt->vars[i])) {
+      // Something else will copy this option, or it doesn't need copying.
       continue;
+    }
     if (struct_var_copy(newopts, old, &fmt->vars[i].member) < 0) {
       // LCOV_EXCL_START
       log_err(LD_BUG, "Unable to copy value for %s.",
@@ -629,11 +646,12 @@ config_dump(const config_format_t *fmt, const void *default_options,
   elements = smartlist_new();
   for (i=0; fmt->vars[i].member.name; ++i) {
     int comment_option = 0;
-    if (fmt->vars[i].member.type == CONFIG_TYPE_OBSOLETE ||
-        fmt->vars[i].member.type == CONFIG_TYPE_LINELIST_S)
+    if (config_var_is_contained(&fmt->vars[i])) {
+      // Something else will dump this option, or it doesn't need dumping.
       continue;
+    }
     /* Don't save 'hidden' control variables. */
-    if (!strcmpstart(fmt->vars[i].member.name, "__"))
+    if (! config_var_is_dumpable(&fmt->vars[i]))
       continue;
     if (minimal && config_is_same(fmt, options, defaults,
                                   fmt->vars[i].member.name))

+ 14 - 58
src/app/config/confparse.h

@@ -14,6 +14,8 @@
 #define TOR_CONFPARSE_H
 
 #include "lib/conf/conftypes.h"
+#include "lib/conf/confmacros.h"
+#include "lib/testsupport/testsupport.h"
 
 /** An abbreviation for a configuration option allowed on the command line. */
 typedef struct config_abbrev_t {
@@ -32,57 +34,6 @@ typedef struct config_deprecation_t {
  * you can abbreviate <b>tok</b>s as <b>tok</b>". */
 #define PLURAL(tok) { #tok, #tok "s", 0, 0 }
 
-/** A variable allowed in the configuration file or on the command line. */
-typedef struct config_var_t {
-  struct_member_t member; /** A struct member corresponding to this
-                           * variable. */
-  const char *initvalue; /**< String (or null) describing initial value. */
-
-#ifdef TOR_UNIT_TESTS
-  /** Used for compiler-magic to typecheck the corresponding field in the
-   * corresponding struct. Only used in unit test mode, at compile-time. */
-  confparse_dummy_values_t var_ptr_dummy;
-#endif
-} config_var_t;
-
-/* Macros to define extra members inside config_var_t fields, and at the
- * end of a list of them.
- */
-#ifdef TOR_UNIT_TESTS
-/* This is a somewhat magic type-checking macro for users of confparse.c.
- * It initializes a union member "confparse_dummy_values_t.conftype" with
- * the address of a static member "tp_dummy.member".   This
- * will give a compiler warning unless the member field is of the correct
- * type.
- *
- * (This warning is mandatory, because a type mismatch here violates the type
- * compatibility constraint for simple assignment, and requires a diagnostic,
- * according to the C spec.)
- *
- * For example, suppose you say:
- *     "CONF_CHECK_VAR_TYPE(or_options_t, STRING, Address)".
- * Then this macro will evaluate to:
- *     { .STRING = &or_options_t_dummy.Address }
- * And since confparse_dummy_values_t.STRING has type "char **", that
- * expression will create a warning unless or_options_t.Address also
- * has type "char *".
- */
-#define CONF_CHECK_VAR_TYPE(tp, conftype, member)       \
-  { . conftype = &tp ## _dummy . member }
-#define CONF_TEST_MEMBERS(tp, conftype, member) \
-  , CONF_CHECK_VAR_TYPE(tp, conftype, member)
-#define END_OF_CONFIG_VARS                                      \
-  { { .name = NULL }, NULL, { .INT=NULL } }
-#define DUMMY_TYPECHECK_INSTANCE(tp)            \
-  static tp tp ## _dummy
-#else /* !(defined(TOR_UNIT_TESTS)) */
-#define CONF_TEST_MEMBERS(tp, conftype, member)
-#define END_OF_CONFIG_VARS { { .name = NULL, }, NULL }
-/* Repeatedly declarable incomplete struct to absorb redundant semicolons */
-#define DUMMY_TYPECHECK_INSTANCE(tp)            \
-  struct tor_semicolon_eater
-#endif /* defined(TOR_UNIT_TESTS) */
-
 /** Type of a callback to validate whether a given configuration is
  * well-formed and consistent. See options_trial_assign() for documentation
  * of arguments. */
@@ -97,16 +48,17 @@ typedef void (*free_cfg_fn_t)(void*);
 typedef struct config_format_t {
   size_t size; /**< Size of the struct that everything gets parsed into. */
   struct_magic_decl_t magic; /**< Magic number info for this struct. */
-  config_abbrev_t *abbrevs; /**< List of abbreviations that we expand when
-                             * parsing this format. */
+  const config_abbrev_t *abbrevs; /**< List of abbreviations that we expand
+                             * when parsing this format. */
   const config_deprecation_t *deprecations; /** List of deprecated options */
-  config_var_t *vars; /**< List of variables we recognize, their default
-                       * values, and where we stick them in the structure. */
+  const config_var_t *vars; /**< List of variables we recognize, their default
+                             * values, and where we stick them in the
+                             * structure. */
   validate_fn_t validate_fn; /**< Function to validate config. */
   free_cfg_fn_t free_fn; /**< Function to free the configuration. */
   /** If present, extra denotes a LINELIST variable for unrecognized
    * lines.  Otherwise, unrecognized lines are an error. */
-  struct_member_t *extra;
+  const struct_member_t *extra;
 } config_format_t;
 
 /** Macro: assert that <b>cfg</b> has the right magic field for format
@@ -143,8 +95,6 @@ bool config_check_ok(const config_format_t *fmt, const void *options,
 int config_assign(const config_format_t *fmt, void *options,
                   struct config_line_t *list,
                   unsigned flags, char **msg);
-config_var_t *config_find_option_mutable(config_format_t *fmt,
-                                         const char *key);
 const char *config_find_deprecation(const config_format_t *fmt,
                                      const char *key);
 const config_var_t *config_find_option(const config_format_t *fmt,
@@ -154,6 +104,12 @@ const char *config_expand_abbrev(const config_format_t *fmt,
                                  int command_line, int warn_obsolete);
 void warn_deprecated_option(const char *what, const char *why);
 
+bool config_var_is_cumulative(const config_var_t *var);
+bool config_var_is_settable(const config_var_t *var);
+bool config_var_is_contained(const config_var_t *var);
+bool config_var_is_invisible(const config_var_t *var);
+bool config_var_is_dumpable(const config_var_t *var);
+
 /* Helper macros to compare an option across two configuration objects */
 #define CFG_EQ_BOOL(a,b,opt) ((a)->opt == (b)->opt)
 #define CFG_EQ_INT(a,b,opt) ((a)->opt == (b)->opt)

+ 3 - 8
src/app/config/statefile.c

@@ -70,18 +70,13 @@ static config_abbrev_t state_abbrevs_[] = {
  * members with CONF_CHECK_VAR_TYPE. */
 DUMMY_TYPECHECK_INSTANCE(or_state_t);
 
-/*XXXX these next two are duplicates or near-duplicates from config.c */
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-      .type = CONFIG_TYPE_ ## conftype,                                 \
-      .offset = offsetof(or_state_t, member), },                        \
-      initvalue CONF_TEST_MEMBERS(or_state_t, conftype, member) }
-/** As VAR, but the option name and member name are the same. */
-#define V(member,conftype,initvalue)                                    \
+  CONFIG_VAR_ETYPE(or_state_t, varname, conftype, member, 0, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
 /** Array of "state" variables saved to the ~/.tor/state file. */
-static config_var_t state_vars_[] = {
+static const config_var_t state_vars_[] = {
   /* Remember to document these in state-contents.txt ! */
 
   V(AccountingBytesReadInInterval,    MEMUNIT,  NULL),

+ 33 - 0
src/app/config/testnet.inc

@@ -0,0 +1,33 @@
+{ "DirAllowPrivateAddresses", "1" },
+{ "EnforceDistinctSubnets", "0" },
+{ "AssumeReachable", "1" },
+{ "AuthDirMaxServersPerAddr", "0" },
+{ "ClientBootstrapConsensusAuthorityDownloadInitialDelay", "0" },
+{ "ClientBootstrapConsensusFallbackDownloadInitialDelay", "0" },
+{ "ClientBootstrapConsensusAuthorityOnlyDownloadInitialDelay", "0" },
+{ "ClientDNSRejectInternalAddresses", "0" },
+{ "ClientRejectInternalAddresses", "0" },
+{ "CountPrivateBandwidth", "1" },
+{ "ExitPolicyRejectPrivate", "0" },
+{ "ExtendAllowPrivateAddresses", "1" },
+{ "V3AuthVotingInterval", "5 minutes" },
+{ "V3AuthVoteDelay", "20 seconds" },
+{ "V3AuthDistDelay", "20 seconds" },
+{ "TestingV3AuthInitialVotingInterval", "150 seconds" },
+{ "TestingV3AuthInitialVoteDelay", "20 seconds" },
+{ "TestingV3AuthInitialDistDelay", "20 seconds" },
+{ "TestingAuthDirTimeToLearnReachability", "0 minutes" },
+{ "TestingEstimatedDescriptorPropagationTime", "0 minutes" },
+{ "MinUptimeHidServDirectoryV2", "0 minutes" },
+{ "TestingServerDownloadInitialDelay", "0" },
+{ "TestingClientDownloadInitialDelay", "0" },
+{ "TestingServerConsensusDownloadInitialDelay", "0" },
+{ "TestingClientConsensusDownloadInitialDelay", "0" },
+{ "TestingBridgeDownloadInitialDelay", "10" },
+{ "TestingBridgeBootstrapDownloadInitialDelay", "0" },
+{ "TestingClientMaxIntervalWithoutRequest", "5 seconds" },
+{ "TestingDirConnectionMaxStall", "30 seconds" },
+{ "TestingEnableConnBwEvent", "1" },
+{ "TestingEnableCellStatsEvent", "1" },
+{ "RendPostPeriod", "2 minutes" },
+{ "___UsingTestNetworkDefaults", "1" },

+ 4 - 3
src/core/include.am

@@ -437,9 +437,10 @@ noinst_HEADERS +=					\
 	src/feature/stats/rephist.h			\
 	src/feature/stats/predict_ports.h
 
-noinst_HEADERS +=			\
-	src/app/config/auth_dirs.inc	\
-	src/app/config/fallback_dirs.inc
+noinst_HEADERS +=				\
+	src/app/config/auth_dirs.inc		\
+	src/app/config/fallback_dirs.inc	\
+	src/app/config/testnet.inc
 
 # This may someday want to be an installed file?
 noinst_HEADERS += src/feature/api/tor_api.h

+ 6 - 10
src/feature/dirauth/shared_random_state.c

@@ -51,15 +51,11 @@ static const char dstate_cur_srv_key[] = "SharedRandCurrentValue";
  * members with CONF_CHECK_VAR_TYPE. */
 DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
 
-/* These next two are duplicates or near-duplicates from config.c */
-#define VAR(varname, conftype, member, initvalue)                       \
-  { { .name = varname,                                                  \
-      .type = CONFIG_TYPE_ ## conftype,                                 \
-      .offset = offsetof(sr_disk_state_t, member), },                   \
-    initvalue CONF_TEST_MEMBERS(sr_disk_state_t, conftype, member) }
-/* As VAR, but the option name and member name are the same. */
-#define V(member, conftype, initvalue) \
+#define VAR(varname,conftype,member,initvalue)                          \
+  CONFIG_VAR_ETYPE(sr_disk_state_t, varname, conftype, member, 0, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
+
 /* Our persistent state magic number. */
 #define SR_DISK_STATE_MAGIC 0x98AB1254
 
@@ -69,7 +65,7 @@ disk_state_validate_cb(void *old_state, void *state, void *default_state,
 static void disk_state_free_cb(void *);
 
 /* Array of variables that are saved to disk as a persistent state. */
-static config_var_t state_vars[] = {
+static const config_var_t state_vars[] = {
   V(Version,                    POSINT, "0"),
   V(TorVersion,                 STRING, NULL),
   V(ValidAfter,                 ISOTIME, NULL),
@@ -85,7 +81,7 @@ static config_var_t state_vars[] = {
 
 /* "Extra" variable in the state that receives lines we can't parse. This
  * lets us preserve options from versions of Tor newer than us. */
-static struct_member_t state_extra_var = {
+static const struct_member_t state_extra_var = {
   .name = "__extra",
   .type = CONFIG_TYPE_LINELIST,
   .offset = offsetof(sr_disk_state_t, ExtraLines),

+ 1 - 0
src/lib/conf/.may_include

@@ -1,2 +1,3 @@
 orconfig.h
 lib/cc/*.h
+lib/conf/*.h

+ 67 - 0
src/lib/conf/confmacros.h

@@ -0,0 +1,67 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file confmacros.h
+ * @brief Macro definitions for declaring configuration variables
+ **/
+
+#ifndef TOR_LIB_CONF_CONFMACROS_H
+#define TOR_LIB_CONF_CONFMACROS_H
+
+#include "orconfig.h"
+#include "lib/conf/conftesting.h"
+
+/**
+ * Used to indicate the end of an array of configuration variables.
+ **/
+#define END_OF_CONFIG_VARS                                      \
+  { .member = { .name = NULL } DUMMY_CONF_TEST_MEMBERS }
+
+/**
+ * Declare a config_var_t as a member named <b>membername</b> of the structure
+ * <b>structtype</b>, whose user-visible name is <b>varname</b>, whose
+ * type corresponds to the config_type_t member CONFIG_TYPE_<b>vartype</b>,
+ * and whose initial value is <b>intval</b>.
+ *
+ * Most modules that use this macro should wrap it in a local macro that
+ * sets structtype to the local configuration type.
+ **/
+#define CONFIG_VAR_ETYPE(structtype, varname, vartype, membername,      \
+                         varflags, initval)                             \
+  { .member =                                                           \
+    { .name = varname,                                                  \
+      .type = CONFIG_TYPE_ ## vartype,                                  \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .flags = varflags,                                                  \
+    .initvalue = initval                                                \
+    CONF_TEST_MEMBERS(structtype, vartype, membername)                  \
+  }
+
+/**
+ * As CONFIG_VAR_XTYPE, but declares a value using an extension type whose
+ * type definition is <b>vartype</b>_type_defn.
+ **/
+#define CONFIG_VAR_DEFN(structtype, varname, vartype, membername,       \
+                        varflags, initval)                              \
+  { .member =                                                           \
+    { .name = varname,                                                \
+      .type = CONFIG_TYPE_EXTENDED,                                     \
+      .type_def = &vartype ## _type_defn,                               \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .flags = varflags,                                                  \
+    .initvalue = initval                                                \
+    CONF_TEST_MEMBERS(structtype, vartype, membername)                  \
+  }
+
+#define CONFIG_VAR_OBSOLETE(varname)            \
+  { .member = { .name = varname, .type = CONFIG_TYPE_OBSOLETE },        \
+    .flags = CVFLAG_OBSOLETE                                            \
+  }
+
+#endif /* !defined(TOR_LIB_CONF_CONFMACROS_H) */

+ 86 - 0
src/lib/conf/conftesting.h

@@ -0,0 +1,86 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file conftesting.h
+ * @brief Macro and type declarations for testing
+ **/
+
+#ifndef TOR_LIB_CONF_CONFTESTING_H
+#define TOR_LIB_CONF_CONFTESTING_H
+
+#ifdef TOR_UNIT_TESTS
+/**
+ * Union used when building in test mode typechecking the members of a type
+ * used with confparse.c.  See CONF_CHECK_VAR_TYPE for a description of how
+ * it is used. */
+typedef union {
+  char **STRING;
+  char **FILENAME;
+  int *POSINT; /* yes, this is really an int, and not an unsigned int.  For
+                * historical reasons, many configuration values are restricted
+                * to the range [0,INT_MAX], and stored in signed ints.
+                */
+  uint64_t *UINT64;
+  int *INT;
+  int *INTERVAL;
+  int *MSEC_INTERVAL;
+  uint64_t *MEMUNIT;
+  double *DOUBLE;
+  int *BOOL;
+  int *AUTOBOOL;
+  time_t *ISOTIME;
+  struct smartlist_t **CSV;
+  int *CSV_INTERVAL;
+  struct config_line_t **LINELIST;
+  struct config_line_t **LINELIST_S;
+  struct config_line_t **LINELIST_V;
+  // XXXX this doesn't belong at this level of abstraction.
+  struct routerset_t **ROUTERSET;
+} confparse_dummy_values_t;
+#endif /* defined(TOR_UNIT_TESTS) */
+
+/* Macros to define extra members inside config_var_t fields, and at the
+ * end of a list of them.
+ */
+#ifdef TOR_UNIT_TESTS
+/* This is a somewhat magic type-checking macro for users of confparse.c.
+ * It initializes a union member "confparse_dummy_values_t.conftype" with
+ * the address of a static member "tp_dummy.member".   This
+ * will give a compiler warning unless the member field is of the correct
+ * type.
+ *
+ * (This warning is mandatory, because a type mismatch here violates the type
+ * compatibility constraint for simple assignment, and requires a diagnostic,
+ * according to the C spec.)
+ *
+ * For example, suppose you say:
+ *     "CONF_CHECK_VAR_TYPE(or_options_t, STRING, Address)".
+ * Then this macro will evaluate to:
+ *     { .STRING = &or_options_t_dummy.Address }
+ * And since confparse_dummy_values_t.STRING has type "char **", that
+ * expression will create a warning unless or_options_t.Address also
+ * has type "char *".
+ */
+#define CONF_CHECK_VAR_TYPE(tp, conftype, member)       \
+  { . conftype = &tp ## _dummy . member }
+#define CONF_TEST_MEMBERS(tp, conftype, member) \
+  , .var_ptr_dummy=CONF_CHECK_VAR_TYPE(tp, conftype, member)
+#define DUMMY_CONF_TEST_MEMBERS , .var_ptr_dummy={ .INT=NULL }
+#define DUMMY_TYPECHECK_INSTANCE(tp)            \
+  static tp tp ## _dummy
+
+#else /* !(defined(TOR_UNIT_TESTS)) */
+
+#define CONF_TEST_MEMBERS(tp, conftype, member)
+/* Repeatedly declarable incomplete struct to absorb redundant semicolons */
+#define DUMMY_TYPECHECK_INSTANCE(tp)            \
+  struct tor_semicolon_eater
+#define DUMMY_CONF_TEST_MEMBERS
+
+#endif /* defined(TOR_UNIT_TESTS) */
+
+#endif /* !defined(TOR_LIB_CONF_CONFTESTING_H) */

+ 33 - 32
src/lib/conf/conftypes.h

@@ -29,6 +29,9 @@
 #define TOR_SRC_LIB_CONF_CONFTYPES_H
 
 #include "lib/cc/torint.h"
+#ifdef TOR_UNIT_TESTS
+#include "lib/conf/conftesting.h"
+#endif
 
 /** Enumeration of types which option values can take */
 typedef enum config_type_t {
@@ -59,9 +62,6 @@ typedef enum config_type_t {
   CONFIG_TYPE_LINELIST_V,   /**< Catch-all "virtual" option to summarize
                              * context-sensitive config lines when fetching.
                              */
-  // XXXX this doesn't belong at this level of abstraction.
-  CONFIG_TYPE_ROUTERSET,    /**< A list of router names, addrs, and fps,
-                             * parsed into a routerset_t. */
   CONFIG_TYPE_OBSOLETE,     /**< Obsolete (ignored) option. */
   CONFIG_TYPE_EXTENDED,     /**< Extended type; definition will appear in
                              * pointer. */
@@ -105,35 +105,36 @@ typedef struct struct_magic_decl_t {
   int magic_offset;
 } struct_magic_decl_t;
 
-#ifdef TOR_UNIT_TESTS
 /**
- * Union used when building in test mode typechecking the members of a type
- * used with confparse.c.  See CONF_CHECK_VAR_TYPE for a description of how
- * it is used. */
-typedef union {
-  char **STRING;
-  char **FILENAME;
-  int *POSINT; /* yes, this is really an int, and not an unsigned int.  For
-                * historical reasons, many configuration values are restricted
-                * to the range [0,INT_MAX], and stored in signed ints.
-                */
-  uint64_t *UINT64;
-  int *INT;
-  int *INTERVAL;
-  int *MSEC_INTERVAL;
-  uint64_t *MEMUNIT;
-  double *DOUBLE;
-  int *BOOL;
-  int *AUTOBOOL;
-  time_t *ISOTIME;
-  struct smartlist_t **CSV;
-  int *CSV_INTERVAL;
-  struct config_line_t **LINELIST;
-  struct config_line_t **LINELIST_S;
-  struct config_line_t **LINELIST_V;
-  // XXXX this doesn't belong at this level of abstraction.
-  struct routerset_t **ROUTERSET;
-} confparse_dummy_values_t;
-#endif /* defined(TOR_UNIT_TESTS) */
+ * Flag to indicate that an option is obsolete. Any attempt to set or
+ * fetch this option should produce a warning.
+ **/
+#define CVFLAG_OBSOLETE  (1u<<0)
+/**
+ * Flag to indicate that an option is undumpable. An undumpable option is
+ * never saved to disk. For historical reasons it is prefixed with __ but
+ * not with ___.
+ **/
+#define CVFLAG_NODUMP    (1u<<1)
+/**
+ * Flag to indicate that an option is "invisible". An invisible option
+ * is always undumpable, and we don't tell the controller about it.
+ * For historical reasons it is prefixed with ___.
+ **/
+#define CVFLAG_INVISIBLE (1u<<2)
+
+/** A variable allowed in the configuration file or on the command line. */
+typedef struct config_var_t {
+  struct_member_t member; /** A struct member corresponding to this
+                           * variable. */
+  const char *initvalue; /**< String (or null) describing initial value. */
+  uint32_t flags; /**< One or more flags describing special handling for this
+                   * variable */
+#ifdef TOR_UNIT_TESTS
+  /** Used for compiler-magic to typecheck the corresponding field in the
+   * corresponding struct. Only used in unit test mode, at compile-time. */
+  confparse_dummy_values_t var_ptr_dummy;
+#endif
+} config_var_t;
 
 #endif /* !defined(TOR_SRC_LIB_CONF_CONFTYPES_H) */

+ 3 - 1
src/lib/conf/include.am

@@ -1,4 +1,6 @@
 
 # ADD_C_FILE: INSERT HEADERS HERE.
 noinst_HEADERS +=				\
-	src/lib/conf/conftypes.h
+	src/lib/conf/conftesting.h              \
+	src/lib/conf/conftypes.h                \
+	src/lib/conf/confmacros.h

+ 37 - 0
src/lib/confmgt/structvar.c

@@ -201,6 +201,19 @@ struct_var_kvencode(const void *object, const struct_member_t *member)
   return typed_var_kvencode_ex(member->name, p, def);
 }
 
+/**
+ * Mark the field in <b>object</b> determined by <b>member</b> -- a variable
+ * that ordinarily would be extended by assignment -- as "fragile", so that it
+ * will get replaced by the next assignment instead.
+ */
+void
+struct_var_mark_fragile(void *object, const struct_member_t *member)
+{
+  void *p = struct_get_mptr(object, member);
+  const var_type_def_t *def = get_type_def(member);
+  return typed_var_mark_fragile_ex(p, def);
+}
+
 /**
  * Return the official name of this struct member.
  **/
@@ -224,3 +237,27 @@ struct_var_get_typename(const struct_member_t *member)
 
   return def ? def->name : NULL;
 }
+
+bool
+struct_var_is_cumulative(const struct_member_t *member)
+{
+  const var_type_def_t *def = get_type_def(member);
+
+  return def ? def->is_cumulative : false;
+}
+
+bool
+struct_var_is_settable(const struct_member_t *member)
+{
+  const var_type_def_t *def = get_type_def(member);
+
+  return def ? !def->is_unsettable : true;
+}
+
+bool
+struct_var_is_contained(const struct_member_t *member)
+{
+  const var_type_def_t *def = get_type_def(member);
+
+  return def ? def->is_contained : false;
+}

+ 5 - 0
src/lib/confmgt/structvar.h

@@ -40,9 +40,14 @@ bool struct_var_eq(const void *a, const void *b,
                    const struct struct_member_t *member);
 bool struct_var_ok(const void *object,
                    const struct struct_member_t *member);
+void struct_var_mark_fragile(void *object,
+                             const struct struct_member_t *member);
 
 const char *struct_var_get_name(const struct struct_member_t *member);
 const char *struct_var_get_typename(const struct struct_member_t *member);
+bool struct_var_is_cumulative(const struct struct_member_t *member);
+bool struct_var_is_settable(const struct struct_member_t *member);
+bool struct_var_is_contained(const struct struct_member_t *member);
 
 int struct_var_kvassign(void *object, const struct config_line_t *line,
                         char **errmsg,

+ 45 - 20
src/lib/confmgt/type_defs.c

@@ -620,12 +620,22 @@ linelist_copy(void *target, const void *value, const void *params)
   return 0;
 }
 
+static void
+linelist_mark_fragile(void *target, const void *params)
+{
+  (void)params;
+  config_line_t **ptr = (config_line_t **)target;
+  if (*ptr)
+    (*ptr)->fragile = 1;
+}
+
 static const var_type_fns_t linelist_fns = {
   .kv_parse = linelist_kv_parse,
   .kv_encode = linelist_kv_encode,
   .clear = linelist_clear,
   .eq = linelist_eq,
   .copy = linelist_copy,
+  .mark_fragile = linelist_mark_fragile,
 };
 
 static const var_type_fns_t linelist_v_fns = {
@@ -634,6 +644,7 @@ static const var_type_fns_t linelist_v_fns = {
   .clear = linelist_clear,
   .eq = linelist_eq,
   .copy = linelist_copy,
+  .mark_fragile = linelist_mark_fragile,
 };
 
 static const var_type_fns_t linelist_s_fns = {
@@ -690,26 +701,40 @@ static const var_type_fns_t ignore_fns = {
  * Table mapping conf_type_t values to var_type_def_t objects.
  **/
 static const var_type_def_t type_definitions_table[] = {
-  [CONFIG_TYPE_STRING] = { "String", &string_fns, NULL },
-  [CONFIG_TYPE_FILENAME] = { "Filename", &string_fns, NULL },
-  [CONFIG_TYPE_INT] = { "SignedInteger", &int_fns, &INT_PARSE_UNRESTRICTED },
-  [CONFIG_TYPE_POSINT] = { "Integer", &int_fns, &INT_PARSE_POSINT },
-  [CONFIG_TYPE_UINT64] = { "Integer", &uint64_fns, NULL, },
-  [CONFIG_TYPE_MEMUNIT] = { "DataSize", &memunit_fns, &memory_units },
-  [CONFIG_TYPE_INTERVAL] = { "TimeInterval", &interval_fns, &time_units },
-  [CONFIG_TYPE_MSEC_INTERVAL] = { "TimeMsecInterval", &interval_fns,
-                                  &time_msec_units },
-  [CONFIG_TYPE_DOUBLE] = { "Float", &double_fns, NULL },
-  [CONFIG_TYPE_BOOL] = { "Boolean", &enum_fns, &enum_table_bool },
-  [CONFIG_TYPE_AUTOBOOL] = { "Boolean+Auto", &enum_fns, &enum_table_autobool },
-  [CONFIG_TYPE_ISOTIME] = { "Time", &time_fns, NULL },
-  [CONFIG_TYPE_CSV] = { "CommaList", &csv_fns, NULL },
-  [CONFIG_TYPE_CSV_INTERVAL] = { "TimeInterval", &legacy_csv_interval_fns,
-                                 NULL },
-  [CONFIG_TYPE_LINELIST] = { "LineList", &linelist_fns, NULL },
-  [CONFIG_TYPE_LINELIST_S] = { "Dependent", &linelist_s_fns, NULL },
-  [CONFIG_TYPE_LINELIST_V] = { "Virtual", &linelist_v_fns, NULL },
-  [CONFIG_TYPE_OBSOLETE] = { "Obsolete", &ignore_fns, NULL }
+  [CONFIG_TYPE_STRING] = { .name="String", .fns=&string_fns },
+  [CONFIG_TYPE_FILENAME] = { .name="Filename", .fns=&string_fns },
+  [CONFIG_TYPE_INT] = { .name="SignedInteger", .fns=&int_fns,
+                        .params=&INT_PARSE_UNRESTRICTED },
+  [CONFIG_TYPE_POSINT] = { .name="Integer", .fns=&int_fns,
+                           .params=&INT_PARSE_POSINT },
+  [CONFIG_TYPE_UINT64] = { .name="Integer", .fns=&uint64_fns, },
+  [CONFIG_TYPE_MEMUNIT] = { .name="DataSize", .fns=&memunit_fns,
+                            .params=&memory_units },
+  [CONFIG_TYPE_INTERVAL] = { .name="TimeInterval", .fns=&interval_fns,
+                             .params=&time_units },
+  [CONFIG_TYPE_MSEC_INTERVAL] = { .name="TimeMsecInterval",
+                                  .fns=&interval_fns,
+                                  .params=&time_msec_units },
+  [CONFIG_TYPE_DOUBLE] = { .name="Float", .fns=&double_fns, },
+  [CONFIG_TYPE_BOOL] = { .name="Boolean", .fns=&enum_fns,
+                         .params=&enum_table_bool },
+  [CONFIG_TYPE_AUTOBOOL] = { .name="Boolean+Auto", .fns=&enum_fns,
+                             .params=&enum_table_autobool },
+  [CONFIG_TYPE_ISOTIME] = { .name="Time", .fns=&time_fns, },
+  [CONFIG_TYPE_CSV] = { .name="CommaList", .fns=&csv_fns, },
+  [CONFIG_TYPE_CSV_INTERVAL] = { .name="TimeInterval",
+                                 .fns=&legacy_csv_interval_fns, },
+  [CONFIG_TYPE_LINELIST] = { .name="LineList", .fns=&linelist_fns,
+                             .is_cumulative=true},
+  [CONFIG_TYPE_LINELIST_S] = { .name="Dependent", .fns=&linelist_s_fns,
+                               .is_cumulative=true,
+                               .is_contained=true, },
+  [CONFIG_TYPE_LINELIST_V] = { .name="Virtual", .fns=&linelist_v_fns,
+                               .is_cumulative=true,
+                               .is_unsettable=true },
+  [CONFIG_TYPE_OBSOLETE] = { .name="Obsolete", .fns=&ignore_fns,
+                             .is_unsettable=true,
+                             .is_contained=true, }
 };
 
 /**

+ 45 - 0
src/lib/confmgt/typedvar.c

@@ -210,6 +210,51 @@ typed_var_ok_ex(const void *value, const var_type_def_t *def)
   return true;
 }
 
+/**
+ * Mark <b>value</b> -- a variable that ordinarily would be extended by
+ * assignment -- as "fragile", so that it will get replaced by the next
+ * assignment instead.
+ **/
+void
+typed_var_mark_fragile_ex(void *value, const var_type_def_t *def)
+{
+  if (BUG(!def)) {
+    return; // LCOV_EXCL_LINE
+  }
+
+  if (def->fns->mark_fragile)
+    def->fns->mark_fragile(value, def->params);
+}
+
+/**
+ * Return true iff multiple assignments to a variable will extend its
+ * value, rather than replacing it.
+ **/
+bool
+var_type_is_cumulative(const var_type_def_t *def)
+{
+  return def->is_cumulative;
+}
+
+/**
+ * Return true iff this variable type is always contained in another variable,
+ * and as such doesn't need to be dumped or copied independently.
+ **/
+bool
+var_type_is_contained(const var_type_def_t *def)
+{
+  return def->is_contained;
+}
+
+/**
+ * Return true iff this type can not be assigned directly by the user.
+ **/
+bool
+var_type_is_settable(const var_type_def_t *def)
+{
+  return ! def->is_unsettable;
+}
+
 /* =====
  * The functions below take a config_type_t instead of a var_type_def_t.
  * I'd like to deprecate them eventually and use var_type_def_t everywhere,

+ 6 - 0
src/lib/confmgt/typedvar.h

@@ -46,4 +46,10 @@ int typed_var_kvassign_ex(void *target, const struct config_line_t *line,
 struct config_line_t *typed_var_kvencode_ex(const char *key, const void *value,
                                             const var_type_def_t *def);
 
+void typed_var_mark_fragile_ex(void *value, const var_type_def_t *def);
+
+bool var_type_is_cumulative(const var_type_def_t *def);
+bool var_type_is_contained(const var_type_def_t *def);
+bool var_type_is_settable(const var_type_def_t *def);
+
 #endif /* !defined(TOR_LIB_CONFMGT_TYPEDVAR_H) */

+ 20 - 0
src/lib/confmgt/var_type_def_st.h

@@ -122,6 +122,15 @@ struct var_type_fns_t {
    * values are valid.
    **/
   bool (*ok)(const void *value, const void *params);
+  /**
+   * Mark a value of this variable as "fragile", so that future attempts to
+   * assign to this variable will replace rather than extending it.
+   *
+   * The default implementation for this function does nothing.
+   *
+   * Only meaningful for types with is_cumulative set.
+   **/
+  void (*mark_fragile)(void *value, const void *params);
 };
 
 /**
@@ -142,6 +151,17 @@ struct var_type_def_t {
    * calling the functions in this type's function table.
    */
   const void *params;
+
+  /** True iff a variable of this type can never be set directly by name. */
+  bool is_unsettable;
+  /** True iff a variable of this type is always contained in another
+   * variable, and as such doesn't need to be dumped or copied
+   * independently. */
+  bool is_contained;
+  /** True iff a variable of this type can be set more than once without
+   * destroying older values. Such variables should implement "mark_fragile".
+   */
+  bool is_cumulative;
 };
 
 #endif /* !defined(TOR_LIB_CONFMGT_VAR_TYPE_DEF_ST_H) */

+ 12 - 18
src/test/test_confparse.c

@@ -49,18 +49,13 @@ typedef struct test_struct_t {
 static test_struct_t test_struct_t_dummy;
 
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_##conftype,                                 \
-        .offset = offsetof(test_struct_t, member), },                   \
-      initvalue CONF_TEST_MEMBERS(test_struct_t, conftype, member) }
+  CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, 0, initvalue)
+#define V(member,conftype,initvalue)            \
+  VAR(#member, conftype, member, initvalue)
+#define OBSOLETE(varname)                       \
+  CONFIG_VAR_OBSOLETE(varname)
 
-#define V(name,conftype,initvalue)                                      \
-  VAR( #name, conftype, name, initvalue )
-
-#define OBSOLETE(varname)                                               \
-  { { .name=varname, .type=CONFIG_TYPE_OBSOLETE }, NULL, {.INT=NULL} }
-
-static config_var_t test_vars[] = {
+static const config_var_t test_vars[] = {
   V(s, STRING, "hello"),
   V(fn, FILENAME, NULL),
   V(pos, POSINT, NULL),
@@ -82,12 +77,11 @@ static config_var_t test_vars[] = {
   VAR("LineTypeB", LINELIST_S, mixed_lines, NULL),
   OBSOLETE("obsolete"),
   {
-   { .name = "routerset",
-     .type = CONFIG_TYPE_ROUTERSET,
-     .type_def = &ROUTERSET_type_defn,
-     .offset = offsetof(test_struct_t, routerset),
-   },
-   NULL, {.INT=NULL}
+   .member = { .name = "routerset",
+               .type = CONFIG_TYPE_EXTENDED,
+               .type_def = &ROUTERSET_type_defn,
+               .offset = offsetof(test_struct_t, routerset),
+             },
   },
   VAR("__HiddenInt", POSINT, hidden_int, "0"),
   VAR("MixedHiddenLines", LINELIST_V, mixed_hidden_lines, NULL),
@@ -129,7 +123,7 @@ static void test_free_cb(void *options);
 
 #define TEST_MAGIC 0x1337
 
-static config_format_t test_fmt = {
+static const config_format_t test_fmt = {
   sizeof(test_struct_t),
   {
    "test_struct_t",