From 1f930a9a70ab5b93b5cb7a076743c6b8fc9cc4fb Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 14 Sep 2005 02:07:35 +0000 Subject: [PATCH] checkpoint: clean up and document the three ways to call config_assign() and reduce code duplication in config_free() and option_is_same(). svn:r5040 --- src/or/config.c | 208 +++++++++++++++++++++++++---------------------- src/or/control.c | 11 +-- src/or/or.h | 2 +- 3 files changed, 117 insertions(+), 104 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 7d4b77320c..6a297e61fc 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -260,10 +260,10 @@ static void config_line_append(config_line_t **lst, static void option_clear(config_format_t *fmt, or_options_t *options, config_var_t *var); static void option_reset(config_format_t *fmt, or_options_t *options, - config_var_t *var); + config_var_t *var, int use_defaults); static void config_free(config_format_t *fmt, void *options); static int option_is_same(config_format_t *fmt, - or_options_t *o1, or_options_t *o2,const char *name); + or_options_t *o1, or_options_t *o2, const char *name); static or_options_t *options_dup(config_format_t *fmt, or_options_t *old); static int options_validate(or_options_t *options); static int options_act(or_options_t *old_options); @@ -753,8 +753,14 @@ config_find_option(config_format_t *fmt, const char *key) return NULL; } +/* + * Functions to assign config options. + */ + /** c-\>key is known to be a real key. Update options * with c-\>value and return 0, or return -1 if bad value. + * + * Called from config_assign_line() and option_reset(). */ static int config_assign_value(config_format_t *fmt, or_options_t *options, @@ -827,10 +833,10 @@ config_assign_value(config_format_t *fmt, or_options_t *options, break; case CONFIG_TYPE_CSV: - if (*(smartlist_t**)lvalue) { - SMARTLIST_FOREACH(*(smartlist_t**)lvalue, char *, cp, tor_free(cp)); - smartlist_clear(*(smartlist_t**)lvalue); - } else { + if (!*(smartlist_t**)lvalue) { +// SMARTLIST_FOREACH(*(smartlist_t**)lvalue, char *, cp, tor_free(cp)); +// smartlist_clear(*(smartlist_t**)lvalue); +// } else { *(smartlist_t**)lvalue = smartlist_create(); } @@ -860,12 +866,14 @@ config_assign_value(config_format_t *fmt, or_options_t *options, * options with its value and return 0. Otherwise return -1 for bad key, * -2 for bad value. * - * If 'reset' is set, and we get a line containing no value, restore the - * option to its default value. + * If clear_first is set, clear the value first. Then if + * use_defaults is set, set the value to the default. + * + * Called from config_assign(). */ static int config_assign_line(config_format_t *fmt, or_options_t *options, - config_line_t *c, int reset) + config_line_t *c, int use_defaults, int clear_first) { config_var_t *var; @@ -883,10 +891,7 @@ config_assign_line(config_format_t *fmt, or_options_t *options, } if (!strlen(c->value)) { /* reset or clear it, then return */ - if (reset) - option_reset(fmt, options, var); - else - option_clear(fmt, options, var); + option_reset(fmt, options, var, use_defaults); return 0; } @@ -895,9 +900,11 @@ config_assign_line(config_format_t *fmt, or_options_t *options, return 0; } -/** restore the option named key in options to its default value. */ +/** Restore the option named key in options to its default value. + * Called from config_assign(). */ static void -config_reset_line(config_format_t *fmt, or_options_t *options, const char *key) +config_reset_line(config_format_t *fmt, or_options_t *options, + const char *key, int use_defaults) { config_var_t *var; @@ -907,7 +914,7 @@ config_reset_line(config_format_t *fmt, or_options_t *options, const char *key) if (!var) return; /* give error on next pass. */ - option_reset(fmt, options, var); + option_reset(fmt, options, var, use_defaults); } /** Return true iff key is a valid configuration option. */ @@ -1044,16 +1051,56 @@ get_assigned_option(config_format_t *fmt, or_options_t *options, const char *key * If an item is unrecognized, return -1 immediately, * else return 0 for success. * - * If reset, then interpret empty lines as meaning "restore to - * default value", and interpret LINELIST* options as replacing (not - * extending) their previous values. Otherwise, interpret empty lines - * as meaning "make the value 0 or null". + * If clear_first, interpret config options as replacing (not + * extending) their previous values. If clear_first is set, + * then use_defaults to decide if you set to defaults after + * clearing, or make the value 0 or NULL. * + * Here are the use cases: + * 1. A non-empty AllowUnverified line in your torrc. Appends to current. + * 2. An empty AllowUnverified line in your torrc. Should clear it. + * 3. "RESETCONF AllowUnverified" sets it to default. + * 4. "SETCONF AllowUnverified" makes it NULL. + * 5. "SETCONF AllowUnverified=foo" clears it and sets it to "foo". + * + * Use_defaults Clear_first + * 0 0 "append" + * 1 0 undefined, don't use + * 0 1 "set to null first" + * 1 1 "set to defaults first" * Return 0 on success, -1 on bad key, -2 on bad value. */ + +/* +There are three call cases for config_assign() currently. + +Case one: Torrc entry +options_init_from_torrc() calls config_assign(0, 0) + calls config_assign_line(0, 0). + if value is empty, calls option_reset(0) and returns. + calls config_assign_value(), appends. + +Case two: setconf +options_trial_assign() calls config_assign(0, 1) + calls config_reset_line(0) + calls option_reset(0) + calls option_clear(). + calls config_assign_line(0, 1). + if value is empty, calls option_reset(0) and returns. + calls config_assign_value(), appends. + +Case three: resetconf +options_trial_assign() calls config_assign(1, 1) + calls config_reset_line(1) + calls option_reset(1) + calls option_clear(). + calls config_assign_value(default) + calls config_assign_line(1, 1). + calls option_reset(1) and returns. +*/ static int -config_assign(config_format_t *fmt, - void *options, config_line_t *list, int reset) +config_assign(config_format_t *fmt, void *options, + config_line_t *list, int use_defaults, int clear_first) { config_line_t *p; @@ -1068,17 +1115,17 @@ config_assign(config_format_t *fmt, } } - /* pass 2: if we're reading from a resetting source, clear all mentioned - * linelists. */ - if (reset) { + /* pass 2: if we're reading from a resetting source, clear all + * mentioned config options, and maybe set to their defaults. */ + if (clear_first) { for (p = list; p; p = p->next) - config_reset_line(fmt, options, p->key); + config_reset_line(fmt, options, p->key, use_defaults); } /* pass 3: assign. */ while (list) { int r; - if ((r=config_assign_line(fmt, options, list, reset))) + if ((r=config_assign_line(fmt, options, list, use_defaults, clear_first))) return r; list = list->next; } @@ -1092,12 +1139,13 @@ config_assign(config_format_t *fmt, * keys, -2 on bad values, -3 on bad transition. */ int -options_trial_assign(config_line_t *list, int reset) +options_trial_assign(config_line_t *list, int use_defaults, int clear_first) { int r; or_options_t *trial_options = options_dup(&options_format, get_options()); - if ((r=config_assign(&options_format, trial_options, list, reset)) < 0) { + if ((r=config_assign(&options_format, trial_options, + list, use_defaults, clear_first)) < 0) { config_free(&options_format, trial_options); return r; } @@ -1116,7 +1164,8 @@ options_trial_assign(config_line_t *list, int reset) return 0; } -/** Reset config option var to 0, 0.0, "", or the equivalent. */ +/** Reset config option var to 0, 0.0, NULL, or the equivalent. + * Called from option_reset() and config_free(). */ static void option_clear(config_format_t *fmt, or_options_t *options, config_var_t *var) { @@ -1158,15 +1207,19 @@ option_clear(config_format_t *fmt, or_options_t *options, config_var_t *var) } } -/** Replace the option indexed by var in options with its - * default value. */ +/** Clear the option indexed by var in options. Then if + * use_defaults, set it to its default value. + * Called by config_init() and option_reset_line() and option_assign_line(). */ static void -option_reset(config_format_t *fmt, or_options_t *options, config_var_t *var) +option_reset(config_format_t *fmt, or_options_t *options, + config_var_t *var, int use_defaults) { config_line_t *c; void *lvalue; CHECK(fmt, options); option_clear(fmt, options, var); /* clear it first */ + if (!use_defaults) + return; /* all done */ lvalue = ((char*)options) + var->var_offset; if (var->initvalue) { c = tor_malloc_zero(sizeof(config_line_t)); @@ -1327,44 +1380,30 @@ static void config_free(config_format_t *fmt, void *options) { int i; - void *lvalue; tor_assert(options); - for (i=0; fmt->vars[i].name; ++i) { - lvalue = ((char*)options) + fmt->vars[i].var_offset; - switch (fmt->vars[i].type) { - case CONFIG_TYPE_MEMUNIT: - case CONFIG_TYPE_INTERVAL: - case CONFIG_TYPE_UINT: - case CONFIG_TYPE_BOOL: - case CONFIG_TYPE_DOUBLE: - case CONFIG_TYPE_ISOTIME: - case CONFIG_TYPE_OBSOLETE: - break; /* nothing to free for these config types */ - case CONFIG_TYPE_STRING: - tor_free(*(char **)lvalue); - break; - case CONFIG_TYPE_LINELIST: - case CONFIG_TYPE_LINELIST_V: - config_free_lines(*(config_line_t**)lvalue); - *(config_line_t**)lvalue = NULL; - break; - case CONFIG_TYPE_CSV: - if (*(smartlist_t**)lvalue) { - SMARTLIST_FOREACH(*(smartlist_t**)lvalue, char *, cp, tor_free(cp)); - smartlist_free(*(smartlist_t**)lvalue); - *(smartlist_t**)lvalue = NULL; - } - break; - case CONFIG_TYPE_LINELIST_S: - /* will be freed by corresponding LINELIST_V. */ - break; - } - } + for (i=0; fmt->vars[i].name; ++i) + option_clear(fmt, options, &(fmt->vars[i])); tor_free(options); } +/** Return true iff a and b contain identical keys and values in identical + * order. */ +static int +config_lines_eq(config_line_t *a, config_line_t *b) +{ + while (a && b) { + if (strcasecmp(a->key, b->key) || strcmp(a->value, b->value)) + return 0; + a = a->next; + b = b->next; + } + if (a || b) + return 0; + return 1; +} + /** Return true iff the option var has the same value in o1 * and o2. Must not be called for LINELIST_S or OBSOLETE options. */ @@ -1379,18 +1418,7 @@ option_is_same(config_format_t *fmt, c1 = get_assigned_option(fmt, o1, name); c2 = get_assigned_option(fmt, o2, name); - while (c1 && c2) { - if (strcasecmp(c1->key, c2->key) || - strcmp(c1->value, c2->value)) { - r = 0; - break; - } - c1 = c1->next; - c2 = c2->next; - } - if (r && (c1 || c2)) { - r = 0; - } + r = config_lines_eq(c1, c2); config_free_lines(c1); config_free_lines(c2); return r; @@ -1412,7 +1440,7 @@ options_dup(config_format_t *fmt, or_options_t *old) continue; line = get_assigned_option(fmt, old, fmt->vars[i].name); if (line) { - if (config_assign(fmt, newopts, line, 0) < 0) { + if (config_assign(fmt, newopts, line, 0, 0) < 0) { log_fn(LOG_WARN,"Bug: config_get_assigned_option() generated " "something we couldn't config_assign()."); tor_assert(0); @@ -1450,7 +1478,7 @@ config_init(config_format_t *fmt, void *options) var = &fmt->vars[i]; if (!var->initvalue) continue; /* defaults to NULL or 0 */ - option_reset(fmt, options, var); + option_reset(fmt, options, var, 1); } } @@ -2051,22 +2079,6 @@ options_transition_allowed(or_options_t *old, or_options_t *new_val) return 0; } -/** Return true iff a and b contain identical keys and values in identical - * order. */ -static int -config_lines_eq(config_line_t *a, config_line_t *b) -{ - while (a && b) { - if (strcasecmp(a->key, b->key) || strcmp(a->value, b->value)) - return 0; - a = a->next; - b = b->next; - } - if (a || b) - return 0; - return 1; -} - /** Return 1 if any change from old_options to new_options * will require us to rotate the cpu and dns workers; else return 0. */ static int @@ -2297,7 +2309,7 @@ options_init_from_torrc(int argc, char **argv) tor_free(cf); if (retval < 0) goto err; - retval = config_assign(&options_format, newoptions, cl, 0); + retval = config_assign(&options_format, newoptions, cl, 0, 0); config_free_lines(cl); if (retval < 0) goto err; @@ -2306,7 +2318,7 @@ options_init_from_torrc(int argc, char **argv) /* Go through command-line variables too */ if (config_get_commandlines(argc, argv, &cl) < 0) goto err; - retval = config_assign(&options_format, newoptions, cl, 0); + retval = config_assign(&options_format, newoptions, cl, 0, 0); config_free_lines(cl); if (retval < 0) goto err; @@ -3241,7 +3253,7 @@ or_state_load(void) int assign_retval; if (config_get_lines(contents, &lines)<0) goto done; - assign_retval = config_assign(&state_format, new_state, lines, 0); + assign_retval = config_assign(&state_format, new_state, lines, 0, 0); config_free_lines(lines); if (assign_retval<0) goto done; diff --git a/src/or/control.c b/src/or/control.c index 2e19069b7c..6ba291d5fa 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -605,10 +605,11 @@ get_stream(const char *id) } /** Helper for setconf and resetconf. Acts like setconf, except - * it passes reset on to options_trial_assign(). + * it passes use_defaults on to options_trial_assign(). */ static int -control_setconf_helper(connection_t *conn, uint32_t len, char *body, int reset) +control_setconf_helper(connection_t *conn, uint32_t len, char *body, + int use_defaults, int clear_first) { int r; config_line_t *lines=NULL; @@ -666,7 +667,7 @@ control_setconf_helper(connection_t *conn, uint32_t len, char *body, int reset) } } - if ((r=options_trial_assign(lines, reset)) < 0) { + if ((r=options_trial_assign(lines, use_defaults, clear_first)) < 0) { log_fn(LOG_WARN,"Controller gave us config lines that didn't validate."); if (r==-1) { if (v0) @@ -693,7 +694,7 @@ control_setconf_helper(connection_t *conn, uint32_t len, char *body, int reset) static int handle_control_setconf(connection_t *conn, uint32_t len, char *body) { - return control_setconf_helper(conn, len, body, 0); + return control_setconf_helper(conn, len, body, 0, 1); } /** Called when we receive a RESETCONF message: parse the body and try @@ -703,7 +704,7 @@ handle_control_resetconf(connection_t *conn, uint32_t len, char *body) { int v0 = STATE_IS_V0(conn->state); tor_assert(!v0); - return control_setconf_helper(conn, len, body, 1); + return control_setconf_helper(conn, len, body, 1, 1); } /** Called when we receive a GETCONF message. Parse the request, and diff --git a/src/or/or.h b/src/or/or.h index b102e766ba..441a556b1f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1444,7 +1444,7 @@ const char *safe_str(const char *address); int config_get_lines(char *string, config_line_t **result); void config_free_lines(config_line_t *front); -int options_trial_assign(config_line_t *list, int reset); +int options_trial_assign(config_line_t *list, int use_defaults, int clear_first); int resolve_my_address(or_options_t *options, uint32_t *addr, char **hostname_out); void options_init(or_options_t *options);