From 4d101b39d74fb467d7fb4ad8ddb27e07c3074a69 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 20 Jun 2019 15:21:21 -0400
Subject: [PATCH 01/15] Move config_var_t info conftypes.h

---
 src/app/config/confparse.h | 13 -------------
 src/lib/conf/conftypes.h   | 13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index b91ea1c13d..f89ff3c214 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -32,19 +32,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.
  */
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index cddfeff2fd..5f13ec3de2 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -136,4 +136,17 @@ typedef union {
 } confparse_dummy_values_t;
 #endif /* defined(TOR_UNIT_TESTS) */
 
+/** 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;
+
 #endif /* !defined(TOR_SRC_LIB_CONF_CONFTYPES_H) */

From c553750e32d1bf669a3e8308fa44319954a627ca Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 20 Jun 2019 15:55:59 -0400
Subject: [PATCH 02/15] Move responsibility for config var macros

The testing-only parts now live in a conftesting.h; the shared parts
of the macros live in confmacros.h
---
 src/app/config/config.c                   | 24 +------
 src/app/config/confparse.h                | 39 +---------
 src/app/config/statefile.c                |  9 +--
 src/feature/dirauth/shared_random_state.c | 12 ++--
 src/lib/conf/.may_include                 |  1 +
 src/lib/conf/confmacros.h                 | 61 ++++++++++++++++
 src/lib/conf/conftesting.h                | 86 +++++++++++++++++++++++
 src/lib/conf/conftypes.h                  | 37 +---------
 src/lib/conf/include.am                   |  4 +-
 src/test/test_confparse.c                 | 12 ++--
 10 files changed, 168 insertions(+), 117 deletions(-)
 create mode 100644 src/lib/conf/confmacros.h
 create mode 100644 src/lib/conf/conftesting.h

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 8da1e2acdc..c15236b0e9 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -259,28 +259,12 @@ 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, 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 }
+  CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, initvalue)
 
-/** As VAR, but the option name and member name are the same. */
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
@@ -289,9 +273,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.
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index f89ff3c214..6d63ba3e0e 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -14,6 +14,7 @@
 #define TOR_CONFPARSE_H
 
 #include "lib/conf/conftypes.h"
+#include "lib/conf/confmacros.h"
 
 /** An abbreviation for a configuration option allowed on the command line. */
 typedef struct config_abbrev_t {
@@ -32,44 +33,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 }
 
-/* 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. */
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index 331592c3a6..e0584c62af 100644
--- a/src/app/config/statefile.c
+++ b/src/app/config/statefile.c
@@ -70,14 +70,9 @@ 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, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
 /** Array of "state" variables saved to the ~/.tor/state file. */
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c
index da4187b38a..3cdb223d25 100644
--- a/src/feature/dirauth/shared_random_state.c
+++ b/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, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
+
 /* Our persistent state magic number. */
 #define SR_DISK_STATE_MAGIC 0x98AB1254
 
