From 197d8550094d0509bed9e682b4a7b723dd948141 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Tue, 19 Aug 2014 22:12:19 +0300 Subject: [PATCH 1/5] Rewriting entry_is_time_to_retry() using table approach. --- changes/bug12205 | 4 ++++ src/or/entrynodes.c | 37 +++++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 changes/bug12205 diff --git a/changes/bug12205 b/changes/bug12205 new file mode 100644 index 0000000000..f71ba4133c --- /dev/null +++ b/changes/bug12205 @@ -0,0 +1,4 @@ + o Minor refactoring: + - Refactoring and unit-testing entry_is_time_to_retry() in + entrynodes.c. Resolves ticket 12205. + diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 365b9274ec..b9a0bf4a99 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -159,18 +159,35 @@ entry_guard_set_status(entry_guard_t *e, const node_t *node, static int entry_is_time_to_retry(const entry_guard_t *e, time_t now) { - long diff; + struct guard_retry_period_s { + time_t period_duration; + time_t interval_during_period; + }; + + struct guard_retry_period_s periods[] = { + { 6*60*60, 60*60 }, + { 3*24*60*60, 4*60*60 }, + { 7*24*60*60, 18*60*60 }, + { TIME_MAX, 36*60*60 } + }; + + time_t ith_deadline_for_retry; + time_t unreachable_for; + int i; + if (e->last_attempted < e->unreachable_since) return 1; - diff = now - e->unreachable_since; - if (diff < 6*60*60) - return now > (e->last_attempted + 60*60); - else if (diff < 3*24*60*60) - return now > (e->last_attempted + 4*60*60); - else if (diff < 7*24*60*60) - return now > (e->last_attempted + 18*60*60); - else - return now > (e->last_attempted + 36*60*60); + + unreachable_for = now - e->unreachable_since; + + for (i = 0; ; i++) { + if (unreachable_for <= periods[i].period_duration) { + ith_deadline_for_retry = e->last_attempted + + periods[i].interval_during_period; + + return (now > ith_deadline_for_retry); + } + } } /** Return the node corresponding to e, if e is From c731a1c68f47812f0bc536a8fab548c3c734ce28 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Tue, 19 Aug 2014 22:23:11 +0300 Subject: [PATCH 2/5] Write comments for members of periods array. --- src/or/entrynodes.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index b9a0bf4a99..ee4f447ee8 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -165,10 +165,12 @@ entry_is_time_to_retry(const entry_guard_t *e, time_t now) }; struct guard_retry_period_s periods[] = { - { 6*60*60, 60*60 }, - { 3*24*60*60, 4*60*60 }, - { 7*24*60*60, 18*60*60 }, - { TIME_MAX, 36*60*60 } + { 6*60*60, 60*60 }, /* For first 6 hrs., retry hourly; */ + { 3*24*60*60, 4*60*60 }, /* Then retry every 4 hrs. until the + 3-day mark; */ + { 7*24*60*60, 18*60*60 }, /* After 3 days, retry every 18 hours until + 1 week mark. */ + { TIME_MAX, 36*60*60 } /* After 1 week, retry every 36 hours. */ }; time_t ith_deadline_for_retry; From 8b539cc27684534a6a71cef92f9a6782040f6ca9 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 20 Aug 2014 22:07:08 +0300 Subject: [PATCH 3/5] Unit testing entry_is_time_to_retry(). --- src/or/entrynodes.c | 2 +- src/or/entrynodes.h | 3 ++ src/test/test_entrynodes.c | 79 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index ee4f447ee8..edf766bb87 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -156,7 +156,7 @@ entry_guard_set_status(entry_guard_t *e, const node_t *node, /** Return true iff enough time has passed since we last tried to connect * to the unreachable guard e that we're willing to try again. */ -static int +STATIC int entry_is_time_to_retry(const entry_guard_t *e, time_t now) { struct guard_retry_period_s { diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 5d91756aa4..52b31a225d 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -104,6 +104,9 @@ typedef enum { STATIC const node_t *entry_is_live(const entry_guard_t *e, entry_is_live_flags_t flags, const char **msg); + +STATIC int entry_is_time_to_retry(const entry_guard_t *e, time_t now); + #endif void remove_all_entry_guards(void); diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index ede93fb43a..5fee0336da 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -538,6 +538,83 @@ test_entry_guards_set_from_config(void *arg) routerset_free(options->EntryNodes); } +static void +test_entry_is_time_to_retry(void *arg) +{ + (void)arg; + + entry_guard_t *test_guard; + time_t now; + int retval; + + now = time(NULL); + + test_guard = tor_malloc(sizeof(entry_guard_t)); + memset(test_guard,0,sizeof(entry_guard_t)); + + test_guard->last_attempted = now - 10; + test_guard->unreachable_since = now - 1; + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->unreachable_since = now - (6*60*60 - 1); + test_guard->last_attempted = now - (60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->last_attempted = now - (60*60 - 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,0); + + test_guard->unreachable_since = now - (6*60*60 + 1); + test_guard->last_attempted = now - (4*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->unreachable_since = now - (3*24*60*60 - 1); + test_guard->last_attempted = now - (4*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->unreachable_since = now - (3*24*60*60 + 1); + test_guard->last_attempted = now - (18*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->unreachable_since = now - (7*24*60*60 - 1); + test_guard->last_attempted = now - (18*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->last_attempted = now - (18*60*60 - 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,0); + + test_guard->unreachable_since = now - (7*24*60*60 + 1); + test_guard->last_attempted = now - (36*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + test_guard->unreachable_since = now - (7*24*60*60 + 1); + test_guard->last_attempted = now - (36*60*60 + 1); + + retval = entry_is_time_to_retry(test_guard,now); + tt_int_op(retval,==,1); + + done: + tor_free(test_guard); + return; +} + /** XXX Do some tests that entry_is_live() */ static void test_entry_is_live(void *arg) @@ -608,6 +685,8 @@ static const struct testcase_setup_t fake_network = { }; struct testcase_t entrynodes_tests[] = { + { "entry_is_time_to_retry", test_entry_is_time_to_retry, + TT_FORK, NULL, NULL }, { "choose_random_entry_no_guards", test_choose_random_entry_no_guards, TT_FORK, &fake_network, NULL }, { "choose_random_entry_one_possibleguard", From a5fe84b5a6a2adb78ea917ad46a77b1793518cc1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 20 Aug 2014 15:31:25 -0400 Subject: [PATCH 4/5] Small cleanups to test_entry_is_time_to_retry --- src/test/test_entrynodes.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 5fee0336da..bf014f1a9d 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -541,16 +541,14 @@ test_entry_guards_set_from_config(void *arg) static void test_entry_is_time_to_retry(void *arg) { - (void)arg; - entry_guard_t *test_guard; time_t now; int retval; + (void)arg; now = time(NULL); - test_guard = tor_malloc(sizeof(entry_guard_t)); - memset(test_guard,0,sizeof(entry_guard_t)); + test_guard = tor_malloc_zero(sizeof(entry_guard_t)); test_guard->last_attempted = now - 10; test_guard->unreachable_since = now - 1; @@ -610,9 +608,8 @@ test_entry_is_time_to_retry(void *arg) retval = entry_is_time_to_retry(test_guard,now); tt_int_op(retval,==,1); - done: + done: tor_free(test_guard); - return; } /** XXX Do some tests that entry_is_live() */ From 2994f00199096860c4cbdfcb6b7d82918d637b27 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 20 Aug 2014 15:32:35 -0400 Subject: [PATCH 5/5] Whitespace fixes --- src/test/test_entrynodes.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index bf014f1a9d..00efa81768 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -561,18 +561,18 @@ test_entry_is_time_to_retry(void *arg) retval = entry_is_time_to_retry(test_guard,now); tt_int_op(retval,==,1); - + test_guard->last_attempted = now - (60*60 - 1); retval = entry_is_time_to_retry(test_guard,now); tt_int_op(retval,==,0); - + test_guard->unreachable_since = now - (6*60*60 + 1); test_guard->last_attempted = now - (4*60*60 + 1); retval = entry_is_time_to_retry(test_guard,now); tt_int_op(retval,==,1); - + test_guard->unreachable_since = now - (3*24*60*60 - 1); test_guard->last_attempted = now - (4*60*60 + 1); @@ -682,7 +682,7 @@ static const struct testcase_setup_t fake_network = { }; struct testcase_t entrynodes_tests[] = { - { "entry_is_time_to_retry", test_entry_is_time_to_retry, + { "entry_is_time_to_retry", test_entry_is_time_to_retry, TT_FORK, NULL, NULL }, { "choose_random_entry_no_guards", test_choose_random_entry_no_guards, TT_FORK, &fake_network, NULL },