commit a7835202cf871f68854494df904058a6e644c0b0 Author: Nick Mathewson nickm@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) */