diff --git a/src/lib/conf/.may_include b/src/lib/conf/.may_include
index 4285c3dcb8..629e2f897d 100644
--- a/src/lib/conf/.may_include
+++ b/src/lib/conf/.may_include
@@ -1,2 +1,3 @@
 orconfig.h
 lib/cc/*.h
+lib/conf/*.h
diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
new file mode 100644
index 0000000000..29040e1f55
--- /dev/null
+++ b/src/lib/conf/confmacros.h
@@ -0,0 +1,61 @@
+/* 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                                      \
+  { { .name = NULL }, 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, initval) \
+  { .member =                                                           \
+    { .name = varname,                                                  \
+      .type = CONFIG_TYPE_ ## vartype,                                  \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .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, initval) \
+  { .member =                                                           \
+    { .name = varname,                                                  \
+      .type = CONFIG_TYPE_EXTENDED,                                     \
+      .type_def = &vartype ## _type_defn,                               \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .initvalue = initval                                                \
+    CONF_TEST_MEMBERS(structtype, vartype, membername)                  \
+  }
+
+#define CONFIG_VAR_OBSOLETE(varname)            \
+  { .member = { .name = varname, .type = CONFIG_TYPE_OBSOLETE } }
+
+#endif /* !defined(TOR_LIB_CONF_CONFMACROS_H) */
diff --git a/src/lib/conf/conftesting.h b/src/lib/conf/conftesting.h
new file mode 100644
index 0000000000..f4aca442a2
--- /dev/null
+++ b/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) \
+  , CONF_CHECK_VAR_TYPE(tp, conftype, member)
+#define DUMMY_CONF_TEST_MEMBERS , { .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) */
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 5f13ec3de2..72697e8ee4 100644
--- a/src/lib/conf/conftypes.h
+++ b/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,37 +105,6 @@ 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) */
-
 /** 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
diff --git a/src/lib/conf/include.am b/src/lib/conf/include.am
index 25355697d2..cb7126184d 100644
--- a/src/lib/conf/include.am
+++ b/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
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index d0c33841fe..2f408b5b6e 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -49,13 +49,9 @@ 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) }
-
-#define V(name,conftype,initvalue)                                      \
-  VAR( #name, conftype, name, initvalue )
+  CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, initvalue)
+#define V(member,conftype,initvalue)            \
+  VAR(#member, conftype, member, initvalue)
 
 #define OBSOLETE(varname)                                               \
   { { .name=varname, .type=CONFIG_TYPE_OBSOLETE }, NULL, {.INT=NULL} }
@@ -83,7 +79,7 @@ static config_var_t test_vars[] = {
   OBSOLETE("obsolete"),
   {
    { .name = "routerset",
-     .type = CONFIG_TYPE_ROUTERSET,
+     .type = CONFIG_TYPE_EXTENDED,
      .type_def = &ROUTERSET_type_defn,
      .offset = offsetof(test_struct_t, routerset),
    },

From 5b252d31ede521bea53c690693a2ab3d74c8fef4 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 20 Jun 2019 16:07:00 -0400
Subject: [PATCH 03/15] Add a "flags" member to config_var_t

Additionally, adjust the macros so that we can add new members like
this more easily.
---
 src/lib/conf/confmacros.h  |  2 +-
 src/lib/conf/conftesting.h |  4 ++--
 src/lib/conf/conftypes.h   |  3 ++-
 src/test/test_confparse.c  | 16 +++++++---------
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
index 29040e1f55..4242137c5a 100644
--- a/src/lib/conf/confmacros.h
+++ b/src/lib/conf/confmacros.h
@@ -19,7 +19,7 @@
  * Used to indicate the end of an array of configuration variables.
  **/
 #define END_OF_CONFIG_VARS                                      \
-  { { .name = NULL }, NULL DUMMY_CONF_TEST_MEMBERS }
+  { .member = { .name = NULL } DUMMY_CONF_TEST_MEMBERS }
 
 /**
  * Declare a config_var_t as a member named <b>membername</b> of the structure
diff --git a/src/lib/conf/conftesting.h b/src/lib/conf/conftesting.h
index f4aca442a2..a40c9bc97c 100644
--- a/src/lib/conf/conftesting.h
+++ b/src/lib/conf/conftesting.h
@@ -68,8 +68,8 @@ typedef union {
 #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 DUMMY_CONF_TEST_MEMBERS , { .INT=NULL }
+  , .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
 
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 72697e8ee4..028a889917 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -110,7 +110,8 @@ 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. */
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index 2f408b5b6e..4bf66f3230 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -52,9 +52,8 @@ static test_struct_t test_struct_t_dummy;
   CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, initvalue)
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
-
-#define OBSOLETE(varname)                                               \
-  { { .name=varname, .type=CONFIG_TYPE_OBSOLETE }, NULL, {.INT=NULL} }
+#define OBSOLETE(varname)                       \
+  CONFIG_VAR_OBSOLETE(varname)
 
 static config_var_t test_vars[] = {
   V(s, STRING, "hello"),
@@ -78,12 +77,11 @@ static config_var_t test_vars[] = {
   VAR("LineTypeB", LINELIST_S, mixed_lines, NULL),
   OBSOLETE("obsolete"),
   {
-   { .name = "routerset",
-     .type = CONFIG_TYPE_EXTENDED,
-     .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),

From b6457d4c08f601c4e42e64aad47ac9c30c36306e Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 20 Jun 2019 16:26:05 -0400
Subject: [PATCH 04/15] Extend macros to allow flag arguments.

---
 src/app/config/config.c                   |  4 ++--
 src/app/config/statefile.c                |  2 +-
 src/feature/dirauth/shared_random_state.c |  2 +-
 src/lib/conf/confmacros.h                 | 10 +++++++---
 src/test/test_confparse.c                 |  2 +-
 5 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index c15236b0e9..5f7a82d15a 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -259,11 +259,11 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
  * or_options_t.<b>member</b>"
  */
 #define VAR(varname,conftype,member,initvalue)                          \
-  CONFIG_VAR_ETYPE(or_options_t, varname, conftype, member, initvalue)
+  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)                        \
-  CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, initvalue)
+  CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, 0, initvalue)
 
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index e0584c62af..4fe415b8cf 100644
--- a/src/app/config/statefile.c
+++ b/src/app/config/statefile.c
@@ -71,7 +71,7 @@ static config_abbrev_t state_abbrevs_[] = {
 DUMMY_TYPECHECK_INSTANCE(or_state_t);
 
 #define VAR(varname,conftype,member,initvalue)                          \
-  CONFIG_VAR_ETYPE(or_state_t, varname, conftype, member, initvalue)
+  CONFIG_VAR_ETYPE(or_state_t, varname, conftype, member, 0, initvalue)
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c
index 3cdb223d25..d89f249a72 100644
--- a/src/feature/dirauth/shared_random_state.c
+++ b/src/feature/dirauth/shared_random_state.c
@@ -52,7 +52,7 @@ static const char dstate_cur_srv_key[] = "SharedRandCurrentValue";
 DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
 
 #define VAR(varname,conftype,member,initvalue)                          \
-  CONFIG_VAR_ETYPE(sr_disk_state_t, 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)
 
diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
index 4242137c5a..ff284e681d 100644
--- a/src/lib/conf/confmacros.h
+++ b/src/lib/conf/confmacros.h
@@ -30,12 +30,14 @@
  * 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, initval) \
