commit 50aecc68ca33f53f7de086990d5b1856c4d40ab8 Author: Nick Mathewson nickm@torproject.org Date: Thu Aug 9 12:41:28 2012 -0400

Use a smarter fix for bug 1203.

Previously, we had incremented rand_bw so that when we later tested "tmp >= rand_bw", we wouldn't have an off-by-one error. But instead, it makes more sense to leave rand_bw alone and test "tmp > rand_bw".

Note that this is still safe. To take the example from the bug1203 writeup: Suppose that we have 3 nodes with bandwidth 1. So the bandwidth array is { 1, 1, 1 }, and the total bandwidth is 3. We choose rand_bw == 0, 1, or 2. With the first iteration of the loop, tmp is now 1; with the second, tmp is 2; with the third, tmp is 3. Now that our check is tmp > rand_bw, we will set i in the first iteration of the loop iff rand_bw == 0; in the second iteration of the loop iff rand_bw == 1, and in the third iff rand_bw == 2. That's what we want.

Incidentally, this change makes the bug 6538 fix more ironclad: once rand_bw is set to UINT64_MAX, tmp > rand_bw is obviously false regardless of the value of tmp. --- src/or/routerlist.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 75cb3f2..67ad2df 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1875,15 +1875,13 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl, }

rand_bw = crypto_rand_uint64(weighted_bw); - rand_bw++; /* crypto_rand_uint64() counts from 0, and we need to count - * from 1 below. See bug 1203 for details. */

/* Last, count through sl until we get to the element we picked */ i_chosen = (unsigned)smartlist_len(sl); tmp = 0; for (i=0; i < (unsigned)smartlist_len(sl); i++) { tmp += bandwidths[i]; - if (tmp >= rand_bw) { + if (tmp > rand_bw) { i_chosen = i; rand_bw = UINT64_MAX; } @@ -2118,8 +2116,6 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl,

/* Almost done: choose a random value from the bandwidth weights. */ rand_bw = crypto_rand_uint64(total_bw); - rand_bw++; /* crypto_rand_uint64() counts from 0, and we need to count - * from 1 below. See bug 1203 for details. */

/* Last, count through sl until we get to the element we picked */ tmp = 0; @@ -2138,7 +2134,7 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl, else tmp += bandwidths[i];

- if (tmp >= rand_bw) { + if (tmp > rand_bw) { i_chosen = i; rand_bw = UINT64_MAX; }