Add a "freeze" function for config_mgr_t objects.

It's important to make sure that we don't change a config_mgr_t
after we start using it to make objects, or we could get into
inconsistent states.  This feature is the start of a safety
mechanism to prevent this problem.
This commit is contained in:
Nick Mathewson 2019-07-23 09:08:31 -04:00
parent 7abd43ac5f
commit dde091ebc7
6 changed files with 39 additions and 0 deletions

View File

@ -904,6 +904,7 @@ get_options_mgr(void)
{ {
if (PREDICT_UNLIKELY(options_mgr == NULL)) { if (PREDICT_UNLIKELY(options_mgr == NULL)) {
options_mgr = config_mgr_new(&options_format); options_mgr = config_mgr_new(&options_format);
config_mgr_freeze(options_mgr);
} }
return options_mgr; return options_mgr;
} }

View File

@ -85,6 +85,10 @@ struct config_mgr_t {
smartlist_t *all_abbrevs; smartlist_t *all_abbrevs;
/** A smartlist of config_deprecation_t for all configuration formats. */ /** A smartlist of config_deprecation_t for all configuration formats. */
smartlist_t *all_deprecations; smartlist_t *all_deprecations;
/** True if this manager has been frozen and cannot have any more formats
* added to it. A manager must be frozen before it can be used to construct
* or manipulate objects. */
bool frozen;
}; };
#define IDX_TOPLEVEL (-1) #define IDX_TOPLEVEL (-1)
@ -112,6 +116,10 @@ config_mgr_register_fmt(config_mgr_t *mgr,
{ {
int i; int i;
tor_assertf(!mgr->frozen,
"Tried to add a format to a configuration manager after "
"it had been frozen.");
/* register variables */ /* register variables */
for (i = 0; fmt->vars[i].member.name; ++i) { for (i = 0; fmt->vars[i].member.name; ++i) {
managed_var_t *mv = tor_malloc_zero(sizeof(managed_var_t)); managed_var_t *mv = tor_malloc_zero(sizeof(managed_var_t));
@ -161,6 +169,16 @@ config_mgr_get_obj(const config_mgr_t *mgr, const void *toplevel, int idx)
return config_mgr_get_obj_mutable(mgr, (void*)toplevel, idx); return config_mgr_get_obj_mutable(mgr, (void*)toplevel, idx);
} }
/**
* Mark a configuration manager as "frozen", so that no more formats can be
* added, and so that it can be used for manipulating configuration objects.
**/
void
config_mgr_freeze(config_mgr_t *mgr)
{
mgr->frozen = true;
}
/** Release all storage held in <b>mgr</b> */ /** Release all storage held in <b>mgr</b> */
void void
config_mgr_free_(config_mgr_t *mgr) config_mgr_free_(config_mgr_t *mgr)
@ -208,6 +226,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr,
{ {
tor_assert(mgr); tor_assert(mgr);
tor_assert(options); tor_assert(options);
tor_assert(mgr->frozen);
struct_check_magic(options, &mgr->toplevel->magic); struct_check_magic(options, &mgr->toplevel->magic);
} }
@ -221,6 +240,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr,
void * void *
config_new(const config_mgr_t *mgr) config_new(const config_mgr_t *mgr)
{ {
tor_assert(mgr->frozen);
const config_format_t *fmt = mgr->toplevel; const config_format_t *fmt = mgr->toplevel;
void *opts = tor_malloc_zero(fmt->size); void *opts = tor_malloc_zero(fmt->size);
struct_set_magic(opts, &fmt->magic); struct_set_magic(opts, &fmt->magic);

View File

@ -73,6 +73,7 @@ typedef struct config_mgr_t config_mgr_t;
config_mgr_t *config_mgr_new(const config_format_t *toplevel_fmt); config_mgr_t *config_mgr_new(const config_format_t *toplevel_fmt);
void config_mgr_free_(config_mgr_t *mgr); void config_mgr_free_(config_mgr_t *mgr);
void config_mgr_freeze(config_mgr_t *mgr);
#define config_mgr_free(mgr) \ #define config_mgr_free(mgr) \
FREE_AND_NULL(config_mgr_t, config_mgr_free_, (mgr)) FREE_AND_NULL(config_mgr_t, config_mgr_free_, (mgr))
struct smartlist_t *config_mgr_list_vars(const config_mgr_t *mgr); struct smartlist_t *config_mgr_list_vars(const config_mgr_t *mgr);

View File

@ -183,6 +183,7 @@ get_state_mgr(void)
{ {
if (PREDICT_UNLIKELY(state_mgr == NULL)) { if (PREDICT_UNLIKELY(state_mgr == NULL)) {
state_mgr = config_mgr_new(&state_format); state_mgr = config_mgr_new(&state_format);
config_mgr_freeze(state_mgr);
} }
return state_mgr; return state_mgr;
} }

View File

@ -112,6 +112,7 @@ get_srs_mgr(void)
{ {
if (PREDICT_UNLIKELY(shared_random_state_mgr == NULL)) { if (PREDICT_UNLIKELY(shared_random_state_mgr == NULL)) {
shared_random_state_mgr = config_mgr_new(&state_format); shared_random_state_mgr = config_mgr_new(&state_format);
config_mgr_freeze(shared_random_state_mgr);
} }
return shared_random_state_mgr; return shared_random_state_mgr;
} }

View File

@ -153,6 +153,7 @@ test_confparse_init(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = config_new(mgr); test_struct_t *tst = config_new(mgr);
config_init(mgr, tst); config_init(mgr, tst);
@ -239,6 +240,7 @@ test_confparse_assign_simple(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
tt_str_op(tst->s, OP_EQ, "this is a"); tt_str_op(tst->s, OP_EQ, "this is a");
@ -300,6 +302,7 @@ test_confparse_assign_obsolete(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -330,6 +333,7 @@ test_confparse_assign_deprecated(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -364,6 +368,7 @@ test_confparse_assign_replaced(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -395,6 +400,7 @@ test_confparse_assign_emptystring(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -423,6 +429,7 @@ test_confparse_assign_twice(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -458,6 +465,7 @@ test_confparse_assign_badval(void *arg)
{ {
const badval_test_t *bt = arg; const badval_test_t *bt = arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -512,6 +520,7 @@ test_confparse_dump(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
char *dumped = NULL; char *dumped = NULL;
@ -603,6 +612,7 @@ test_confparse_reset(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_reset_line(mgr, tst, "interval", 0); config_reset_line(mgr, tst, "interval", 0);
@ -623,6 +633,7 @@ test_confparse_reassign(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL, *rs = NULL; char *msg = NULL, *rs = NULL;
@ -679,6 +690,7 @@ test_confparse_reassign_extend(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL; char *msg = NULL;
@ -738,6 +750,7 @@ test_confparse_get_assigned(void *arg)
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = get_simple_config(mgr); test_struct_t *tst = get_simple_config(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
@ -824,6 +837,7 @@ test_confparse_extra_lines(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&etest_fmt); config_mgr_t *mgr = config_mgr_new(&etest_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = config_new(mgr); test_struct_t *tst = config_new(mgr);
config_line_t *lines = NULL; config_line_t *lines = NULL;
char *msg = NULL, *dump = NULL; char *msg = NULL, *dump = NULL;
@ -919,6 +933,7 @@ test_confparse_check_ok_fail(void *arg)
{ {
(void)arg; (void)arg;
config_mgr_t *mgr = config_mgr_new(&test_fmt); config_mgr_t *mgr = config_mgr_new(&test_fmt);
config_mgr_freeze(mgr);
test_struct_t *tst = config_new(mgr); test_struct_t *tst = config_new(mgr);
tst->pos = -10; tst->pos = -10;
tt_assert(! config_check_ok(mgr, tst, LOG_INFO)); tt_assert(! config_check_ok(mgr, tst, LOG_INFO));