+#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)                  \
   }
@@ -44,13 +46,15 @@
  * 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, initval) \
+#define CONFIG_VAR_DEFN(structtype, varname, vartype, membername,       \
+                        varflags, initval)                              \
   { .member =                                                           \
-    { .name = varname,                                                  \
+      { .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)                  \
   }
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index 4bf66f3230..4612419dff 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -49,7 +49,7 @@ typedef struct test_struct_t {
 static test_struct_t test_struct_t_dummy;
 
 #define VAR(varname,conftype,member,initvalue)                          \
-  CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, initvalue)
+  CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, 0, initvalue)
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 #define OBSOLETE(varname)                       \

From a7835202cf871f68854494df904058a6e644c0b0 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 21 Jun 2019 09:58:40 -0400
Subject: [PATCH 05/15] Turn several properties of types or variables into
 flags.

"unsettable" is a property of types.  LINELIST_V and OBSOLETE are
unsettable, meaning that they cannot be set by name.

"contained" is a property of types.  I'm hoping to find a better
name here.  LINELIST_S is "contained" because it always appears
within a LINELIST_V, and as such doesn't need to be dumped ore
copied independently.

"cumulative" is a property of types. Cumulative types can appear
more than once in a torrc without causing a warning, because they
add to each other rather than replacing each other.

"obsolete" is a property of variables.

"marking fragile" is now a command that struct members can accept.

With these changes, confparse and config no longer ever need to
mention CONFIG_TYPE_XYZ values by name.
---
 src/app/config/config.c           |  9 +++--
 src/app/config/confparse.c        | 45 ++++++++++++---------
 src/app/config/confparse.h        |  4 ++
 src/lib/conf/confmacros.h         |  4 +-
 src/lib/conf/conftypes.h          |  6 +++
 src/lib/confmgt/structvar.c       | 37 ++++++++++++++++++
 src/lib/confmgt/structvar.h       |  5 +++
 src/lib/confmgt/type_defs.c       | 65 +++++++++++++++++++++----------
 src/lib/confmgt/typedvar.c        | 45 +++++++++++++++++++++
 src/lib/confmgt/typedvar.h        |  6 +++
 src/lib/confmgt/var_type_def_st.h | 20 ++++++++++
 11 files changed, 203 insertions(+), 43 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 5f7a82d15a..d240a73fe6 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -960,8 +960,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)) {
@@ -2663,9 +2663,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);
   }
 }
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 2890d8c81b..0d19974d70 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -148,6 +148,24 @@ 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);
+}
+
 /*
  * Functions to assign config options.
  */
