[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Turn several properties of types or variables into flags.



commit a7835202cf871f68854494df904058a6e644c0b0
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Fri Jun 21 09:58:40 2019 -0400

    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 5f7a82d15..d240a73fe 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 2890d8c81..0d19974d7 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 6d63ba3e0..c53e3224d 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 ff284e681..aa89965e6 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 028a88991..6a44fb92e 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 38f8e5dd7..97a8fb363 100644
--- a/src/lib/confmgt/structvar.c
+++ b/src/lib/confmgt/structvar.c
@@ -202,6 +202,19 @@ struct_var_kvencode(const void *object, const struct_member_t *member)
 }
 
 /**
+ * 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.
  **/
 const char *
@@ -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 92b9b6fc7..e6dbc6d6e 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 62b4c1019..f8b2681aa 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 c2b9b4572..3cba07539 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 720ad54fc..2e36f9d67 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 c63ff66ef..c4f7acec6 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) */



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits