Skip to content
Snippets Groups Projects
Commit 39640728 authored by Nick Mathewson's avatar Nick Mathewson :game_die:
Browse files

Add comments to try to prevent recurrence of #31495.

There is a bad design choice in two of our configuration types,
where the empty string encodes a value that is not the same as the
default value.  This design choice, plus an implementation mistake,
meant that config_dup() did not preserve the value of routerset_t,
and thereby caused bug #31495.

This comment-only patch documents the two types with the problem,
and suggests that implementors try to avoid it in the future.

Closes ticket 31907.
parent 53116ca0
No related branches found
No related tags found
No related merge requests found
......@@ -473,6 +473,13 @@ routerset_free_(routerset_t *routerset)
* routerset_t** passed as <b>target</b>. On success return 0; on failure
* return -1 and store an error message into *<b>errmsg</b>.
**/
/*
* Warning: For this type, the default value (NULL) and "" are sometimes
* considered different values. That is generally risky, and best avoided for
* other types in the future. For cases where we want the default to be "all
* routers" (like EntryNodes) we should add a new routerset value indicating
* "all routers" (see #31908)
*/
static int
routerset_kv_parse(void *target, const config_line_t *line, char **errmsg,
const void *params)
......
......@@ -44,6 +44,10 @@
// CONFIG_TYPE_FILENAME
//
// These two types are the same for now, but they have different names.
//
// Warning: For this type, the default value (NULL) and "" are considered
// different values. That is generally risky, and best avoided for other
// types in the future.
//////
static int
......
......@@ -39,6 +39,12 @@ struct config_line_t;
* All functions here take a <b>params</b> argument, whose value
* is determined by the type definition. Two types may have the
* same functions, but differ only in parameters.
*
* Implementation considerations: If "" encodes a valid value for a type, try
* to make sure that it encodes the same thing as the default value for the
* type (that is, the value that is set by config_clear() or memset(0)). If
* this is not the case, you need to make extra certain that your parse/encode
* implementations preserve the NULL/"" distinction.
**/
struct var_type_fns_t {
/**
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment