improve code comments, based on comments from nick

3 files changed, 26 insertions, 19 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index dd92e78cb5..2b4540bf3b 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -4605,7 +4605,7 @@ entries_retry_helper(or_options_t *options, int act)
         if (ri->is_running)
           any_running = 1; /* some entry is both known and running */
         else if (act) {
-          /* Mark-for-close all TLS connections to this node, since
+          /* Mark all current connections to this OR as unhealthy, since
            * otherwise there could be one that started 30 seconds
            * ago, and in 30 seconds it will time out, causing us to mark
            * the node down and undermine the retry attempt. We mark even
@@ -4613,7 +4613,7 @@ entries_retry_helper(or_options_t *options, int act)
            * we'll want to attach circuits to fresh conns. */
           connection_or_set_bad_connections(ri->cache_info.identity_digest, 1);
 
-          /* mark it for retry */
+          /* mark this entry node for retry */
           router_set_status(ri->cache_info.identity_digest, 1);
           e->can_retry = 1;
           e->bad_since = 0;
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index f651ef7c33..f369678ab0 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -959,9 +959,13 @@ circuit_build_failed(origin_circuit_t *circ)
     if (circ->_base.n_conn) {
       or_connection_t *n_conn = circ->_base.n_conn;
       if (n_conn->is_bad_for_new_circs) {
-        /* no need to blow away circuits/streams/etc. Also, don't mark this
-         * router as newly down, since maybe this was just an old circuit
-         * attempt that's finally timing out now. */
+        /* We only want to blame this router when a fresh healthy
+         * connection fails. So don't mark this router as newly failed,
+         * since maybe this was just an old circuit attempt that's
+         * finally timing out now. Also, there's no need to blow away
+         * circuits/streams/etc, since the failure of an unhealthy conn
+         * doesn't tell us much about whether a healthy conn would
+         * succeed. */
         already_marked = 1;
       }
       log_info(LD_OR,
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 836e7faef5..09f310a3df 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -606,8 +606,21 @@ connection_or_get_for_extend(const char *digest,
 #define TIME_BEFORE_OR_CONN_IS_TOO_OLD (60*60*24*7)
 
 /** Given the head of the linked list for all the or_connections with a given
- * identity, set elements of that list as is_bad_for_new_circs() as
- * appropriate.  Helper for connection_or_set_bad_connections().
+ * identity, set elements of that list as is_bad_for_new_circs as
+ * appropriate. Helper for connection_or_set_bad_connections().
+ *
+ * Specifically, we set the is_bad_for_new_circs flag on:
+ *    - all connections if <b>force</b> is true.
+ *    - all connections that are too old.
+ *    - all open non-canonical connections for which a canonical connection
+ *      exists to the same router.
+ *    - all open canonical connections for which a 'better' canonical
+ *      connection exists to the same router.
+ *    - all open non-canonical connections for which a 'better' non-canonical
+ *      connection exists to the same router at the same address.
+ *
+ * See connection_or_is_better() for our idea of what makes one OR connection
+ * better than another.
  */
 static void
 connection_or_group_set_badness(or_connection_t *head, int force)
@@ -721,18 +734,8 @@ connection_or_group_set_badness(or_connection_t *head, int force)
 
 /** Go through all the OR connections (or if <b>digest</b> is non-NULL, just
  * the OR connections with that digest), and set the is_bad_for_new_circs
- * flag on:
- *    - all connections if <b>force</b> is true.
- *    - all connections that are too old.
- *    - all open non-canonical connections for which a canonical connection
- *      exists to the same router.
- *    - all open canonical connections for which a 'better' canonical
- *      connection exists to the same router.
- *    - all open non-canonical connections for which a 'better' non-canonical
- *      connection exists to the same router at the same address.
- *
- * See connection_or_is_better() for our idea of what makes one OR connection
- * better than another.
+ * flag based on the rules in connection_or_group_set_badness() (or just
+ * always set it if <b>force</b> is true).
  */
 void
 connection_or_set_bad_connections(const char *digest, int force)