@@ -183,14 +201,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 +266,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 +282,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 +570,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,9 +637,10 @@ 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, "__"))
       continue;
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index 6d63ba3e0e..c53e3224db 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -104,6 +104,10 @@ 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);
+
 /* 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)
diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
index ff284e681d..aa89965e69 100644
--- a/src/lib/conf/confmacros.h
+++ b/src/lib/conf/confmacros.h
@@ -60,6 +60,8 @@
   }
 
 #define CONFIG_VAR_OBSOLETE(varname)            \
-  { .member = { .name = varname, .type = CONFIG_TYPE_OBSOLETE } }
+  { .member = { .name = varname, .type = CONFIG_TYPE_OBSOLETE },        \
+    .flags = CVFLAG_OBSOLETE                                            \
+  }
 
 #endif /* !defined(TOR_LIB_CONF_CONFMACROS_H) */
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 028a889917..6a44fb92ed 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -105,6 +105,12 @@ typedef struct struct_magic_decl_t {
   int magic_offset;
 } struct_magic_decl_t;
 
+/**
+ * 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)
+
 /** 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
diff --git a/src/lib/confmgt/structvar.c b/src/lib/confmgt/structvar.c
index 38f8e5dd7a..97a8fb3633 100644
--- a/src/lib/confmgt/structvar.c
+++ b/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;
+}
diff --git a/src/lib/confmgt/structvar.h b/src/lib/confmgt/structvar.h
index 92b9b6fc71..e6dbc6d6ec 100644
--- a/src/lib/confmgt/structvar.h
+++ b/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,
diff --git a/src/lib/confmgt/type_defs.c b/src/lib/confmgt/type_defs.c
index 62b4c1019d..f8b2681aa0 100644
--- a/src/lib/confmgt/type_defs.c
+++ b/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, }
 };
 
 /**
diff --git a/src/lib/confmgt/typedvar.c b/src/lib/confmgt/typedvar.c
index c2b9b45725..3cba075390 100644
--- a/src/lib/confmgt/typedvar.c
+++ b/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,
diff --git a/src/lib/confmgt/typedvar.h b/src/lib/confmgt/typedvar.h
index 720ad54fc6..2e36f9d673 100644
--- a/src/lib/confmgt/typedvar.h
+++ b/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) */
diff --git a/src/lib/confmgt/var_type_def_st.h b/src/lib/confmgt/var_type_def_st.h
index c63ff66eff..c4f7acec69 100644
--- a/src/lib/confmgt/var_type_def_st.h
+++ b/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) */

From f3330d2be3de287d4b15bd4d9b0e86fab3ffb9fd Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 21 Jun 2019 10:17:59 -0400
Subject: [PATCH 06/15] Make "invisibility" and "undumpability" properties of
 variables.

Previously, these were magical things that we detected by checking
whether a variable's name was prefixed with two or three underscores.
---
 src/app/config/config.c    | 35 +++++++++++++++++++++++------------
 src/app/config/confparse.c |  2 +-
 src/lib/conf/conftypes.h   | 10 ++++++++++
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index d240a73fe6..5667702a60 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -265,6 +265,13 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
 #define VAR_D(varname,conftype,member,initvalue)                        \
   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)
 
@@ -285,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"
@@ -682,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"),
@@ -743,7 +752,8 @@ 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
 };
@@ -783,7 +793,8 @@ static const config_var_t testing_tor_network_defaults[] = {
   V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"),
   V(TestingEnableConnBwEvent,    BOOL,     "1"),
   V(TestingEnableCellStatsEvent, BOOL,     "1"),
-  VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "1"),
+  VAR_INVIS("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_,
+             "1"),
   V(RendPostPeriod,              INTERVAL, "2 minutes"),
 
   END_OF_CONFIG_VARS
@@ -8180,7 +8191,7 @@ getinfo_helper_config(control_connection_t *conn,
     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))
+      if (option_vars_[i].flags & CVFLAG_INVISIBLE)
         continue;
       const char *type = struct_var_get_typename(&var->member);
       if (!type)
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 0d19974d70..450ff5e083 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -642,7 +642,7 @@ config_dump(const config_format_t *fmt, const void *default_options,
       continue;
     }
     /* Don't save 'hidden' control variables. */
