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.
This commit is contained in:
Nick Mathewson 2012-08-09 12:41:28 -04:00
parent 640a51684c
commit 50aecc68ca

View File

@ -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;
}