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.

1 files changed, 2 insertions, 6 deletions

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 75cb3f2e6..67ad2dfe1 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;
     }