commit dc6d7f072d7307c21df80b982c1d1f98130d3286 Author: Nick Mathewson nickm@torproject.org Date: Wed Oct 23 15:38:15 2019 -0400
Make foo_validate() functions call config_validate().
The former foo_validate() functions are now toplevel legacy_validate_fn callbacks. The new foo_validate() functions now call them.
This change lets us remove the old shared_random disk state validation callback entirely. --- scripts/maint/practracker/exceptions.txt | 2 +- src/app/config/config.c | 37 ++++++++++++++++++------------- src/app/config/statefile.c | 26 +++++++++++++--------- src/feature/dirauth/shared_random_state.c | 19 ---------------- src/lib/confmgt/confparse.c | 2 +- 5 files changed, 39 insertions(+), 47 deletions(-)
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 054bbbdeb..f7bd8287e 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -38,7 +38,7 @@ problem include-count /src/app/config/config.c 89 problem function-size /src/app/config/config.c:options_act_reversible() 296 problem function-size /src/app/config/config.c:options_act() 589 problem function-size /src/app/config/config.c:resolve_my_address() 190 -problem function-size /src/app/config/config.c:options_validate() 1209 +problem function-size /src/app/config/config.c:options_validate_cb() 1209 problem function-size /src/app/config/config.c:options_init_from_torrc() 207 problem function-size /src/app/config/config.c:options_init_from_string() 113 problem function-size /src/app/config/config.c:options_init_logs() 145 diff --git a/src/app/config/config.c b/src/app/config/config.c index 4e7720b0b..b372791a4 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -3239,13 +3239,20 @@ compute_publishserverdescriptor(or_options_t *options) * */ #define RECOMMENDED_MIN_CIRCUIT_BUILD_TIMEOUT (10)
-static int -options_validate_cb(const void *old_options, void *options, char **msg) +/** + * Return 0 if every setting in <b>options</b> is reasonable, is a + * permissible transition from <b>old_options</b>, and none of the + * testing-only settings differ from <b>default_options</b> unless in + * testing mode. Else return -1. Should have no side effects, except for + * normalizing the contents of <b>options</b>. + * + * On error, tor_strdup an error explanation into *<b>msg</b>. + */ +STATIC int +options_validate(const or_options_t *old_options, or_options_t *options, + char **msg) { - in_option_validation = 1; - int rv = options_validate(old_options, options, msg); - in_option_validation = 0; - return rv; + return config_validate(get_options_mgr(), old_options, options, msg); }
#define REJECT(arg) \ @@ -3431,18 +3438,16 @@ options_validate_single_onion(or_options_t *options, char **msg) return 0; }
-/** Return 0 if every setting in <b>options</b> is reasonable, is a - * permissible transition from <b>old_options</b>, and none of the - * testing-only settings differ from <b>default_options</b> unless in - * testing mode. Else return -1. Should have no side effects, except for - * normalizing the contents of <b>options</b>. - * - * On error, tor_strdup an error explanation into *<b>msg</b>. +/** + * Legacy validation/normalization callback for or_options_t. See + * legacy_validate_fn_t for more information. */ -STATIC int -options_validate(const or_options_t *old_options, or_options_t *options, - char **msg) +static int +options_validate_cb(const void *old_options_, void *options_, char **msg) { + const or_options_t *old_options = old_options_; + or_options_t *options = options_; + config_line_t *cl; const char *uname = get_uname(); int n_ports=0; diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index e277e722c..d3a0ec179 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -265,16 +265,6 @@ validate_transports_in_state(or_state_t *state) return 0; }
-static int -or_state_validate_cb(const void *old_state, void *state, char **msg) -{ - /* We don't use these; only options do. Still, we need to match that - * signature. */ - (void) old_state; - - return or_state_validate(state, msg); -} - /** Return 0 if every setting in <b>state</b> is reasonable, and a * permissible transition from <b>old_state</b>. Else warn and return -1. * Should have no side effects, except for normalizing the contents of @@ -283,6 +273,22 @@ or_state_validate_cb(const void *old_state, void *state, char **msg) static int or_state_validate(or_state_t *state, char **msg) { + return config_validate(get_state_mgr(), NULL, state, msg); +} + +/** + * Legacy validation/normalization callback for or_state_t. See + * legacy_validate_fn_t for more information. + */ +static int +or_state_validate_cb(const void *old_state, void *state_, char **msg) +{ + /* There is not a meaningful concept of a state-to-state transition, + * since we do not reload the state after we start. */ + (void) old_state; + + or_state_t *state = state_; + if (entry_guards_parse_state(state, 0, msg)<0) return -1;
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index eeb005d47..e548eb402 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -59,9 +59,6 @@ DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t); /** Our persistent state magic number. */ #define SR_DISK_STATE_MAGIC 0x98AB1254
-static int -disk_state_validate_cb(const void *old_state, void *state, char **msg); - /** Array of variables that are saved to disk as a persistent state. */ static const config_var_t state_vars[] = { V(Version, POSINT, "0"), @@ -94,7 +91,6 @@ static const config_format_t state_format = { offsetof(sr_disk_state_t, magic_), }, .vars = state_vars, - .legacy_validate_fn = disk_state_validate_cb, .extra = &state_extra_var, .config_suite_offset = -1, }; @@ -339,21 +335,6 @@ disk_state_validate(const sr_disk_state_t *state) return -1; }
-/** Validate the disk state (NOP for now). */ -static int -disk_state_validate_cb(const void *old_state, void *state, char **msg) -{ - /* We don't use these; only options do. */ - (void) old_state; - - /* This is called by config_dump which is just before we are about to - * write it to disk. At that point, our global memory state has been - * copied to the disk state so it's fair to assume it's trustable. */ - (void) state; - (void) msg; - return 0; -} - /** Parse the Commit line(s) in the disk state and translate them to the * the memory state. Return 0 on success else -1 on error. */ static int diff --git a/src/lib/confmgt/confparse.c b/src/lib/confmgt/confparse.c index 6d272ed04..f1bec76c0 100644 --- a/src/lib/confmgt/confparse.c +++ b/src/lib/confmgt/confparse.c @@ -1239,7 +1239,7 @@ config_dump(const config_mgr_t *mgr, const void *default_options,
/* XXX use a 1 here so we don't add a new log line while dumping */ if (default_options == NULL) { - if (fmt->legacy_validate_fn(NULL, defaults_tmp, &msg) < 0) { + if (config_validate(mgr, NULL, defaults_tmp, &msg) < 0) { // LCOV_EXCL_START log_err(LD_BUG, "Failed to validate default config: %s", msg); tor_free(msg);