-    if (!strcmpstart(fmt->vars[i].member.name, "__"))
+    if (fmt->vars[i].flags & CVFLAG_NODUMP)
       continue;
     if (minimal && config_is_same(fmt, options, defaults,
                                   fmt->vars[i].member.name))
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 6a44fb92ed..3b754e07be 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -110,6 +110,16 @@ typedef struct struct_magic_decl_t {
  * 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, and is prefixed 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.
+ **/
+#define CVFLAG_INVISIBLE (1u<<2)
 
 /** A variable allowed in the configuration file or on the command line. */
 typedef struct config_var_t {

From f306d12b58a9447076b961da072061f65830692c Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 21 Jun 2019 11:11:48 -0400
Subject: [PATCH 07/15] Refactor handling of TestingTorNetwork

Previously, when TestingTorNetwork was set, we would manually adjust
the initvalue members of a bunch of other config_var_t, and then
re-run the early parts or parsing the options.

Now we treat the initvalue fields as immutable, but instead assign
to them in options_init(), as early as possible.  Rather than
re-running the early parts of options, we just re-call the
options_init_from_string() function.

This patch de-kludges some of our code pretty handily.  I think it
could later handle authorities and fallbacks, but for now I think we
should leave those alone.
---
 scripts/maint/practracker/exceptions.txt |   2 +-
 src/app/config/config.c                  | 186 ++++++++---------------
 src/app/config/testnet.inc               |  33 ++++
 src/core/include.am                      |   7 +-
 4 files changed, 104 insertions(+), 124 deletions(-)
 create mode 100644 src/app/config/testnet.inc

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index 2190fb1caa..1e1ccda782 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/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() 192
diff --git a/src/app/config/config.c b/src/app/config/config.c
index 5667702a60..5ea6c2d40b 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -758,46 +758,28 @@ static config_var_t option_vars_[] = {
   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_INVIS("___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
@@ -936,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.
@@ -1185,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>.
@@ -3008,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
@@ -5397,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;
@@ -5454,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;
@@ -5565,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;
 }
 
diff --git a/src/app/config/testnet.inc b/src/app/config/testnet.inc
new file mode 100644
index 0000000000..0ed3c38627
--- /dev/null
+++ b/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" },
diff --git a/src/core/include.am b/src/core/include.am
index 1a4b9fb8ab..0dc8d7cad8 100644
--- a/src/core/include.am
+++ b/src/core/include.am
@@ -435,9 +435,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

From f8b193a74a3967d23bf55c8dcfb5bb2a16692c97 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 21 Jun 2019 11:20:21 -0400
Subject: [PATCH 08/15] Make config_var and config_fmt const.

Now that we have a reasonable implementation for overriding the
default options for TestingTorNetwork, we don't need to modify
config_var_t structs any more.  And therefore, we can have constant
format options, like reasonable people.
---
 src/app/config/config.c                   |  6 +++---
 src/app/config/config.h                   |  2 +-
 src/app/config/confparse.c                | 21 +++++++--------------
 src/app/config/confparse.h                | 13 ++++++-------
 src/app/config/statefile.c                |  2 +-
 src/feature/dirauth/shared_random_state.c |  4 ++--
 src/test/test_confparse.c                 |  4 ++--
 7 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 5ea6c2d40b..4bc807a6fb 100644
--- a/src/app/config/config.c
+++ b/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),
@@ -301,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),
@@ -851,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",
diff --git a/src/app/config/config.h b/src/app/config/config.h
index 46db02f944..c6feb89fe7 100644
--- a/src/app/config/config.h
+++ b/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);
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 450ff5e083..9bc0b1dc75 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -100,9 +100,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 +131,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)
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index c53e3224db..65972d6fcb 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -47,16 +47,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
@@ -93,8 +94,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,
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index 4fe415b8cf..d997d3932e 100644
--- a/src/app/config/statefile.c
+++ b/src/app/config/statefile.c
@@ -76,7 +76,7 @@ DUMMY_TYPECHECK_INSTANCE(or_state_t);
   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),
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c
index d89f249a72..c2ad3e7cca 100644
--- a/src/feature/dirauth/shared_random_state.c
+++ b/src/feature/dirauth/shared_random_state.c
@@ -65,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),
@@ -81,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),
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index 4612419dff..ec018f0c52 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -55,7 +55,7 @@ static test_struct_t test_struct_t_dummy;
 #define OBSOLETE(varname)                       \
   CONFIG_VAR_OBSOLETE(varname)
 
-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),
@@ -123,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",

From c32d485942e766eeea70cab468cc7c727a5be270 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 26 Jun 2019 11:02:23 -0400
Subject: [PATCH 09/15] Remove src/core/ and src/feature dependencies from
 confparse.c

This C file will eventually belong in lib/confmgt, so it needs to
have only low-level dependencies.  Now that it no longers needs
routerset.c, we can adjust its includes accordingly.

I'm not moving the file yet, since it would make fixup commits on
earlier branches here really hard to do.
---
 src/app/config/confparse.c | 12 +++++++++---
 src/app/config/confparse.h |  1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 9bc0b1dc75..699fc0e3dd 100644
--- a/src/app/config/confparse.c
+++ b/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);
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index 65972d6fcb..3633c2a80b 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -15,6 +15,7 @@
 
 #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 {

