Merge branch 'tor-github/pr/1244'

changes/ticket30914

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Lower another layer of object management from confparse.c to
+      a more general tool.  Now typed structure members are accessible
+      via an abstract type. Implements ticket 30914.

scripts/maint/practracker/exceptions.txt

@@ -29,8 +29,8 @@
 #
 # Remember: It is better to fix the problem than to add a new exception!
 
-problem file-size /src/app/config/config.c 8510
-problem include-count /src/app/config/config.c 87
+problem file-size /src/app/config/config.c 8518
+problem include-count /src/app/config/config.c 88
 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

src/app/config/config.c

@@ -111,6 +111,7 @@
 #include "feature/stats/predict_ports.h"
 #include "feature/stats/rephist.h"
 #include "lib/compress/compress.h"
+#include "lib/confmgt/structvar.h"
 #include "lib/crypt_ops/crypto_init.h"
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
@@ -253,23 +254,45 @@ static config_abbrev_t option_abbrevs_[] = {
  * members with CONF_CHECK_VAR_TYPE. */
 DUMMY_TYPECHECK_INSTANCE(or_options_t);
 
-/** An entry for config_vars: "The option <b>name</b> has type
+/** An entry for config_vars: "The option <b>varname</b> has type
  * CONFIG_TYPE_<b>conftype</b>, and corresponds to
  * or_options_t.<b>member</b>"
  */
-#define VAR(name,conftype,member,initvalue)                             \
-  { name, CONFIG_TYPE_ ## conftype, offsetof(or_options_t, member),     \
+#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) }
-/** As VAR, but the option name and member name are the same. */
-#define V(member,conftype,initvalue)                                    \
-  VAR(#member, conftype, member, initvalue)
-/** An entry for config_vars: "The option <b>name</b> is obsolete." */
+
 #ifdef TOR_UNIT_TESTS
-#define OBSOLETE(name) { name, CONFIG_TYPE_OBSOLETE, 0, NULL, {.INT=NULL} }
+#define DUMMY_TEST_MEMBERS , {.INT=NULL}
 #else
-#define OBSOLETE(name) { name, CONFIG_TYPE_OBSOLETE, 0, NULL }
+#define DUMMY_TEST_MEMBERS
 #endif
 
+/* 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. */
+#define V(member,conftype,initvalue)            \
+  VAR(#member, conftype, member, initvalue)
+
+/** As V, but uses a type definition instead of a type enum */
+#define V_D(member,type,initvalue)              \
+  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 }
+
 /**
  * Macro to declare *Port options.  Each one comes in three entries.
  * For example, most users should use "SocksPort" to configure the
@@ -418,17 +441,17 @@ static config_var_t option_vars_[] = {
   V(TestingEnableCellStatsEvent, BOOL,     "0"),
   OBSOLETE("TestingEnableTbEmptyEvent"),
   V(EnforceDistinctSubnets,      BOOL,     "1"),
-  V(EntryNodes,                  ROUTERSET,   NULL),
+  V_D(EntryNodes,                ROUTERSET,   NULL),
   V(EntryStatistics,             BOOL,     "0"),
   V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "10 minutes"),
-  V(ExcludeNodes,                ROUTERSET, NULL),
-  V(ExcludeExitNodes,            ROUTERSET, NULL),
+  V_D(ExcludeNodes,              ROUTERSET, NULL),
+  V_D(ExcludeExitNodes,          ROUTERSET, NULL),
   OBSOLETE("ExcludeSingleHopRelays"),
-  V(ExitNodes,                   ROUTERSET, NULL),
+  V_D(ExitNodes,                 ROUTERSET, NULL),
   /* Researchers need a way to tell their clients to use specific
    * middles that they also control, to allow safe live-network
    * experimentation with new padding machines. */
-  V(MiddleNodes,                 ROUTERSET, NULL),
+  V_D(MiddleNodes,               ROUTERSET, NULL),
   V(ExitPolicy,                  LINELIST, NULL),
   V(ExitPolicyRejectPrivate,     BOOL,     "1"),
   V(ExitPolicyRejectLocalInterfaces, BOOL, "0"),
@@ -507,8 +530,8 @@ static config_var_t option_vars_[] = {
   V(Socks5ProxyPassword,         STRING,   NULL),
   VAR("KeyDirectory",            FILENAME, KeyDirectory_option, NULL),
   V(KeyDirectoryGroupReadable,   BOOL,     "0"),
-  VAR("HSLayer2Nodes",           ROUTERSET,  HSLayer2Nodes,  NULL),
-  VAR("HSLayer3Nodes",           ROUTERSET,  HSLayer3Nodes,  NULL),
+  VAR_D("HSLayer2Nodes",         ROUTERSET,  HSLayer2Nodes,  NULL),
+  VAR_D("HSLayer3Nodes",         ROUTERSET,  HSLayer3Nodes,  NULL),
   V(KeepalivePeriod,             INTERVAL, "5 minutes"),
   V(KeepBindCapabilities,            AUTOBOOL, "auto"),
   VAR("Log",                     LINELIST, Logs,             NULL),
@@ -732,11 +755,11 @@ static config_var_t option_vars_[] = {
   OBSOLETE("TestingDescriptorMaxDownloadTries"),
   OBSOLETE("TestingMicrodescMaxDownloadTries"),
   OBSOLETE("TestingCertMaxDownloadTries"),
-  V(TestingDirAuthVoteExit, ROUTERSET, NULL),
+  V_D(TestingDirAuthVoteExit, ROUTERSET, NULL),
   V(TestingDirAuthVoteExitIsStrict,  BOOL,     "0"),
-  V(TestingDirAuthVoteGuard, ROUTERSET, NULL),
+  V_D(TestingDirAuthVoteGuard, ROUTERSET, NULL),
   V(TestingDirAuthVoteGuardIsStrict,  BOOL,     "0"),
-  V(TestingDirAuthVoteHSDir, ROUTERSET, NULL),
+  V_D(TestingDirAuthVoteHSDir, ROUTERSET, NULL),
   V(TestingDirAuthVoteHSDirIsStrict,  BOOL,     "0"),
   VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "0"),
 
@@ -855,8 +878,11 @@ static void set_protocol_warning_severity_level(int warning_severity);
 /** Configuration format for or_options_t. */
 STATIC config_format_t options_format = {
   sizeof(or_options_t),
-  OR_OPTIONS_MAGIC,
-  offsetof(or_options_t, magic_),
+  {
+   "or_options_t",
+   OR_OPTIONS_MAGIC,
+   offsetof(or_options_t, magic_),
+  },
   option_abbrevs_,
   option_deprecation_notes_,
   option_vars_,
@@ -949,11 +975,11 @@ set_options(or_options_t *new_val, char **msg)
    * just starting up then the old_options will be undefined. */
   if (old_options && old_options != global_options) {
     elements = smartlist_new();
-    for (i=0; options_format.vars[i].name; ++i) {
+    for (i=0; options_format.vars[i].member.name; ++i) {
       const config_var_t *var = &options_format.vars[i];
-      const char *var_name = var->name;
-      if (var->type == CONFIG_TYPE_LINELIST_S ||
-          var->type == CONFIG_TYPE_OBSOLETE) {
+      const char *var_name = var->member.name;
+      if (var->member.type == CONFIG_TYPE_LINELIST_S ||
+          var->member.type == CONFIG_TYPE_OBSOLETE) {
         continue;
       }
       if (!config_is_same(&options_format, new_val, old_options, var_name)) {
@@ -969,7 +995,7 @@ set_options(or_options_t *new_val, char **msg)
             tor_free(line);
           }
         } else {
-          smartlist_add_strdup(elements, options_format.vars[i].name);
+          smartlist_add_strdup(elements, options_format.vars[i].member.name);
           smartlist_add(elements, NULL);
         }
       }
@@ -2571,7 +2597,7 @@ const char *
 option_get_canonical_name(const char *key)
 {
   const config_var_t *var = config_find_option(&options_format, key);
-  return var ? var->name : NULL;
+  return var ? var->member.name : NULL;
 }
 
 /** Return a canonical list of the options assigned for key.
@@ -2653,12 +2679,12 @@ static void
 list_torrc_options(void)
 {
   int i;
-  for (i = 0; option_vars_[i].name; ++i) {
+  for (i = 0; option_vars_[i].member.name; ++i) {
     const config_var_t *var = &option_vars_[i];
-    if (var->type == CONFIG_TYPE_OBSOLETE ||
-        var->type == CONFIG_TYPE_LINELIST_V)
+    if (var->member.type == CONFIG_TYPE_OBSOLETE ||
+        var->member.type == CONFIG_TYPE_LINELIST_V)
       continue;
-    printf("%s\n", var->name);
+    printf("%s\n", var->member.name);
   }
 }
 
@@ -5445,18 +5471,18 @@ options_init_from_string(const char *cf_defaults, const char *cf,
      * let's clean it up.  -NM */
 
     /* Change defaults. */
-    for (int i = 0; testing_tor_network_defaults[i].name; ++i) {
+    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->name);
+        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->name))) {
+      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->name);
+                 new_var->member.name);
       }
     }
 
@@ -8168,41 +8194,15 @@ getinfo_helper_config(control_connection_t *conn,
   if (!strcmp(question, "config/names")) {
     smartlist_t *sl = smartlist_new();
     int i;
-    for (i = 0; option_vars_[i].name; ++i) {
+    for (i = 0; option_vars_[i].member.name; ++i) {
       const config_var_t *var = &option_vars_[i];
-      const char *type;
       /* don't tell controller about triple-underscore options */
-      if (!strncmp(option_vars_[i].name, "___", 3))
+      if (!strncmp(option_vars_[i].member.name, "___", 3))
         continue;
-      switch (var->type) {
-        case CONFIG_TYPE_STRING: type = "String"; break;
-        case CONFIG_TYPE_FILENAME: type = "Filename"; break;
-        case CONFIG_TYPE_POSINT: type = "Integer"; break;
-        case CONFIG_TYPE_UINT64: type = "Integer"; break;
-        case CONFIG_TYPE_INT: type = "SignedInteger"; break;
-        case CONFIG_TYPE_INTERVAL: type = "TimeInterval"; break;
-        case CONFIG_TYPE_MSEC_INTERVAL: type = "TimeMsecInterval"; break;
-        case CONFIG_TYPE_MEMUNIT: type = "DataSize"; break;
-        case CONFIG_TYPE_DOUBLE: type = "Float"; break;
-        case CONFIG_TYPE_BOOL: type = "Boolean"; break;
-        case CONFIG_TYPE_AUTOBOOL: type = "Boolean+Auto"; break;
-        case CONFIG_TYPE_ISOTIME: type = "Time"; break;
-        case CONFIG_TYPE_ROUTERSET: type = "RouterList"; break;
-        case CONFIG_TYPE_CSV: type = "CommaList"; break;
-        /* This type accepts more inputs than TimeInterval, but it ignores
-         * everything after the first entry, so we may as well pretend
-         * it's a TimeInterval. */
-        case CONFIG_TYPE_CSV_INTERVAL: type = "TimeInterval"; break;
-        case CONFIG_TYPE_LINELIST: type = "LineList"; break;
-        case CONFIG_TYPE_LINELIST_S: type = "Dependent"; break;
-        case CONFIG_TYPE_LINELIST_V: type = "Virtual"; break;
-        default:
-        case CONFIG_TYPE_OBSOLETE:
-          type = NULL; break;
-      }
+      const char *type = struct_var_get_typename(&var->member);
       if (!type)
         continue;
-      smartlist_add_asprintf(sl, "%s %s\n",var->name,type);
+      smartlist_add_asprintf(sl, "%s %s\n",var->member.name,type);
     }
     *answer = smartlist_join_strings(sl, "", 0, NULL);
     SMARTLIST_FOREACH(sl, char *, c, tor_free(c));
@@ -8210,17 +8210,17 @@ getinfo_helper_config(control_connection_t *conn,
   } else if (!strcmp(question, "config/defaults")) {
     smartlist_t *sl = smartlist_new();
     int dirauth_lines_seen = 0, fallback_lines_seen = 0;
-    for (int i = 0; option_vars_[i].name; ++i) {
+    for (int i = 0; option_vars_[i].member.name; ++i) {
       const config_var_t *var = &option_vars_[i];
       if (var->initvalue != NULL) {
-        if (strcmp(option_vars_[i].name, "DirAuthority") == 0) {
+        if (strcmp(option_vars_[i].member.name, "DirAuthority") == 0) {
           /*
            * Count dirauth lines we have a default for; we'll use the
            * count later to decide whether to add the defaults manually
            */
           ++dirauth_lines_seen;
         }
-        if (strcmp(option_vars_[i].name, "FallbackDir") == 0) {
+        if (strcmp(option_vars_[i].member.name, "FallbackDir") == 0) {
           /*
            * Similarly count fallback lines, so that we can decided later
            * to add the defaults manually.
@@ -8228,7 +8228,7 @@ getinfo_helper_config(control_connection_t *conn,
           ++fallback_lines_seen;
         }
         char *val = esc_for_log(var->initvalue);
-        smartlist_add_asprintf(sl, "%s %s\n",var->name,val);
+        smartlist_add_asprintf(sl, "%s %s\n",var->member.name,val);
         tor_free(val);
       }
     }

src/app/config/confparse.c

@@ -29,8 +29,7 @@
 #include "lib/confmgt/unitparse.h"
 #include "lib/container/bitarray.h"
 #include "lib/encoding/confline.h"
-
-#include "lib/confmgt/typedvar.h"
+#include "lib/confmgt/structvar.h"
 
 static void config_reset(const config_format_t *fmt, void *options,
                          const config_var_t *var, int use_defaults);
@@ -40,7 +39,7 @@ void *
 config_new(const config_format_t *fmt)
 {
   void *opts = tor_malloc_zero(fmt->size);
-  *(uint32_t*)STRUCT_VAR_P(opts, fmt->magic_offset) = fmt->magic;
+  struct_set_magic(opts, &fmt->magic);
   CONFIG_CHECK(fmt, opts);
   return opts;
 }
@@ -110,17 +109,17 @@ config_find_option_mutable(config_format_t *fmt, const char *key)
   if (!keylen)
     return NULL; /* if they say "--" on the command line, it's not an option */
   /* First, check for an exact (case-insensitive) match */
-  for (i=0; fmt->vars[i].name; ++i) {
-    if (!strcasecmp(key, fmt->vars[i].name)) {
+  for (i=0; fmt->vars[i].member.name; ++i) {
+    if (!strcasecmp(key, fmt->vars[i].member.name)) {
       return &fmt->vars[i];
     }
   }
   /* If none, check for an abbreviated match */
-  for (i=0; fmt->vars[i].name; ++i) {
-    if (!strncasecmp(key, fmt->vars[i].name, keylen)) {
+  for (i=0; fmt->vars[i].member.name; ++i) {
+    if (!strncasecmp(key, fmt->vars[i].member.name, keylen)) {
       log_warn(LD_CONFIG, "The abbreviation '%s' is deprecated. "
                "Please use '%s' instead",
-               key, fmt->vars[i].name);
+               key, fmt->vars[i].member.name);
       return &fmt->vars[i];
     }
   }
@@ -144,7 +143,7 @@ static int
 config_count_options(const config_format_t *fmt)
 {
   int i;
-  for (i=0; fmt->vars[i].name; ++i)
+  for (i=0; fmt->vars[i].member.name; ++i)
     ;
   return i;
 }
@@ -163,33 +162,14 @@ config_assign_value(const config_format_t *fmt, void *options,
                     config_line_t *c, char **msg)
 {
   const config_var_t *var;
-  void *lvalue;
 
   CONFIG_CHECK(fmt, options);
 
   var = config_find_option(fmt, c->key);
   tor_assert(var);
-  tor_assert(!strcmp(c->key, var->name));
-
-  lvalue = STRUCT_VAR_P(options, var->var_offset);
+  tor_assert(!strcmp(c->key, var->member.name));
 
-  if (var->type == CONFIG_TYPE_ROUTERSET) {
-    // XXXX make the backend extensible so that we don't have to
-    // XXXX handle ROUTERSET specially.
-
-    if (*(routerset_t**)lvalue) {
-      routerset_free(*(routerset_t**)lvalue);
-    }
-    *(routerset_t**)lvalue = routerset_new();
-    if (routerset_parse(*(routerset_t**)lvalue, c->value, c->key)<0) {
-      tor_asprintf(msg, "Invalid exit list '%s' for option '%s'",
-                   c->value, c->key);
-      return -1;
-    }
-    return 0;
-  }
-
-  return typed_var_kvassign(lvalue, c, msg, var->type);
+  return struct_var_kvassign(options, c, msg, &var->member);
 }
 
 /** Mark every linelist in <b>options</b> "fragile", so that fresh assignments
@@ -201,14 +181,14 @@ config_mark_lists_fragile(const config_format_t *fmt, void *options)
   tor_assert(fmt);
   tor_assert(options);
 
-  for (i = 0; fmt->vars[i].name; ++i) {
+  for (i = 0; fmt->vars[i].member.name; ++i) {
     const config_var_t *var = &fmt->vars[i];
     config_line_t *list;
-    if (var->type != CONFIG_TYPE_LINELIST &&
-        var->type != CONFIG_TYPE_LINELIST_V)
+    if (var->member.type != CONFIG_TYPE_LINELIST &&
+        var->member.type != CONFIG_TYPE_LINELIST_V)
       continue;
 
-    list = *(config_line_t **)STRUCT_VAR_P(options, var->var_offset);
+    list = *(config_line_t **)STRUCT_VAR_P(options, var->member.offset);
     if (list)
       list->fragile = 1;
   }
@@ -248,7 +228,7 @@ config_assign_line(const config_format_t *fmt, void *options,
   var = config_find_option(fmt, c->key);
   if (!var) {
     if (fmt->extra) {
-      void *lvalue = STRUCT_VAR_P(options, fmt->extra->var_offset);
+      void *lvalue = STRUCT_VAR_P(options, fmt->extra->offset);
       log_info(LD_CONFIG,
                "Found unrecognized option '%s'; saving it.", c->key);
       config_line_append((config_line_t**)lvalue, c->key, c->value);
@@ -261,22 +241,22 @@ config_assign_line(const config_format_t *fmt, void *options,
   }
 
   /* Put keyword into canonical case. */
-  if (strcmp(var->name, c->key)) {
+  if (strcmp(var->member.name, c->key)) {
     tor_free(c->key);
-    c->key = tor_strdup(var->name);
+    c->key = tor_strdup(var->member.name);
   }
 
   const char *deprecation_msg;
   if (warn_deprecations &&
-      (deprecation_msg = config_find_deprecation(fmt, var->name))) {
-    warn_deprecated_option(var->name, deprecation_msg);
+      (deprecation_msg = config_find_deprecation(fmt, var->member.name))) {
+    warn_deprecated_option(var->member.name, deprecation_msg);
   }
 
   if (!strlen(c->value)) {
     /* reset or clear it, then return */
     if (!clear_first) {
-      if ((var->type == CONFIG_TYPE_LINELIST ||
-           var->type == CONFIG_TYPE_LINELIST_S) &&
+      if ((var->member.type == CONFIG_TYPE_LINELIST ||
+           var->member.type == CONFIG_TYPE_LINELIST_S) &&
           c->command != CONFIG_LINE_CLEAR) {
         /* We got an empty linelist from the torrc or command line.
            As a special case, call this an error. Warn and ignore. */
@@ -293,14 +273,14 @@ config_assign_line(const config_format_t *fmt, void *options,
     config_reset(fmt, options, var, use_defaults); // LCOV_EXCL_LINE
   }
 
-  if (options_seen && (var->type != CONFIG_TYPE_LINELIST &&
-                       var->type != CONFIG_TYPE_LINELIST_S)) {
+  if (options_seen && (var->member.type != CONFIG_TYPE_LINELIST &&
+                       var->member.type != CONFIG_TYPE_LINELIST_S)) {
     /* We're tracking which options we've seen, and this option is not
      * supposed to occur more than once. */
     int var_index = (int)(var - fmt->vars);
     if (bitarray_is_set(options_seen, var_index)) {
       log_warn(LD_CONFIG, "Option '%s' used more than once; all but the last "
-               "value will be ignored.", var->name);
+               "value will be ignored.", var->member.name);
     }
     bitarray_set(options_seen, var_index);
   }
@@ -362,7 +342,6 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
                            const char *key, int escape_val)
 {
   const config_var_t *var;
-  const void *value;
   config_line_t *result;
   tor_assert(options && key);
 
@@ -373,17 +352,8 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
     log_warn(LD_CONFIG, "Unknown option '%s'.  Failing.", key);
     return NULL;
   }
-  value = STRUCT_VAR_P(options, var->var_offset);
-
-  if (var->type == CONFIG_TYPE_ROUTERSET) {
-    // XXXX make the backend extensible so that we don't have to
-    // XXXX handle ROUTERSET specially.
-    result = tor_malloc_zero(sizeof(config_line_t));
-    result->key = tor_strdup(var->name);
-    result->value = routerset_to_string(*(routerset_t**)value);
-  } else {
-    result = typed_var_kvencode(var->name, value, var->type);
-  }
+
+  result = struct_var_kvencode(options, &var->member);
 
   if (escape_val) {
     config_line_t *line;
@@ -509,19 +479,10 @@ static void
 config_clear(const config_format_t *fmt, void *options,
              const config_var_t *var)
 {
-  void *lvalue = STRUCT_VAR_P(options, var->var_offset);
+
   (void)fmt; /* unused */
-  if (var->type == CONFIG_TYPE_ROUTERSET) {
-    // XXXX make the backend extensible so that we don't have to
-    // XXXX handle ROUTERSET specially.
-    if (*(routerset_t**)lvalue) {
-      routerset_free(*(routerset_t**)lvalue);
-      *(routerset_t**)lvalue = NULL;
-    }
-    return;
-  }
 
-  typed_var_free(lvalue, var->type);
+  struct_var_free(options, &var->member);
 }
 
 /** Clear the option indexed by <b>var</b> in <b>options</b>. Then if
@@ -539,7 +500,7 @@ config_reset(const config_format_t *fmt, void *options,
     return; /* all done */
   if (var->initvalue) {
     c = tor_malloc_zero(sizeof(config_line_t));
-    c->key = tor_strdup(var->name);
+    c->key = tor_strdup(var->member.name);
     c->value = tor_strdup(var->initvalue);
     if (config_assign_value(fmt, options, c, &msg) < 0) {
       // LCOV_EXCL_START
@@ -562,10 +523,11 @@ config_free_(const config_format_t *fmt, void *options)
 
   tor_assert(fmt);
 
-  for (i=0; fmt->vars[i].name; ++i)
+  for (i=0; fmt->vars[i].member.name; ++i)
     config_clear(fmt, options, &(fmt->vars[i]));
+
   if (fmt->extra) {
-    config_line_t **linep = STRUCT_VAR_P(options, fmt->extra->var_offset);
+    config_line_t **linep = STRUCT_VAR_P(options, fmt->extra->offset);
     config_free_lines(*linep);
     *linep = NULL;
   }
@@ -580,17 +542,15 @@ config_is_same(const config_format_t *fmt,
                const void *o1, const void *o2,
                const char *name)
 {
-  config_line_t *c1, *c2;
-  int r = 1;
   CONFIG_CHECK(fmt, o1);
   CONFIG_CHECK(fmt, o2);
 
-  c1 = config_get_assigned_option(fmt, o1, name, 0);
-  c2 = config_get_assigned_option(fmt, o2, name, 0);
-  r = config_lines_eq(c1, c2);
-  config_free_lines(c1);
-  config_free_lines(c2);
-  return r;
+  const config_var_t *var = config_find_option(fmt, name);
+  if (!var) {
+    return true;
+  }
+
+  return struct_var_eq(o1, o2, &var->member);
 }
 
 /** Copy storage held by <b>old</b> into a new or_options_t and return it. */
@@ -599,27 +559,20 @@ config_dup(const config_format_t *fmt, const void *old)
 {
   void *newopts;
   int i;
-  config_line_t *line;
 
   newopts = config_new(fmt);
-  for (i=0; fmt->vars[i].name; ++i) {
-    if (fmt->vars[i].type == CONFIG_TYPE_LINELIST_S)
+  for (i=0; fmt->vars[i].member.name; ++i) {
+    if (fmt->vars[i].member.type == CONFIG_TYPE_LINELIST_S)
       continue;
-    if (fmt->vars[i].type == CONFIG_TYPE_OBSOLETE)
+    if (fmt->vars[i].member.type == CONFIG_TYPE_OBSOLETE)
       continue;
-    line = config_get_assigned_option(fmt, old, fmt->vars[i].name, 0);
-    if (line) {
-      char *msg = NULL;
-      if (config_assign(fmt, newopts, line, 0, &msg) < 0) {
-        // LCOV_EXCL_START
-        log_err(LD_BUG, "config_get_assigned_option() generated "
-                "something we couldn't config_assign(): %s", msg);
-        tor_free(msg);
-        tor_assert(0);
-        // LCOV_EXCL_STOP
-      }
+    if (struct_var_copy(newopts, old, &fmt->vars[i].member) < 0) {
+      // LCOV_EXCL_START
+      log_err(LD_BUG, "Unable to copy value for %s.",
+              fmt->vars[i].member.name);
+      tor_assert_unreached();
+      // LCOV_EXCL_STOP
     }
-    config_free_lines(line);
   }
   return newopts;
 }
@@ -632,7 +585,7 @@ config_init(const config_format_t *fmt, void *options)
   const config_var_t *var;
   CONFIG_CHECK(fmt, options);
 
-  for (i=0; fmt->vars[i].name; ++i) {
+  for (i=0; fmt->vars[i].member.name; ++i) {
     var = &fmt->vars[i];
     if (!var->initvalue)
       continue; /* defaults to NULL or 0 */
@@ -674,22 +627,23 @@ config_dump(const config_format_t *fmt, const void *default_options,
   }
 
   elements = smartlist_new();
-  for (i=0; fmt->vars[i].name; ++i) {
+  for (i=0; fmt->vars[i].member.name; ++i) {
     int comment_option = 0;
-    if (fmt->vars[i].type == CONFIG_TYPE_OBSOLETE ||
-        fmt->vars[i].type == CONFIG_TYPE_LINELIST_S)
+    if (fmt->vars[i].member.type == CONFIG_TYPE_OBSOLETE ||
+        fmt->vars[i].member.type == CONFIG_TYPE_LINELIST_S)
       continue;
     /* Don't save 'hidden' control variables. */
-    if (!strcmpstart(fmt->vars[i].name, "__"))
+    if (!strcmpstart(fmt->vars[i].member.name, "__"))
       continue;
-    if (minimal && config_is_same(fmt, options, defaults, fmt->vars[i].name))
+    if (minimal && config_is_same(fmt, options, defaults,
+                                  fmt->vars[i].member.name))
       continue;
     else if (comment_defaults &&
-             config_is_same(fmt, options, defaults, fmt->vars[i].name))
+             config_is_same(fmt, options, defaults, fmt->vars[i].member.name))
       comment_option = 1;
 
     line = assigned =
-      config_get_assigned_option(fmt, options, fmt->vars[i].name, 1);
+      config_get_assigned_option(fmt, options, fmt->vars[i].member.name, 1);
 
     for (; line; line = line->next) {
       if (!strcmpstart(line->key, "__")) {
@@ -705,7 +659,7 @@ config_dump(const config_format_t *fmt, const void *default_options,
   }
 
   if (fmt->extra) {
-    line = *(config_line_t**)STRUCT_VAR_P(options, fmt->extra->var_offset);
+    line = *(config_line_t**)STRUCT_VAR_P(options, fmt->extra->offset);
     for (; line; line = line->next) {
       smartlist_add_asprintf(elements, "%s %s\n", line->key, line->value);
     }
@@ -719,3 +673,21 @@ config_dump(const config_format_t *fmt, const void *default_options,
   }
   return result;
 }
+
+/**
+ * Return true if every member of <b>options</b> is in-range and well-formed.
+ * Return false otherwise.  Log errors at level <b>severity</b>.
+ */
+bool
+config_check_ok(const config_format_t *fmt, const void *options, int severity)
+{
+  bool all_ok = true;
+  for (int i=0; fmt->vars[i].member.name; ++i) {
+    if (!struct_var_ok(options, &fmt->vars[i].member)) {
+      log_fn(severity, LD_BUG, "Invalid value for %s",
+             fmt->vars[i].member.name);
+      all_ok = false;
+    }
+  }
+  return all_ok;
+}

src/app/config/confparse.h

@@ -34,10 +34,8 @@ typedef struct config_deprecation_t {
 
 /** A variable allowed in the configuration file or on the command line. */
 typedef struct config_var_t {
-  const char *name; /**< The full keyword (case insensitive). */
-  config_type_t type; /**< How to interpret the type and turn it into a
-                       * value. */
-  off_t var_offset; /**< Offset of the corresponding member of or_options_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
@@ -74,12 +72,12 @@ typedef struct config_var_t {
 #define CONF_TEST_MEMBERS(tp, conftype, member) \
   , CONF_CHECK_VAR_TYPE(tp, conftype, member)
 #define END_OF_CONFIG_VARS                                      \
-  { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL, { .INT=NULL } }
+  { { .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 { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }
+#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
@@ -98,9 +96,7 @@ typedef void (*free_cfg_fn_t)(void*);
  * configuration or storage format. */
 typedef struct config_format_t {
   size_t size; /**< Size of the struct that everything gets parsed into. */
-  uint32_t magic; /**< Required 'magic value' to make sure we have a struct
-                   * of the right type. */
-  off_t magic_offset; /**< Offset of the magic value within the struct. */
+  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_deprecation_t *deprecations; /** List of deprecated options */
@@ -108,17 +104,16 @@ typedef struct config_format_t {
                        * 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 is a LINELIST variable for unrecognized
+  /** If present, extra denotes a LINELIST variable for unrecognized
    * lines.  Otherwise, unrecognized lines are an error. */
-  config_var_t *extra;
+  struct_member_t *extra;
 } config_format_t;
 
 /** Macro: assert that <b>cfg</b> has the right magic field for format
  * <b>fmt</b>. */
 #define CONFIG_CHECK(fmt, cfg) STMT_BEGIN                               \
-    tor_assert(fmt && cfg);                                             \
-    tor_assert((fmt)->magic ==                                          \
-               *(uint32_t*)STRUCT_VAR_P(cfg,fmt->magic_offset));        \
+    tor_assert(fmt);                                                    \
+    struct_check_magic((cfg), &fmt->magic);                             \
   STMT_END
 
 #define CAL_USE_DEFAULTS      (1u<<0)
@@ -143,6 +138,8 @@ void *config_dup(const config_format_t *fmt, const void *old);
 char *config_dump(const config_format_t *fmt, const void *default_options,
                   const void *options, int minimal,
                   int comment_defaults);
+bool config_check_ok(const config_format_t *fmt, const void *options,
+                     int severity);
 int config_assign(const config_format_t *fmt, void *options,
                   struct config_line_t *list,
                   unsigned flags, char **msg);

src/app/config/statefile.c

@@ -71,8 +71,10 @@ static config_abbrev_t state_abbrevs_[] = {
 DUMMY_TYPECHECK_INSTANCE(or_state_t);
 
 /*XXXX these next two are duplicates or near-duplicates from config.c */
-#define VAR(name,conftype,member,initvalue)                             \
-  { name, CONFIG_TYPE_ ## conftype, offsetof(or_state_t, member),       \
+#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)                                    \
@@ -155,16 +157,20 @@ static void or_state_free_cb(void *state);
 
 /** "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 config_var_t state_extra_var = {
-  "__extra", CONFIG_TYPE_LINELIST, offsetof(or_state_t, ExtraLines), NULL
-  CONF_TEST_MEMBERS(or_state_t, LINELIST, ExtraLines)
+static struct_member_t state_extra_var = {
+  .name = "__extra",
+  .type = CONFIG_TYPE_LINELIST,
+  .offset = offsetof(or_state_t, ExtraLines),
 };
 
 /** Configuration format for or_state_t. */
 static const config_format_t state_format = {
   sizeof(or_state_t),
-  OR_STATE_MAGIC,
-  offsetof(or_state_t, magic_),
+  {
+   "or_state_t",
+   OR_STATE_MAGIC,
+   offsetof(or_state_t, magic_),
+  },
   state_abbrevs_,
   NULL,
   state_vars_,

src/feature/dirauth/shared_random_state.c

@@ -52,9 +52,11 @@ static const char dstate_cur_srv_key[] = "SharedRandCurrentValue";
 DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
 
 /* These next two are duplicates or near-duplicates from config.c */
-#define VAR(name, conftype, member, initvalue)                              \
-  { name, CONFIG_TYPE_ ## conftype, offsetof(sr_disk_state_t, member),      \
-      initvalue CONF_TEST_MEMBERS(sr_disk_state_t, conftype, member) }
+#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) \
   VAR(#member, conftype, member, initvalue)
@@ -83,17 +85,20 @@ 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 config_var_t state_extra_var = {
-  "__extra", CONFIG_TYPE_LINELIST,
-  offsetof(sr_disk_state_t, ExtraLines), NULL
-  CONF_TEST_MEMBERS(sr_disk_state_t, LINELIST, ExtraLines)
+static struct_member_t state_extra_var = {
+  .name = "__extra",
+  .type = CONFIG_TYPE_LINELIST,
+  .offset = offsetof(sr_disk_state_t, ExtraLines),
 };
 
 /* Configuration format of sr_disk_state_t. */
 static const config_format_t state_format = {
   sizeof(sr_disk_state_t),
-  SR_DISK_STATE_MAGIC,
-  offsetof(sr_disk_state_t, magic_),
+  {
+   "sr_disk_state_t",
+   SR_DISK_STATE_MAGIC,
+   offsetof(sr_disk_state_t, magic_),
+  },
   NULL,
   NULL,
   state_vars,

src/feature/nodelist/routerset.c

@@ -34,6 +34,9 @@ n * Copyright (c) 2001-2004, Roger Dingledine.
 #include "feature/nodelist/nickname.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerset.h"
+#include "lib/conf/conftypes.h"
+#include "lib/confmgt/typedvar.h"
+#include "lib/encoding/confline.h"
 #include "lib/geoip/geoip.h"
 
 #include "core/or/addr_policy_st.h"
@@ -41,6 +44,7 @@ n * Copyright (c) 2001-2004, Roger Dingledine.
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerinfo_st.h"
 #include "feature/nodelist/routerstatus_st.h"
+#include "lib/confmgt/var_type_def_st.h"
 
 /** Return a new empty routerset. */
 routerset_t *
@@ -461,3 +465,61 @@ routerset_free_(routerset_t *routerset)
   bitarray_free(routerset->countries);
   tor_free(routerset);
 }
+
+static int
+routerset_kv_parse(void *target, const config_line_t *line, char **errmsg,
+                  const void *params)
+{
+  (void)params;
+  routerset_t **p = (routerset_t**)target;
+  routerset_free(*p); // clear the old value, if any.
+  routerset_t *rs = routerset_new();
+  if (routerset_parse(rs, line->value, line->key) < 0) {
+    routerset_free(rs);
+    *errmsg = tor_strdup("Invalid router list.");
+    return -1;
+  } else {
+    *p = rs;
+    return 0;
+  }
+}
+
+static char *
+routerset_encode(const void *value, const void *params)
+{
+  (void)params;
+  const routerset_t **p = (const routerset_t**)value;
+  return routerset_to_string(*p);
+}
+
+static void
+routerset_clear(void *value, const void *params)
+{
+  (void)params;
+  routerset_t **p = (routerset_t**)value;
+  routerset_free(*p); // sets *p to NULL.
+}
+
+static int
+routerset_copy(void *dest, const void *src, const void *params)
+{
+  (void)params;
+  routerset_t **output = (routerset_t**)dest;
+  const routerset_t *input = *(routerset_t**)src;
+  routerset_free(*output); // sets *output to NULL
+  *output = routerset_new();
+  routerset_union(*output, input);
+  return 0;
+}
+
+static const var_type_fns_t routerset_type_fns = {
+  .kv_parse = routerset_kv_parse,
+  .encode = routerset_encode,
+  .clear = routerset_clear,
+  .copy = routerset_copy
+};
+
+const var_type_def_t ROUTERSET_type_defn = {
+  .name = "RouterList",
+  .fns = &routerset_type_fns
+};

src/feature/nodelist/routerset.h

@@ -44,6 +44,9 @@ void routerset_free_(routerset_t *routerset);
 #define routerset_free(rs) FREE_AND_NULL(routerset_t, routerset_free_, (rs))
 int routerset_len(const routerset_t *set);
 
+struct var_type_def_t;
+extern const struct var_type_def_t ROUTERSET_type_defn;
+
 #ifdef ROUTERSET_PRIVATE
 #include "lib/container/bitarray.h"
 

src/lib/conf/conftypes.h

@@ -63,8 +63,48 @@ typedef enum config_type_t {
   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. */
 } config_type_t;
 
+/* Forward delcaration for var_type_def_t, for extended types. */
+struct var_type_def_t;
+
+/** Structure to specify a named, typed member within a structure. */
+typedef struct struct_member_t {
+  /** Name of the field. */
+  const char *name;
+  /** Type of the field, according to the config_type_t enumeration.
+   *
+   * This value is CONFIG_TYPE_EXTENDED for any type not listed in
+   * config_type_t.
+   **/
+  config_type_t type;
+  /**
+   * Pointer to a type definition for the type of this field. Overrides
+   * <b>type</b> if not NULL.
+   **/
+  const struct var_type_def_t *type_def;
+  /**
+   * Offset of this field within the structure.  Compute this with
+   * offsetof(structure, fieldname).
+   **/
+  int offset;
+} struct_member_t;
+
+/**
+ * Structure to describe the location and preferred value of a "magic number"
+ * field within a structure.
+ *
+ * These 'magic numbers' are 32-bit values used to tag objects to make sure
+ * that they have the correct type.
+ */
+typedef struct struct_magic_decl_t {
+  const char *typename;
+  uint32_t magic_val;
+  int magic_offset;
+} struct_magic_decl_t;
+
 #ifdef TOR_UNIT_TESTS
 /**
  * Union used when building in test mode typechecking the members of a type

src/lib/confmgt/include.am

@@ -6,6 +6,7 @@ endif
 
 # ADD_C_FILE: INSERT SOURCES HERE.
 src_lib_libtor_confmgt_a_SOURCES =			\
+	src/lib/confmgt/structvar.c                      \
 	src/lib/confmgt/type_defs.c                      \
 	src/lib/confmgt/typedvar.c                      \
 	src/lib/confmgt/unitparse.c
@@ -17,6 +18,7 @@ src_lib_libtor_confmgt_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 
 # ADD_C_FILE: INSERT HEADERS HERE.
 noinst_HEADERS +=					\
+	src/lib/confmgt/structvar.h			\
 	src/lib/confmgt/type_defs.h			\
 	src/lib/confmgt/typedvar.h			\
 	src/lib/confmgt/unitparse.h			\

src/lib/confmgt/structvar.c

@@ -0,0 +1,226 @@
+/* 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 structvar.c
+ * @brief Functions to manipulate named and typed elements of
+ *    a structure.
+ *
+ * These functions represent a low-level API for accessing a member of a
+ * structure.  They use typedvar.c to work, and they are used in turn by the
+ * configuration system to examine and set fields in configuration objects
+ * used by individual modules.
+ *
+ * Almost no code should call these directly.
+ **/
+
+#include "orconfig.h"
+#include "lib/confmgt/structvar.h"
+#include "lib/cc/compat_compiler.h"
+#include "lib/conf/conftypes.h"
+#include "lib/confmgt/type_defs.h"
+#include "lib/confmgt/typedvar.h"
+#include "lib/log/util_bug.h"
+
+#include "lib/confmgt/var_type_def_st.h"
+
+#include <stddef.h>
+
+/**
+ * Set the 'magic number' on <b>object</b> to correspond to decl.
+ **/
+void
+struct_set_magic(void *object, const struct_magic_decl_t *decl)
+{
+  tor_assert(object);
+  tor_assert(decl);
+  uint32_t *ptr = STRUCT_VAR_P(object, decl->magic_offset);
+  *ptr = decl->magic_val;
+}
+
+/**
+ * Assert that the 'magic number' on <b>object</b> to corresponds to decl.
+ **/
+void
+struct_check_magic(const void *object, const struct_magic_decl_t *decl)
+{
+  tor_assert(object);
+  tor_assert(decl);
+
+  const uint32_t *ptr = STRUCT_VAR_P(object, decl->magic_offset);
+  tor_assertf(*ptr == decl->magic_val,
+              "Bad magic number on purported %s object. "
+              "Expected %"PRIu32"x but got "PRIu32"x.",
+              decl->magic_val, *ptr);
+}
+
+/**
+ * Return a mutable pointer to the member of <b>object</b> described
+ * by <b>member</b>.
+ **/
+void *
+struct_get_mptr(void *object, const struct_member_t *member)
+{
+  tor_assert(object);
+  return STRUCT_VAR_P(object, member->offset);
+}
+
+/**
+ * Return a const pointer to the member of <b>object</b> described
+ * by <b>member</b>.
+ **/
+const void *
+struct_get_ptr(const void *object, const struct_member_t *member)
+{
+  tor_assert(object);
+  return STRUCT_VAR_P(object, member->offset);
+}
+
+/**
+ * Helper: given a struct_member_t, look up the type definition for its
+ * variable.
+ */
+static const var_type_def_t *
+get_type_def(const struct_member_t *member)
+{
+  if (member->type_def)
+    return member->type_def;
+
+  return lookup_type_def(member->type);
+}
+
+/**
+ * (As typed_var_assign, but assign a value to the member of <b>object</b>
+ * defined by <b>member</b>.)
+ **/
+int
+struct_var_assign(void *object, const char *value, char **errmsg,
+                  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_assign_ex(p, value, errmsg, def);
+}
+
+/**
+ * (As typed_var_free, but free and clear the member of <b>object</b> defined
+ * by <b>member</b>.)
+ **/
+void
+struct_var_free(void *object, const struct_member_t *member)
+{
+  void *p = struct_get_mptr(object, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  typed_var_free_ex(p, def);
+}
+
+/**
+ * (As typed_var_encode, but encode the member of <b>object</b> defined
+ * by <b>member</b>.)
+ **/
+char *
+struct_var_encode(const void *object, const struct_member_t *member)
+{
+  const void *p = struct_get_ptr(object, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  return typed_var_encode_ex(p, def);
+}
+
+/**
+ * (As typed_var_copy, but copy from <b>src</b> to <b>dest</b> the member
+ * defined by <b>member</b>.)
+ **/
+int
+struct_var_copy(void *dest, const void *src, const struct_member_t *member)
+{
+  void *p_dest = struct_get_mptr(dest, member);
+  const void *p_src = struct_get_ptr(src, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  return typed_var_copy_ex(p_dest, p_src, def);
+}
+
+/**
+ * (As typed_var_eq, but compare the members of <b>a</b> and <b>b</b>
+ * defined by <b>member</b>.)
+ **/
+bool
+struct_var_eq(const void *a, const void *b, const struct_member_t *member)
+{
+  const void *p_a = struct_get_ptr(a, member);
+  const void *p_b = struct_get_ptr(b, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  return typed_var_eq_ex(p_a, p_b, def);
+}
+
+/**
+ * (As typed_var_ok, but validate the member of <b>object</b> defined by
+ * <b>member</b>.)
+ **/
+bool
+struct_var_ok(const void *object, const struct_member_t *member)
+{
+  const void *p = struct_get_ptr(object, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  return typed_var_ok_ex(p, def);
+}
+
+/**
+ * (As typed_var_kvassign, but assign a value to the member of <b>object</b>
+ * defined by <b>member</b>.)
+ **/
+int
+struct_var_kvassign(void *object, const struct config_line_t *line,
+                    char **errmsg,
+                    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_kvassign_ex(p, line, errmsg, def);
+}
+
+/**
+ * (As typed_var_kvencode, but encode the value of the member of <b>object</b>
+ * defined by <b>member</b>.)
+ **/
+struct config_line_t *
+struct_var_kvencode(const void *object, const struct_member_t *member)
+{
+  const void *p = struct_get_ptr(object, member);
+  const var_type_def_t *def = get_type_def(member);
+
+  return typed_var_kvencode_ex(member->name, p, def);
+}
+
+/**
+ * Return the official name of this struct member.
+ **/
+const char *
+struct_var_get_name(const struct_member_t *member)
+{
+  return member->name;
+}
+
+/**
+ * Return the type name for this struct member.
+ *
+ * Do not use the output of this function to inspect a type within Tor.  It is
+ * suitable for debugging, informing the controller or user of a variable's
+ * type, etc.
+ **/
+const char *
+struct_var_get_typename(const struct_member_t *member)
+{
+  const var_type_def_t *def = get_type_def(member);
+
+  return def ? def->name : NULL;
+}

src/lib/confmgt/structvar.h

@@ -0,0 +1,54 @@
+/* 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 structvar.h
+ * @brief Header for lib/confmgt/structvar.c
+ **/
+
+#ifndef TOR_LIB_CONFMGT_STRUCTVAR_H
+#define TOR_LIB_CONFMGT_STRUCTVAR_H
+
+struct struct_magic_decl_t;
+struct struct_member_t;
+struct config_line_t;
+
+#include <stdbool.h>
+
+void struct_set_magic(void *object,
+                      const struct struct_magic_decl_t *decl);
+void struct_check_magic(const void *object,
+                        const struct struct_magic_decl_t *decl);
+
+void *struct_get_mptr(void *object,
+                      const struct struct_member_t *member);
+const void *struct_get_ptr(const void *object,
+                           const struct struct_member_t *member);
+
+int struct_var_assign(void *object, const char *value, char **errmsg,
+                      const struct struct_member_t *member);
+void struct_var_free(void *object,
+                     const struct struct_member_t *member);
+char *struct_var_encode(const void *object,
+                        const struct struct_member_t *member);
+int struct_var_copy(void *dest, const void *src,
+                    const struct struct_member_t *member);
+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);
+
+const char *struct_var_get_name(const struct struct_member_t *member);
+const char *struct_var_get_typename(const struct struct_member_t *member);
+
+int struct_var_kvassign(void *object, const struct config_line_t *line,
+                        char **errmsg,
+                        const struct struct_member_t *member);
+struct config_line_t *struct_var_kvencode(
+                        const void *object,
+                        const struct struct_member_t *member);
+
+#endif /* !defined(TOR_LIB_CONFMGT_STRUCTVAR_H) */

src/lib/confmgt/typedvar.c

@@ -44,7 +44,7 @@ typed_var_assign_ex(void *target, const char *value, char **errmsg,
                     const var_type_def_t *def)
 {
   if (BUG(!def))
-    return -1;
+    return -1; // LCOV_EXCL_LINE
   // clear old value if needed.
   typed_var_free_ex(target, def);
 
@@ -67,7 +67,7 @@ typed_var_kvassign_ex(void *target, const config_line_t *line,
                       char **errmsg, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return -1;
+    return -1; // LCOV_EXCL_LINE
 
   if (def->fns->kv_parse) {
     // We do _not_ free the old value here, since linelist options
@@ -86,7 +86,7 @@ void
 typed_var_free_ex(void *target, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return;
+    return; // LCOV_EXCL_LINE
   if (def->fns->clear) {
     def->fns->clear(target, def->params);
   }
@@ -102,7 +102,7 @@ char *
 typed_var_encode_ex(const void *value, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return NULL;
+    return NULL; // LCOV_EXCL_LINE
   tor_assert(def->fns->encode);
   return def->fns->encode(value, def->params);
 }
@@ -121,7 +121,7 @@ typed_var_kvencode_ex(const char *key, const void *value,
                       const var_type_def_t *def)
 {
   if (BUG(!def))
-    return NULL;
+    return NULL; // LCOV_EXCL_LINE
   if (def->fns->kv_encode) {
     return def->fns->kv_encode(key, value, def->params);
   }
@@ -145,7 +145,7 @@ int
 typed_var_copy_ex(void *dest, const void *src, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return -1;
+    return -1; // LCOV_EXCL_LINE
   if (def->fns->copy) {
     // If we have been provided a copy fuction, use it.
     return def->fns->copy(dest, src, def);
@@ -160,8 +160,10 @@ typed_var_copy_ex(void *dest, const void *src, const var_type_def_t *def)
   char *err = NULL;
   int rv = typed_var_assign_ex(dest, enc, &err, def);
   if (BUG(rv < 0)) {
+    // LCOV_EXCL_START
     log_warn(LD_BUG, "Encoded value %s was not parseable as a %s: %s",
              escaped(enc), def->name, err?err:"");
+    // LCOV_EXCL_STOP
   }
   tor_free(err);
   tor_free(enc);
@@ -176,7 +178,7 @@ bool
 typed_var_eq_ex(const void *a, const void *b, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return false;
+    return false; // LCOV_EXCL_LINE
 
   if (def->fns->eq) {
     // Use a provided eq function if we got one.
@@ -200,7 +202,7 @@ bool
 typed_var_ok_ex(const void *value, const var_type_def_t *def)
 {
   if (BUG(!def))
-    return false;
+    return false; // LCOV_EXCL_LINE
 
   if (def->fns->ok)
     return def->fns->ok(value, def->params);

src/test/test_confparse.c

@@ -48,15 +48,17 @@ typedef struct test_struct_t {
 
 static test_struct_t test_struct_t_dummy;
 
-#define VAR(name,conftype,member,initvalue)                             \
-  { name, CONFIG_TYPE_##conftype, offsetof(test_struct_t, member),      \
+#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 )
 
-#define OBSOLETE(name)                          \
-  { name, CONFIG_TYPE_OBSOLETE, 0, NULL, {.INT=NULL} }
+#define OBSOLETE(varname)                                               \
+  { { .name=varname, .type=CONFIG_TYPE_OBSOLETE }, NULL, {.INT=NULL} }
 
 static config_var_t test_vars[] = {
   V(s, STRING, "hello"),
@@ -79,7 +81,14 @@ static config_var_t test_vars[] = {
   VAR("LineTypeA", LINELIST_S, mixed_lines, NULL),
   VAR("LineTypeB", LINELIST_S, mixed_lines, NULL),
   OBSOLETE("obsolete"),
-  V(routerset, ROUTERSET, NULL),
+  {
+   { .name = "routerset",
+     .type = CONFIG_TYPE_ROUTERSET,
+     .type_def = &ROUTERSET_type_defn,
+     .offset = offsetof(test_struct_t, routerset),
+   },
+   NULL, {.INT=NULL}
+  },
   VAR("__HiddenInt", POSINT, hidden_int, "0"),
   VAR("MixedHiddenLines", LINELIST_V, mixed_hidden_lines, NULL),
   VAR("__HiddenLineA", LINELIST_S, mixed_hidden_lines, NULL),
@@ -122,8 +131,11 @@ static void test_free_cb(void *options);
 
 static config_format_t test_fmt = {
   sizeof(test_struct_t),
-  TEST_MAGIC,
-  offsetof(test_struct_t, magic),
+  {
+   "test_struct_t",
+   TEST_MAGIC,
+   offsetof(test_struct_t, magic),
+  },
   test_abbrevs,
   test_deprecation_notes,
   test_vars,
@@ -278,6 +290,8 @@ test_confparse_assign_simple(void *arg)
   tt_str_op(tst->mixed_hidden_lines->next->value, OP_EQ, "ABC");
   tt_assert(!tst->mixed_hidden_lines->next->next);
 
+  tt_assert(config_check_ok(&test_fmt, tst, LOG_ERR));
+
  done:
   config_free(&test_fmt, tst);
 }
@@ -332,6 +346,8 @@ test_confparse_assign_deprecated(void *arg)
 
   tt_int_op(tst->deprecated_int, OP_EQ, 7);
 
+  tt_assert(config_check_ok(&test_fmt, tst, LOG_ERR));
+
  done:
   teardown_capture_of_logs();
   config_free(&test_fmt, tst);
@@ -477,6 +493,8 @@ static const badval_test_t bv_badabool =
 static const badval_test_t bv_badtime = { "time lunchtime\n", "Invalid time" };
 static const badval_test_t bv_virt = { "MixedLines 7\n", "virtual option" };
 static const badval_test_t bv_rs = { "Routerset 2.2.2.2.2\n", "Invalid" };
+static const badval_test_t bv_big_interval =
+  { "interval 1000 months", "too large" };
 
 /* Try config_dump(), and make sure it behaves correctly */
 static void
@@ -757,12 +775,19 @@ test_confparse_get_assigned(void *arg)
 /* Another variant, which accepts and stores unrecognized lines.*/
 #define ETEST_MAGIC 13371337
 
-static config_var_t extra = VAR("__extra", LINELIST, extra_lines, NULL);
+static struct_member_t extra = {
+  .name = "__extra",
+  .type = CONFIG_TYPE_LINELIST,
+  .offset = offsetof(test_struct_t, extra_lines),
+};
 
 static config_format_t etest_fmt = {
   sizeof(test_struct_t),
-  ETEST_MAGIC,
-  offsetof(test_struct_t, magic),
+  {
+   "test_struct_t (with extra lines)",
+   ETEST_MAGIC,
+   offsetof(test_struct_t, magic),
+  },
   test_abbrevs,
   test_deprecation_notes,
   test_vars,
@@ -866,6 +891,18 @@ test_confparse_unitparse(void *args)
   ;
 }
 
+static void
+test_confparse_check_ok_fail(void *arg)
+{
+  (void)arg;
+  test_struct_t *tst = config_new(&test_fmt);
+  tst->pos = -10;
+  tt_assert(! config_check_ok(&test_fmt, tst, LOG_INFO));
+
+ done:
+  config_free(&test_fmt, tst);
+}
+
 #define CONFPARSE_TEST(name, flags)                          \
   { #name, test_confparse_ ## name, flags, NULL, NULL }
 
@@ -893,6 +930,7 @@ struct testcase_t confparse_tests[] = {
   BADVAL_TEST(badtime),
   BADVAL_TEST(virt),
   BADVAL_TEST(rs),
+  BADVAL_TEST(big_interval),
   CONFPARSE_TEST(dump, 0),
   CONFPARSE_TEST(reset, 0),
   CONFPARSE_TEST(reassign, 0),
@@ -900,5 +938,6 @@ struct testcase_t confparse_tests[] = {
   CONFPARSE_TEST(get_assigned, 0),
   CONFPARSE_TEST(extra_lines, 0),
   CONFPARSE_TEST(unitparse, 0),
+  CONFPARSE_TEST(check_ok_fail, 0),
   END_OF_TESTCASES
 };