[tor-commits] [tor/master] Turn several properties of types or variables into flags.
dgoulet at torproject.org
dgoulet at torproject.org
Mon Aug 26 13:43:23 UTC 2019
commit a7835202cf871f68854494df904058a6e644c0b0
Author: Nick Mathewson <nickm at torproject.org>
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) */
More information about the tor-commits
mailing list