From 03247c8eaa305fc527ecda075a7b0a7145014c98 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 24 Aug 2019 17:11:41 -0400
Subject: [PATCH 10/15] whitespace fix

---
 src/lib/conf/confmacros.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
index aa89965e69..2a15f09aac 100644
--- a/src/lib/conf/confmacros.h
+++ b/src/lib/conf/confmacros.h
@@ -49,7 +49,7 @@
 #define CONFIG_VAR_DEFN(structtype, varname, vartype, membername,       \
                         varflags, initval)                              \
   { .member =                                                           \
-      { .name = varname,                                                \
+    { .name = varname,                                                \
       .type = CONFIG_TYPE_EXTENDED,                                     \
       .type_def = &vartype ## _type_defn,                               \
       .offset = offsetof(structtype, membername),                       \

From 65511a45adfeff258e71329b3ff98d5159f09f88 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 24 Aug 2019 17:26:02 -0400
Subject: [PATCH 11/15] Fix comment: ___ options are now that way because of
 the INVISIBLE flag.

---
 src/app/config/config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 4bc807a6fb..f2e8ec8200 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -8136,7 +8136,7 @@ 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 */
+      /* don't tell controller about invisible options */
       if (option_vars_[i].flags & CVFLAG_INVISIBLE)
         continue;
       const char *type = struct_var_get_typename(&var->member);

From eb54fe8f97701a11e30009a94f11b7e3e5d69b4f Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 24 Aug 2019 17:32:46 -0400
Subject: [PATCH 12/15] Make config_var_is_invisible a function.

---
 src/app/config/config.c    | 2 +-
 src/app/config/confparse.c | 5 +++++
 src/app/config/confparse.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index f2e8ec8200..740315f3e4 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -8137,7 +8137,7 @@ getinfo_helper_config(control_connection_t *conn,
     for (i = 0; option_vars_[i].member.name; ++i) {
       const config_var_t *var = &option_vars_[i];
       /* don't tell controller about invisible options */
-      if (option_vars_[i].flags & CVFLAG_INVISIBLE)
+      if (config_var_is_invisible(var))
         continue;
       const char *type = struct_var_get_typename(&var->member);
       if (!type)
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 699fc0e3dd..7127e62cde 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -164,6 +164,11 @@ 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;
+}
 
 /*
  * Functions to assign config options.
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index 3633c2a80b..0ae675bcdf 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -107,6 +107,7 @@ 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);
 
 /* Helper macros to compare an option across two configuration objects */
 #define CFG_EQ_BOOL(a,b,opt) ((a)->opt == (b)->opt)

From 078ba326a1255dbfbb442219c599e7961f4f85e2 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 24 Aug 2019 17:40:38 -0400
Subject: [PATCH 13/15] Make a function for NODUMP.

---
 src/app/config/confparse.c | 7 ++++++-
 src/app/config/confparse.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 7127e62cde..6e2624466a 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -169,6 +169,11 @@ 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.
@@ -646,7 +651,7 @@ config_dump(const config_format_t *fmt, const void *default_options,
       continue;
     }
     /* Don't save 'hidden' control variables. */
-    if (fmt->vars[i].flags & CVFLAG_NODUMP)
+    if (! config_var_is_dumpable(&fmt->vars[i]))
       continue;
     if (minimal && config_is_same(fmt, options, defaults,
                                   fmt->vars[i].member.name))
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index 0ae675bcdf..d6571692f9 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -108,6 +108,7 @@ 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)

From cde662e259005887b897b5108e8a4b63f99abb59 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sat, 24 Aug 2019 17:46:08 -0400
Subject: [PATCH 14/15] clarify usage of __ and ___ prefixes on config options

---
 src/lib/conf/conftypes.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 3b754e07be..fabad97d0c 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -112,12 +112,14 @@ typedef struct struct_magic_decl_t {
 #define CVFLAG_OBSOLETE  (1u<<0)
 /**
  * Flag to indicate that an option is undumpable. An undumpable option is
- * never saved to disk, and is prefixed with __.
+ * 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)
 

From 38c4e1426c2964f4c27d2486b81867bff6d55b8a Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 26 Aug 2019 09:33:29 -0400
Subject: [PATCH 15/15] changes file for ticket30935

---
 changes/ticket30935 | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changes/ticket30935

diff --git a/changes/ticket30935 b/changes/ticket30935
new file mode 100644
index 0000000000..5a7e918895
--- /dev/null
+++ b/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.