mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-28 06:13:31 +01:00
Change the type of "download schedule" from smartlist to int.
This is done as follows:
* Only one function (find_dl_schedule()) actually returned a
smartlist. Now it returns an int.
* The CSV_INTERVAL type has been altered to ignore everything
after the first comma, and to store the value before the first
comma in an int.
This commit is contained in:
Nick Mathewson
parent
9aaed729c1
commit
2d7b5c6fe5
@ -162,8 +162,6 @@ config_assign_value(const config_format_t *fmt, void *options,
|
||||
int i, ok;
|
||||
const config_var_t *var;
|
||||
void *lvalue;
|
||||
int *csv_int;
|
||||
smartlist_t *csv_str;
|
||||
|
||||
CONFIG_CHECK(fmt, options);
|
||||
|
||||
@ -195,6 +193,30 @@ config_assign_value(const config_format_t *fmt, void *options,
|
||||
*(int *)lvalue = i;
|
||||
break;
|
||||
|
||||
case CONFIG_TYPE_CSV_INTERVAL: {
|
||||
/* We used to have entire smartlists here. But now that all of our
|
||||
* download schedules use exponential backoff, only the first part
|
||||
* matters. */
|
||||
const char *comma = strchr(c->value, ',');
|
||||
const char *val = c->value;
|
||||
char *tmp = NULL;
|
||||
if (comma) {
|
||||
tmp = tor_strndup(c->value, comma - c->value);
|
||||
val = tmp;
|
||||
}
|
||||
|
||||
i = config_parse_interval(val, &ok);
|
||||
if (!ok) {
|
||||
tor_asprintf(msg,
|
||||
"Interval '%s %s' is malformed or out of bounds.",
|
||||
c->key, c->value);
|
||||
return -1;
|
||||
}
|
||||
*(int *)lvalue = i;
|
||||
tor_free(tmp);
|
||||
break;
|
||||
}
|
||||
|
||||
case CONFIG_TYPE_INTERVAL: {
|
||||
i = config_parse_interval(c->value, &ok);
|
||||
if (!ok) {
|
||||
@ -298,36 +320,6 @@ config_assign_value(const config_format_t *fmt, void *options,
|
||||
SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
|
||||
break;
|
||||
|
||||
case CONFIG_TYPE_CSV_INTERVAL:
|
||||
if (*(smartlist_t**)lvalue) {
|
||||
SMARTLIST_FOREACH(*(smartlist_t**)lvalue, int *, cp, tor_free(cp));
|
||||
smartlist_clear(*(smartlist_t**)lvalue);
|
||||
} else {
|
||||
*(smartlist_t**)lvalue = smartlist_new();
|
||||
}
|
||||
csv_str = smartlist_new();
|
||||
smartlist_split_string(csv_str, c->value, ",",
|
||||
SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
|
||||
SMARTLIST_FOREACH_BEGIN(csv_str, char *, str)
|
||||
{
|
||||
i = config_parse_interval(str, &ok);
|
||||
if (!ok) {
|
||||
tor_asprintf(msg,
|
||||
"Interval in '%s %s' is malformed or out of bounds.",
|
||||
c->key, c->value);
|
||||
SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
|
||||
smartlist_free(csv_str);
|
||||
return -1;
|
||||
}
|
||||
csv_int = tor_malloc_zero(sizeof(int));
|
||||
*csv_int = i;
|
||||
smartlist_add(*(smartlist_t**)lvalue, csv_int);
|
||||
}
|
||||
SMARTLIST_FOREACH_END(str);
|
||||
SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
|
||||
smartlist_free(csv_str);
|
||||
break;
|
||||
|
||||
case CONFIG_TYPE_LINELIST:
|
||||
case CONFIG_TYPE_LINELIST_S:
|
||||
{
|
||||
@ -528,7 +520,6 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
|
||||
const config_var_t *var;
|
||||
const void *value;
|
||||
config_line_t *result;
|
||||
smartlist_t *csv_str;
|
||||
tor_assert(options && key);
|
||||
|
||||
CONFIG_CHECK(fmt, options);
|
||||
@ -571,6 +562,7 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
|
||||
break;
|
||||
}
|
||||
/* fall through */
|
||||
case CONFIG_TYPE_CSV_INTERVAL:
|
||||
case CONFIG_TYPE_INTERVAL:
|
||||
case CONFIG_TYPE_MSEC_INTERVAL:
|
||||
case CONFIG_TYPE_UINT:
|
||||
@ -611,20 +603,6 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
|
||||
else
|
||||
result->value = tor_strdup("");
|
||||
break;
|
||||
case CONFIG_TYPE_CSV_INTERVAL:
|
||||
if (*(smartlist_t**)value) {
|
||||
csv_str = smartlist_new();
|
||||
SMARTLIST_FOREACH_BEGIN(*(smartlist_t**)value, int *, i)
|
||||
{
|
||||
smartlist_add_asprintf(csv_str, "%d", *i);
|
||||
}
|
||||
SMARTLIST_FOREACH_END(i);
|
||||
result->value = smartlist_join_strings(csv_str, ",", 0, NULL);
|
||||
SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
|
||||
smartlist_free(csv_str);
|
||||
} else
|
||||
result->value = tor_strdup("");
|
||||
break;
|
||||
case CONFIG_TYPE_OBSOLETE:
|
||||
log_fn(LOG_INFO, LD_CONFIG,
|
||||
"You asked me for the value of an obsolete config option '%s'.",
|
||||
@ -789,6 +767,7 @@ config_clear(const config_format_t *fmt, void *options,
|
||||
case CONFIG_TYPE_ISOTIME:
|
||||
*(time_t*)lvalue = 0;
|
||||
break;
|
||||
case CONFIG_TYPE_CSV_INTERVAL:
|
||||
case CONFIG_TYPE_INTERVAL:
|
||||
case CONFIG_TYPE_MSEC_INTERVAL:
|
||||
case CONFIG_TYPE_UINT:
|
||||
@ -816,13 +795,6 @@ config_clear(const config_format_t *fmt, void *options,
|
||||
*(smartlist_t **)lvalue = NULL;
|
||||
}
|
||||
break;
|
||||
case CONFIG_TYPE_CSV_INTERVAL:
|
||||
if (*(smartlist_t**)lvalue) {
|
||||
SMARTLIST_FOREACH(*(smartlist_t **)lvalue, int *, cp, tor_free(cp));
|
||||
smartlist_free(*(smartlist_t **)lvalue);
|
||||
*(smartlist_t **)lvalue = NULL;
|
||||
}
|
||||
break;
|
||||
case CONFIG_TYPE_LINELIST:
|
||||
case CONFIG_TYPE_LINELIST_S:
|
||||
config_free_lines(*(config_line_t **)lvalue);
|
||||
|
||||
@ -62,7 +62,7 @@ typedef union {
|
||||
int *AUTOBOOL;
|
||||
time_t *ISOTIME;
|
||||
smartlist_t **CSV;
|
||||
smartlist_t **CSV_INTERVAL;
|
||||
int *CSV_INTERVAL;
|
||||
config_line_t **LINELIST;
|
||||
config_line_t **LINELIST_S;
|
||||
config_line_t **LINELIST_V;
|
||||
|
||||
@ -5303,7 +5303,7 @@ connection_dir_finished_connecting(dir_connection_t *conn)
|
||||
* Then return a list of int pointers defining download delays in seconds.
|
||||
* Helper function for download_status_increment_failure(),
|
||||
* download_status_reset(), and download_status_increment_attempt(). */
|
||||
STATIC const smartlist_t *
|
||||
STATIC int
|
||||
find_dl_schedule(const download_status_t *dls, const or_options_t *options)
|
||||
{
|
||||
switch (dls->schedule) {
|
||||
@ -5359,25 +5359,19 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options)
|
||||
}
|
||||
|
||||
/* Impossible, but gcc will fail with -Werror without a `return`. */
|
||||
return NULL;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/** Decide which minimum delay step we want to use based on
|
||||
* descriptor type in <b>dls</b> and <b>options</b>.
|
||||
* Helper function for download_status_schedule_get_delay(). */
|
||||
STATIC int
|
||||
find_dl_min_delay(download_status_t *dls, const or_options_t *options)
|
||||
find_dl_min_delay(const download_status_t *dls, const or_options_t *options)
|
||||
{
|
||||
tor_assert(dls);
|
||||
tor_assert(options);
|
||||
|
||||
/*
|
||||
* For now, just use the existing schedule config stuff and pick the
|
||||
* first/last entries off to get min/max delay for backoff purposes
|
||||
*/
|
||||
const smartlist_t *schedule = find_dl_schedule(dls, options);
|
||||
tor_assert(schedule != NULL && smartlist_len(schedule) >= 2);
|
||||
return *(int *)(smartlist_get(schedule, 0));
|
||||
return find_dl_schedule(dls, options);
|
||||
}
|
||||
|
||||
/** As next_random_exponential_delay() below, but does not compute a random
|
||||
@ -5634,10 +5628,9 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
|
||||
static time_t
|
||||
download_status_get_initial_delay_from_now(const download_status_t *dls)
|
||||
{
|
||||
const smartlist_t *schedule = find_dl_schedule(dls, get_options());
|
||||
/* We use constant initial delays, even in exponential backoff
|
||||
* schedules. */
|
||||
return time(NULL) + *(int *)smartlist_get(schedule, 0);
|
||||
return time(NULL) + find_dl_min_delay(dls, get_options());
|
||||
}
|
||||
|
||||
/** Reset <b>dls</b> so that it will be considered downloadable
|
||||
|
||||
@ -259,9 +259,9 @@ STATIC char* authdir_type_to_string(dirinfo_type_t auth);
|
||||
STATIC const char * dir_conn_purpose_to_string(int purpose);
|
||||
STATIC int should_use_directory_guards(const or_options_t *options);
|
||||
STATIC compression_level_t choose_compression_level(ssize_t n_bytes);
|
||||
STATIC const smartlist_t *find_dl_schedule(const download_status_t *dls,
|
||||
const or_options_t *options);
|
||||
STATIC int find_dl_min_delay(download_status_t *dls,
|
||||
STATIC int find_dl_schedule(const download_status_t *dls,
|
||||
const or_options_t *options);
|
||||
STATIC int find_dl_min_delay(const download_status_t *dls,
|
||||
const or_options_t *options);
|
||||
|
||||
STATIC int next_random_exponential_delay(int delay,
|
||||
|
||||
@ -4063,34 +4063,19 @@ test_dir_download_status_increment(void *arg)
|
||||
DL_WANT_ANY_DIRSERVER,
|
||||
DL_SCHED_INCREMENT_ATTEMPT,
|
||||
0, 0 };
|
||||
int no_delay = 0;
|
||||
int delay0 = -1;
|
||||
int delay1 = -1;
|
||||
int delay2 = -1;
|
||||
smartlist_t *schedule = smartlist_new();
|
||||
smartlist_t *schedule_no_initial_delay = smartlist_new();
|
||||
or_options_t test_options;
|
||||
time_t current_time = time(NULL);
|
||||
|
||||
/* Provide some values for the schedules */
|
||||
delay0 = 10;
|
||||
delay1 = 99;
|
||||
delay2 = 20;
|
||||
|
||||
/* Make the schedules */
|
||||
smartlist_add(schedule, (void *)&delay0);
|
||||
smartlist_add(schedule, (void *)&delay1);
|
||||
smartlist_add(schedule, (void *)&delay2);
|
||||
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&no_delay);
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&delay1);
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&delay2);
|
||||
const int delay0 = 10;
|
||||
const int no_delay = 0;
|
||||
const int schedule = 10;
|
||||
const int schedule_no_initial_delay = 0;
|
||||
|
||||
/* Put it in the options */
|
||||
mock_options = &test_options;
|
||||
reset_options(mock_options, &mock_get_options_calls);
|
||||
mock_options->TestingBridgeBootstrapDownloadSchedule = schedule;
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
mock_options->TestingBridgeBootstrapDownloadInitialDelay = schedule;
|
||||
mock_options->TestingClientDownloadInitialDelay = schedule;
|
||||
|
||||
MOCK(get_options, mock_get_options);
|
||||
|
||||
@ -4098,13 +4083,13 @@ test_dir_download_status_increment(void *arg)
|
||||
* whether or not it was reset before being used */
|
||||
|
||||
/* regression test for 17750: no initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
|
||||
mock_options->TestingClientDownloadInitialDelay = schedule_no_initial_delay;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
|
||||
/* regression test for 17750: exponential, no initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
|
||||
mock_options->TestingClientDownloadInitialDelay = schedule_no_initial_delay;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
@ -4117,7 +4102,7 @@ test_dir_download_status_increment(void *arg)
|
||||
tt_int_op(mock_get_options_calls, OP_GE, 1);
|
||||
|
||||
/* regression test for 17750: exponential, initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
mock_options->TestingClientDownloadInitialDelay = schedule;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
@ -4130,9 +4115,6 @@ test_dir_download_status_increment(void *arg)
|
||||
tt_int_op(mock_get_options_calls, OP_GE, 1);
|
||||
|
||||
done:
|
||||
/* the pointers in schedule are allocated on the stack */
|
||||
smartlist_free(schedule);
|
||||
smartlist_free(schedule_no_initial_delay);
|
||||
UNMOCK(get_options);
|
||||
mock_options = NULL;
|
||||
mock_get_options_calls = 0;
|
||||
@ -5483,44 +5465,45 @@ test_dir_find_dl_schedule(void* data)
|
||||
mock_num_bridges_usable);
|
||||
|
||||
download_status_t dls;
|
||||
smartlist_t server, client, server_cons, client_cons;
|
||||
smartlist_t client_boot_auth_only_cons, client_boot_auth_cons;
|
||||
smartlist_t client_boot_fallback_cons, bridge, bridge_bootstrap;
|
||||
|
||||
const int server=10, client=20, server_cons=30, client_cons=40;
|
||||
const int client_boot_auth_only_cons=50, client_boot_auth_cons=60;
|
||||
const int client_boot_fallback_cons=70, bridge=80, bridge_bootstrap=90;
|
||||
|
||||
mock_options = tor_malloc(sizeof(or_options_t));
|
||||
reset_options(mock_options, &mock_get_options_calls);
|
||||
MOCK(get_options, mock_get_options);
|
||||
|
||||
mock_options->TestingServerDownloadSchedule = &server;
|
||||
mock_options->TestingClientDownloadSchedule = &client;
|
||||
mock_options->TestingServerConsensusDownloadSchedule = &server_cons;
|
||||
mock_options->TestingClientConsensusDownloadSchedule = &client_cons;
|
||||
mock_options->ClientBootstrapConsensusAuthorityOnlyDownloadSchedule =
|
||||
&client_boot_auth_only_cons;
|
||||
mock_options->ClientBootstrapConsensusAuthorityDownloadSchedule =
|
||||
&client_boot_auth_cons;
|
||||
mock_options->ClientBootstrapConsensusFallbackDownloadSchedule =
|
||||
&client_boot_fallback_cons;
|
||||
mock_options->TestingBridgeDownloadSchedule = &bridge;
|
||||
mock_options->TestingBridgeBootstrapDownloadSchedule = &bridge_bootstrap;
|
||||
mock_options->TestingServerDownloadInitialDelay = server;
|
||||
mock_options->TestingClientDownloadInitialDelay = client;
|
||||
mock_options->TestingServerConsensusDownloadInitialDelay = server_cons;
|
||||
mock_options->TestingClientConsensusDownloadInitialDelay = client_cons;
|
||||
mock_options->ClientBootstrapConsensusAuthorityOnlyDownloadInitialDelay =
|
||||
client_boot_auth_only_cons;
|
||||
mock_options->ClientBootstrapConsensusAuthorityDownloadInitialDelay =
|
||||
client_boot_auth_cons;
|
||||
mock_options->ClientBootstrapConsensusFallbackDownloadInitialDelay =
|
||||
client_boot_fallback_cons;
|
||||
mock_options->TestingBridgeDownloadInitialDelay = bridge;
|
||||
mock_options->TestingBridgeBootstrapDownloadInitialDelay = bridge_bootstrap;
|
||||
|
||||
dls.schedule = DL_SCHED_GENERIC;
|
||||
/* client */
|
||||
mock_options->ClientOnly = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &client);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, client);
|
||||
mock_options->ClientOnly = 0;
|
||||
|
||||
/* dir mode */
|
||||
mock_options->DirPort_set = 1;
|
||||
mock_options->DirCache = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &server);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, server);
|
||||
mock_options->DirPort_set = 0;
|
||||
mock_options->DirCache = 0;
|
||||
|
||||
dls.schedule = DL_SCHED_CONSENSUS;
|
||||
/* public server mode */
|
||||
mock_options->ORPort_set = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &server_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, server_cons);
|
||||
mock_options->ORPort_set = 0;
|
||||
|
||||
/* client and bridge modes */
|
||||
@ -5529,30 +5512,30 @@ test_dir_find_dl_schedule(void* data)
|
||||
dls.want_authority = 1;
|
||||
/* client */
|
||||
mock_options->ClientOnly = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_auth_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_auth_cons);
|
||||
mock_options->ClientOnly = 0;
|
||||
|
||||
/* bridge relay */
|
||||
mock_options->ORPort_set = 1;
|
||||
mock_options->BridgeRelay = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_auth_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_auth_cons);
|
||||
mock_options->ORPort_set = 0;
|
||||
mock_options->BridgeRelay = 0;
|
||||
|
||||
dls.want_authority = 0;
|
||||
/* client */
|
||||
mock_options->ClientOnly = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_fallback_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_fallback_cons);
|
||||
mock_options->ClientOnly = 0;
|
||||
|
||||
/* bridge relay */
|
||||
mock_options->ORPort_set = 1;
|
||||
mock_options->BridgeRelay = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_fallback_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_fallback_cons);
|
||||
mock_options->ORPort_set = 0;
|
||||
mock_options->BridgeRelay = 0;
|
||||
|
||||
@ -5560,30 +5543,30 @@ test_dir_find_dl_schedule(void* data)
|
||||
/* dls.want_authority is ignored */
|
||||
/* client */
|
||||
mock_options->ClientOnly = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_auth_only_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_auth_only_cons);
|
||||
mock_options->ClientOnly = 0;
|
||||
|
||||
/* bridge relay */
|
||||
mock_options->ORPort_set = 1;
|
||||
mock_options->BridgeRelay = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_boot_auth_only_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_boot_auth_only_cons);
|
||||
mock_options->ORPort_set = 0;
|
||||
mock_options->BridgeRelay = 0;
|
||||
}
|
||||
} else {
|
||||
/* client */
|
||||
mock_options->ClientOnly = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_cons);
|
||||
mock_options->ClientOnly = 0;
|
||||
|
||||
/* bridge relay */
|
||||
mock_options->ORPort_set = 1;
|
||||
mock_options->BridgeRelay = 1;
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
&client_cons);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
|
||||
client_cons);
|
||||
mock_options->ORPort_set = 0;
|
||||
mock_options->BridgeRelay = 0;
|
||||
}
|
||||
@ -5593,9 +5576,9 @@ test_dir_find_dl_schedule(void* data)
|
||||
mock_options->ClientOnly = 1;
|
||||
mock_options->UseBridges = 1;
|
||||
if (num_bridges_usable(0) > 0) {
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, bridge);
|
||||
} else {
|
||||
tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge_bootstrap);
|
||||
tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, bridge_bootstrap);
|
||||
}
|
||||
|
||||
done:
|
||||
|
||||
Loading…
Reference in New Issue
Block a user