From 444e46d96d05c3e12eca02769512c74f2eb16b48 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 15:27:06 -0400
Subject: [PATCH 01/14] Remove the "fuzzy time" code

It was the start of a neat idea, but it only got used in 3 places,
none of which really needed it.
---
 changes/kill_ftime     |  7 ++++
 src/common/util.c      | 77 ------------------------------------------
 src/common/util.h      | 10 ------
 src/or/networkstatus.c |  6 +++-
 src/or/routerlist.c    |  4 +--
 5 files changed, 14 insertions(+), 90 deletions(-)
 create mode 100644 changes/kill_ftime

diff --git a/changes/kill_ftime b/changes/kill_ftime
new file mode 100644
index 0000000000..47f4769735
--- /dev/null
+++ b/changes/kill_ftime
@@ -0,0 +1,7 @@
+  o Code simplification and refactoring
+    - Remove the old 'fuzzy time' logic.  It was supposed to be used
+      for handling calculations where we have a known amount of clock
+      skew and an allowed amount of unknown skew.  But we only used it
+      in three places, and we never adjusted the known/unknown skew
+      values.  This is still something we might want to do someday,
+      but if we do, we'll want to do it differently.
diff --git a/src/common/util.c b/src/common/util.c
index abd87ea652..38c0ad05e6 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1510,83 +1510,6 @@ update_approx_time(time_t now)
 }
 #endif
 
-/* =====
- * Fuzzy time
- * XXXX022 Use this consistently or rip most of it out.
- * ===== */
-
-/* In a perfect world, everybody would run NTP, and NTP would be perfect, so
- * if we wanted to know "Is the current time before time X?" we could just say
- * "time(NULL) < X".
- *
- * But unfortunately, many users are running Tor in an imperfect world, on
- * even more imperfect computers.  Hence, we need to track time oddly.  We
- * model the user's computer as being "skewed" from accurate time by
- * -<b>ftime_skew</b> seconds, such that our best guess of the current time is
- * time(NULL)+ftime_skew.  We also assume that our measurements of time may
- * have up to <b>ftime_slop</b> seconds of inaccuracy; IOW, our window of
- * estimate for the current time is now + ftime_skew +/- ftime_slop.
- */
-/** Our current estimate of our skew, such that we think the current time is
- * closest to time(NULL)+ftime_skew. */
-static int ftime_skew = 0;
-/** Tolerance during time comparisons, in seconds. */
-static int ftime_slop = 60;
-/** Set the largest amount of sloppiness we'll allow in fuzzy time
- * comparisons. */
-void
-ftime_set_maximum_sloppiness(int seconds)
-{
-  tor_assert(seconds >= 0);
-  ftime_slop = seconds;
-}
-/** Set the amount by which we believe our system clock to differ from
- * real time. */
-void
-ftime_set_estimated_skew(int seconds)
-{
-  ftime_skew = seconds;
-}
-#if 0
-void
-ftime_get_window(time_t now, ftime_t *ft_out)
-{
-  ft_out->earliest = now + ftime_skew - ftime_slop;
-  ft_out->latest =  now + ftime_skew + ftime_slop;
-}
-#endif
-/** Return true iff we think that <b>now</b> might be after <b>when</b>. */
-int
-ftime_maybe_after(time_t now, time_t when)
-{
-  /* It may be after when iff the latest possible current time is after when */
-  return (now + ftime_skew + ftime_slop) >= when;
-}
-/** Return true iff we think that <b>now</b> might be before <b>when</b>. */
-int
-ftime_maybe_before(time_t now, time_t when)
-{
-  /* It may be before when iff the earliest possible current time is before */
-  return (now + ftime_skew - ftime_slop) < when;
-}
-/** Return true if we think that <b>now</b> is definitely after <b>when</b>. */
-int
-ftime_definitely_after(time_t now, time_t when)
-{
-  /* It is definitely after when if the earliest time it could be is still
-   * after when. */
-  return (now + ftime_skew - ftime_slop) >= when;
-}
-/** Return true if we think that <b>now</b> is definitely before <b>when</b>.
- */
-int
-ftime_definitely_before(time_t now, time_t when)
-{
-  /* It is definitely before when if the latest time it could be is still
-   * before when. */
-  return (now + ftime_skew + ftime_slop) < when;
-}
-
 /* =====
  * Rate limiting
  * ===== */
diff --git a/src/common/util.h b/src/common/util.h
index 3736237b32..6b54856743 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -247,16 +247,6 @@ time_t approx_time(void);
 void update_approx_time(time_t now);
 #endif
 
-/* Fuzzy time. */
-void ftime_set_maximum_sloppiness(int seconds);
-void ftime_set_estimated_skew(int seconds);
-/* typedef struct ftime_t { time_t earliest; time_t latest; } ftime_t; */
-/* void ftime_get_window(time_t now, ftime_t *ft_out); */
-int ftime_maybe_after(time_t now, time_t when);
-int ftime_maybe_before(time_t now, time_t when);
-int ftime_definitely_after(time_t now, time_t when);
-int ftime_definitely_before(time_t now, time_t when);
-
 /* Rate-limiter */
 
 /** A ratelim_t remembers how often an event is occurring, and how often
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 687ac03fa0..155dffe630 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1764,7 +1764,11 @@ networkstatus_set_current_consensus(const char *consensus,
     write_str_to_file(consensus_fname, consensus, 0);
   }
 
-  if (ftime_definitely_before(now, current_consensus->valid_after)) {
+/** If a consensus appears more than this many seconds before its declared
+ * valid-after time, declare that our clock is skewed. */
+#define EARLY_CONSENSUS_NOTICE_SKEW 60
+
+  if (now < current_consensus->valid_after - EARLY_CONSENSUS_NOTICE_SKEW) {
     char tbuf[ISO_TIME_LEN+1];
     char dbuf[64];
     long delta = now - current_consensus->valid_after;
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 4421d5cf81..2dca57899a 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -328,7 +328,7 @@ trusted_dirs_remove_old_certs(void)
         time_t cert_published;
         if (newest == cert)
           continue;
-        expired = ftime_definitely_after(now, cert->expires);
+        expired = now > cert->expires;
         cert_published = cert->cache_info.published_on;
         /* Store expired certs for 48 hours after a newer arrives;
          */
@@ -520,7 +520,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now)
       continue;
     cl = get_cert_list(ds->v3_identity_digest);
     SMARTLIST_FOREACH(cl->certs, authority_cert_t *, cert, {
-      if (!ftime_definitely_after(now, cert->expires)) {
+      if (now < cert->expires) {
         /* It's not expired, and we weren't looking for something to
          * verify a consensus with.  Call it done. */
         download_status_reset(&cl->dl_status);

From 88bb40d8f87077e2f954b6de779ac52c637c69ec Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 15:32:45 -0400
Subject: [PATCH 02/14] Clean up a comment-conversation about bad libevent
 version/method combos

---
 src/common/compat_libevent.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c
index 3efc56d8d8..3ad9be145d 100644
--- a/src/common/compat_libevent.c
+++ b/src/common/compat_libevent.c
@@ -330,17 +330,12 @@ tor_check_libevent_version(const char *m, int server,
 
   version = tor_get_libevent_version(&v);
 
-  /* XXX Would it be worthwhile disabling the methods that we know
-   * are buggy, rather than just warning about them and then proceeding
-   * to use them? If so, we should probably not wrap this whole thing
-   * in HAVE_EVENT_GET_VERSION and HAVE_EVENT_GET_METHOD. -RD */
-  /* XXXX The problem is that it's not trivial to get libevent to change it's
-   * method once it's initialized, and it's not trivial to tell what method it
-   * will use without initializing it.  I guess we could preemptively disable
-   * buggy libevent modes based on the version _before_ initializing it,
-   * though, but then there's no good way (afaict) to warn "I would have used
-   * kqueue, but instead I'm using select." -NM */
-  /* XXXX022 revist the above; it is fixable now. */
+  /* It would be better to disable known-buggy methods rather than warning
+   * about them.  But the problem is that with older versions of Libevent,
+   * it's not trivial to get them to change their methods once they're
+   * initialized... and with newer versions of Libevent, they aren't actually
+   * broken.  But we should revisit this if we ever find a post-1.4 version
+   * of Libevent where we need to disable a given method. */
   if (!strcmp(m, "kqueue")) {
     if (version < V_OLD(1,1,'b'))
       buggy = 1;

From 41380fa3b3bc3647699116866afda4286ba90f8b Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 15:41:19 -0400
Subject: [PATCH 03/14] Fixup tor_addr_to_sockaddr return convention

---
 src/common/address.c | 18 ++++++++++--------
 src/or/dns.c         |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/common/address.c b/src/common/address.c
index 0046d2da36..a4f8d3448f 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -50,11 +50,13 @@
 #include <assert.h>
 
 /** Convert the tor_addr_t in <b>a</b>, with port in <b>port</b>, into a
- * socklen object in *<b>sa_out</b> of object size <b>len</b>.  If not enough
- * room is free, or on error, return -1.  Else return the length of the
- * sockaddr. */
-/* XXXX021 This returns socklen_t.  socklen_t is sometimes unsigned.  This
- *   function claims to return -1 sometimes.  Problematic! */
+ * sockaddr object in *<b>sa_out</b> of object size <b>len</b>.  If not enough
+ * room is available in sa_out, or on error, return 0  On success, return
+ * the length of the sockaddr.
+ *
+ * Interface note: ordinarly, we return -1 for error.  We can't do that here,
+ * since socklen is unsigned on some platforms.
+ **/
 socklen_t
 tor_addr_to_sockaddr(const tor_addr_t *a,
                      uint16_t port,
@@ -65,7 +67,7 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
   if (family == AF_INET) {
     struct sockaddr_in *sin;
     if (len < (int)sizeof(struct sockaddr_in))
-      return -1;
+      return 0;
     sin = (struct sockaddr_in *)sa_out;
     memset(sin, 0, sizeof(struct sockaddr_in));
 #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
@@ -78,7 +80,7 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
   } else if (family == AF_INET6) {
     struct sockaddr_in6 *sin6;
     if (len < (int)sizeof(struct sockaddr_in6))
-      return -1;
+      return 0;
     sin6 = (struct sockaddr_in6 *)sa_out;
     memset(sin6, 0, sizeof(struct sockaddr_in6));
 #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_LEN
@@ -89,7 +91,7 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
     memcpy(&sin6->sin6_addr, tor_addr_to_in6(a), sizeof(struct in6_addr));
     return sizeof(struct sockaddr_in6);
   } else {
-    return -1;
+    return 0;
   }
 }
 
diff --git a/src/or/dns.c b/src/or/dns.c
index dcccd1390d..61c8f32c98 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -1206,7 +1206,7 @@ configure_nameservers(int force)
       struct sockaddr_storage ss;
       socklen = tor_addr_to_sockaddr(&addr, 0,
                                      (struct sockaddr *)&ss, sizeof(ss));
-      if (socklen < 0) {
+      if (socklen <= 0) {
         log_warn(LD_BUG, "Couldn't convert outbound bind address to sockaddr."
                  " Ignoring.");
       } else {

From c4bd067359b07f6a57adf9d444d2a46e514043ff Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 15:44:02 -0400
Subject: [PATCH 04/14] Comment out ancient asserts for bug 930; resolve an
 xxx021

---
 src/common/memarea.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/common/memarea.c b/src/common/memarea.c
index 6893639a6e..a6b8c4ee9c 100644
--- a/src/common/memarea.c
+++ b/src/common/memarea.c
@@ -59,7 +59,9 @@ realign_pointer(void *ptr)
 {
   uintptr_t x = (uintptr_t)ptr;
   x = (x+MEMAREA_ALIGN_MASK) & ~MEMAREA_ALIGN_MASK;
-  tor_assert(((void*)x) >= ptr); // XXXX021 remove this once bug 930 is solved
+  /* Reinstate this if bug 930 ever reappears
+  tor_assert(((void*)x) >= ptr);
+  */
   return (void*)x;
 }
 
@@ -241,9 +243,10 @@ memarea_alloc(memarea_t *area, size_t sz)
   }
   result = chunk->next_mem;
   chunk->next_mem = chunk->next_mem + sz;
-  // XXXX021 remove these once bug 930 is solved.
+  /* Reinstate these if bug 930 ever comes back
   tor_assert(chunk->next_mem >= chunk->u.mem);
   tor_assert(chunk->next_mem <= chunk->u.mem+chunk->mem_size);
+  */
   chunk->next_mem = realign_pointer(chunk->next_mem);
   return result;
 }

From 05887f10ffe14498e96c34d2faa535187719689f Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 16:01:16 -0400
Subject: [PATCH 05/14] Triage the XXX022 and XXX021 comments remaining in the
 code

Remove some, postpone others, leave some alone.  Now the only
remaining XXX022s are ones that seem important to fix or investigate.
---
 src/common/address.c     |  2 +-
 src/or/buffers.c         |  6 +++---
 src/or/circuitbuild.c    |  9 +++++----
 src/or/circuituse.c      |  5 +++--
 src/or/config.c          |  6 +++---
 src/or/connection.c      |  6 +++---
 src/or/connection_edge.c |  6 +++---
 src/or/directory.c       |  6 +++---
 src/or/dirserv.c         | 11 +++++++----
 src/or/dnsserv.c         |  2 +-
 src/or/eventdns.c        |  2 +-
 src/or/geoip.c           |  5 +++--
 src/or/networkstatus.c   |  2 +-
 src/or/or.h              |  2 +-
 src/or/reasons.c         |  2 +-
 src/or/rendclient.c      |  4 ++--
 src/or/rendcommon.c      |  2 +-
 src/or/rephist.c         |  2 +-
 src/or/routerlist.c      | 14 +++++++-------
 src/or/routerparse.c     |  2 +-
 20 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/src/common/address.c b/src/common/address.c
index a4f8d3448f..a089260de8 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -1087,7 +1087,7 @@ get_interface_address6(int severity, sa_family_t family, tor_addr_t *addr)
 
 /* ======
  * IPv4 helpers
- * XXXX022 IPv6 deprecate some of these.
+ * XXXX023 IPv6 deprecate some of these.
  */
 
 /** Return true iff <b>ip</b> (in host order) is an IP reserved to localhost,
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 9a30a7b591..db926955b4 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -666,12 +666,12 @@ read_to_chunk_tls(buf_t *buf, chunk_t *chunk, tor_tls_t *tls,
  * (because of EOF), set *<b>reached_eof</b> to 1 and return 0. Return -1 on
  * error; else return the number of bytes read.
  */
-/* XXXX021 indicate "read blocked" somehow? */
+/* XXXX023 indicate "read blocked" somehow? */
 int
 read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof,
             int *socket_error)
 {
-  /* XXXX021 It's stupid to overload the return values for these functions:
+  /* XXXX023 It's stupid to overload the return values for these functions:
    * "error status" and "number of bytes read" are not mutually exclusive.
    */
   int r = 0;
@@ -856,7 +856,7 @@ flush_chunk_tls(tor_tls_t *tls, buf_t *buf, chunk_t *chunk,
 int
 flush_buf(int s, buf_t *buf, size_t sz, size_t *buf_flushlen)
 {
-  /* XXXX021 It's stupid to overload the return values for these functions:
+  /* XXXX023 It's stupid to overload the return values for these functions:
    * "error status" and "number of bytes flushed" are not mutually exclusive.
    */
   int r;
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 83ac7a5a19..992b2a55cc 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -42,11 +42,12 @@
 
 /********* START VARIABLES **********/
 /** Global list of circuit build times */
-// FIXME: Add this as a member for entry_guard_t instead of global?
+// XXXX023: Add this as a member for entry_guard_t instead of global?
 // Then we could do per-guard statistics, as guards are likely to
 // vary in their own latency. The downside of this is that guards
 // can change frequently, so we'd be building a lot more circuits
 // most likely.
+/* XXXX023 Make this static; add accessor functions. */
 circuit_build_times_t circ_times;
 
 /** A global list of all circuits at this hop. */
@@ -3814,7 +3815,7 @@ entry_guards_compute_status(or_options_t *options, time_t now)
  * If <b>mark_relay_status</b>, also call router_set_status() on this
  * relay.
  *
- * XXX022 change succeeded and mark_relay_status into 'int flags'.
+ * XXX023 change succeeded and mark_relay_status into 'int flags'.
  */
 int
 entry_guard_register_connect_status(const char *digest, int succeeded,
@@ -3972,7 +3973,7 @@ entry_guards_prepend_from_config(or_options_t *options)
 
   /* Split entry guards into those on the list and those not. */
 
-  /* XXXX022 Now that we allow countries and IP ranges in EntryNodes, this is
+  /* XXXX023 Now that we allow countries and IP ranges in EntryNodes, this is
    *  potentially an enormous list. For now, we disable such values for
    *  EntryNodes in options_validate(); really, this wants a better solution.
    *  Perhaps we should do this calculation once whenever the list of routers
@@ -4304,7 +4305,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
     }
     entry_guards = new_entry_guards;
     entry_guards_dirty = 0;
-    /* XXX022 hand new_entry_guards to this func, and move it up a
+    /* XXX023 hand new_entry_guards to this func, and move it up a
      * few lines, so we don't have to re-dirty it */
     if (remove_obsolete_entry_guards(now))
       entry_guards_dirty = 1;
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 8d9d115863..607beaa06a 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1266,7 +1266,8 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn,
         return -1;
       }
     } else {
-      /* XXXX022 Duplicates checks in connection_ap_handshake_attach_circuit */
+      /* XXXX023 Duplicates checks in connection_ap_handshake_attach_circuit:
+       * refactor into a single function? */
       routerinfo_t *router = router_get_by_nickname(conn->chosen_exit_name, 1);
       int opt = conn->chosen_exit_optional;
       if (router && !connection_ap_can_use_exit(conn, router, 0)) {
@@ -1605,7 +1606,7 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn)
     /* find the circuit that we should use, if there is one. */
     retval = circuit_get_open_circ_or_launch(
         conn, CIRCUIT_PURPOSE_C_GENERAL, &circ);
-    if (retval < 1) // XXX021 if we totally fail, this still returns 0 -RD
+    if (retval < 1) // XXX022 if we totally fail, this still returns 0 -RD
       return retval;
 
     log_debug(LD_APP|LD_CIRC,
diff --git a/src/or/config.c b/src/or/config.c
index 5719a64088..5000f5d60e 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1338,7 +1338,7 @@ options_act(or_options_t *old_options)
        || !geoip_is_loaded())) {
     /* XXXX Don't use this "<default>" junk; make our filename options
      * understand prefixes somehow. -NM */
-    /* XXXX021 Reload GeoIPFile on SIGHUP. -NM */
+    /* XXXX023 Reload GeoIPFile on SIGHUP. -NM */
     char *actual_fname = tor_strdup(options->GeoIPFile);
 #ifdef WIN32
     if (!strcmp(actual_fname, "<default>")) {
@@ -2483,7 +2483,7 @@ is_local_addr(const tor_addr_t *addr)
   if (get_options()->EnforceDistinctSubnets == 0)
     return 0;
   if (tor_addr_family(addr) == AF_INET) {
-    /*XXXX022 IP6 what corresponds to an /24? */
+    /*XXXX023 IP6 what corresponds to an /24? */
     uint32_t ip = tor_addr_to_ipv4h(addr);
 
     /* It's possible that this next check will hit before the first time
@@ -3652,7 +3652,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
              "ignore you.");
   }
 
-  /*XXXX022 checking for defaults manually like this is a bit fragile.*/
+  /*XXXX023 checking for defaults manually like this is a bit fragile.*/
 
   /* Keep changes to hard-coded values synchronous to man page and default
    * values table. */
diff --git a/src/or/connection.c b/src/or/connection.c
index c65c91b73b..7fa6cd9c17 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2575,13 +2575,13 @@ connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error)
   }
 
   if (n_read > 0) { /* change *max_to_read */
-    /*XXXX021 check for overflow*/
+    /*XXXX022 check for overflow*/
     *max_to_read = (int)(at_most - n_read);
   }
 
   if (conn->type == CONN_TYPE_AP) {
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    /*XXXX021 check for overflow*/
+    /*XXXX022 check for overflow*/
     edge_conn->n_read += (int)n_read;
   }
 
@@ -2783,7 +2783,7 @@ connection_handle_write_impl(connection_t *conn, int force)
 
   if (conn->type == CONN_TYPE_AP) {
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    /*XXXX021 check for overflow.*/
+    /*XXXX022 check for overflow.*/
     edge_conn->n_written += (int)n_written;
   }
 
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 85a52257a8..82b71395b5 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -557,7 +557,7 @@ connection_ap_attach_pending(void)
 
 /** Tell any AP streams that are waiting for a one-hop tunnel to
  * <b>failed_digest</b> that they are going to fail. */
-/* XXX022 We should get rid of this function, and instead attach
+/* XXX023 We should get rid of this function, and instead attach
  * one-hop streams to circ->p_streams so they get marked in
  * circuit_mark_for_close like normal p_streams. */
 void
@@ -2382,7 +2382,7 @@ tell_controller_about_resolved_result(edge_connection_t *conn,
  * certain errors or for values that didn't come via DNS.  <b>expires</b> is
  * a time when the answer expires, or -1 or TIME_MAX if there's a good TTL.
  **/
-/* XXXX022 the use of the ttl and expires fields is nutty.  Let's make this
+/* XXXX023 the use of the ttl and expires fields is nutty.  Let's make this
  * interface and those that use it less ugly. */
 void
 connection_ap_handshake_socks_resolved(edge_connection_t *conn,
@@ -2824,7 +2824,7 @@ connection_exit_connect(edge_connection_t *edge_conn)
   log_debug(LD_EXIT,"about to try connecting");
   switch (connection_connect(conn, conn->address, addr, port, &socket_error)) {
     case -1:
-      /* XXX021 use socket_error below rather than trying to piece things
+      /* XXX022 use socket_error below rather than trying to piece things
        * together from the current errno, which may have been clobbered. */
       connection_edge_end_errno(edge_conn);
       circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn);
diff --git a/src/or/directory.c b/src/or/directory.c
index 00de1f2f80..1347c8b4e9 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -372,7 +372,7 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
   if (!get_via_tor) {
     if (options->UseBridges && type != BRIDGE_AUTHORITY) {
       /* want to ask a running bridge for which we have a descriptor. */
-      /* XXX022 we assume that all of our bridges can answer any
+      /* XXX023 we assume that all of our bridges can answer any
        * possible directory question. This won't be true forever. -RD */
       /* It certainly is not true with conditional consensus downloading,
        * so, for now, never assume the server supports that. */
@@ -1876,7 +1876,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
                        ds->nickname);
               /* XXXX use this information; be sure to upload next one
                * sooner. -NM */
-              /* XXXX021 On further thought, the task above implies that we're
+              /* XXXX023 On further thought, the task above implies that we're
                * basing our regenerate-descriptor time on when we uploaded the
                * last descriptor, not on the published time of the last
                * descriptor.  If those are different, that's a bad thing to
@@ -2706,7 +2706,7 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers,
     ssize_t estimated_len = 0;
     smartlist_t *items = smartlist_create();
     smartlist_t *dir_items = smartlist_create();
-    int lifetime = 60; /* XXXX022 should actually use vote intervals. */
+    int lifetime = 60; /* XXXX023 should actually use vote intervals. */
     url += strlen("/tor/status-vote/");
     current = !strcmpstart(url, "current/");
     url = strchr(url, '/');
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 18abd1865f..f65f25811b 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -946,7 +946,7 @@ running_long_enough_to_decide_unreachable(void)
 void
 dirserv_set_router_is_running(routerinfo_t *router, time_t now)
 {
-  /*XXXX022 This function is a mess.  Separate out the part that calculates
+  /*XXXX023 This function is a mess.  Separate out the part that calculates
     whether it's reachable and the part that tells rephist that the router was
     unreachable.
    */
@@ -1754,9 +1754,12 @@ dirserv_thinks_router_is_unreliable(time_t now,
 {
   if (need_uptime) {
     if (!enough_mtbf_info) {
-      /* XXX022 Once most authorities are on v3, we should change the rule from
+      /* XXX023 Once most authorities are on v3, we should change the rule from
        * "use uptime if we don't have mtbf data" to "don't advertise Stable on
-       * v3 if we don't have enough mtbf data." */
+       * v3 if we don't have enough mtbf data."  Or maybe not, since if we ever
+       * hit a point where we need to reset a lot of authorities at once,
+       * none of them would be in a position to declare Stable.
+       */
       long uptime = real_uptime(router, now);
       if ((unsigned)uptime < stable_uptime &&
           (unsigned)uptime < UPTIME_TO_GUARANTEE_STABLE)
@@ -3260,7 +3263,7 @@ lookup_cached_dir_by_fp(const char *fp)
     d = strmap_get(cached_consensuses, "ns");
   else if (memchr(fp, '\0', DIGEST_LEN) && cached_consensuses &&
            (d = strmap_get(cached_consensuses, fp))) {
-    /* this here interface is a nasty hack XXXX022 */;
+    /* this here interface is a nasty hack XXXX023 */;
   } else if (router_digest_is_me(fp) && the_v2_networkstatus)
     d = the_v2_networkstatus;
   else if (cached_v2_networkstatus)
diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c
index 8222c8b45d..f7a8d35f78 100644
--- a/src/or/dnsserv.c
+++ b/src/or/dnsserv.c
@@ -19,7 +19,7 @@
 #ifdef HAVE_EVENT2_DNS_H
 #include <event2/dns.h>
 #include <event2/dns_compat.h>
-/* XXXX022 this implies we want an improved evdns  */
+/* XXXX023 this implies we want an improved evdns  */
 #include <event2/dns_struct.h>
 #else
 #include "eventdns.h"
diff --git a/src/or/eventdns.c b/src/or/eventdns.c
index 75a25bd088..8b679f8985 100644
--- a/src/or/eventdns.c
+++ b/src/or/eventdns.c
@@ -1999,7 +1999,7 @@ evdns_request_timeout_callback(int fd, short events, void *arg) {
 		/* retransmit it */
 		/* Stop waiting for the timeout.  No need to do this in
 		 * request_finished; that one already deletes the timeout event.
-		 * XXXX021 port this change to libevent. */
+		 * XXXX023 port this change to libevent. */
 		del_timeout_event(req);
 		evdns_request_transmit(req);
 	}
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 654241c1c0..e41d5e36b8 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -601,8 +601,9 @@ _dirreq_map_put(dirreq_map_entry_t *entry, dirreq_type_t type,
   tor_assert(entry->type == type);
   tor_assert(entry->dirreq_id == dirreq_id);
 
-  /* XXXX022 once we're sure the bug case never happens, we can switch
-   * to HT_INSERT */
+  /* XXXX we could switch this to HT_INSERT some time, since it seems that
+   * this bug doesn't happen. But since this function doesn't seem to be
+   * critical-path, it's sane to leave it alone. */
   old_ent = HT_REPLACE(dirreqmap, &dirreq_map, entry);
   if (old_ent && old_ent != entry) {
     log_warn(LD_BUG, "Error when putting directory request into local "
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 155dffe630..18cb4779e0 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1606,7 +1606,7 @@ networkstatus_set_current_consensus(const char *consensus,
 
   if (from_cache && !accept_obsolete &&
       c->valid_until < now-OLD_ROUTER_DESC_MAX_AGE) {
-    /* XXX022 when we try to make fallbackconsensus work again, we should
+    /* XXXX If we try to make fallbackconsensus work again, we should
      * consider taking this out. Until then, believing obsolete consensuses
      * is causing more harm than good. See also bug 887. */
     log_info(LD_DIR, "Loaded an expired consensus. Discarding.");
diff --git a/src/or/or.h b/src/or/or.h
index e3e01cff55..124892ce7b 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1006,7 +1006,7 @@ typedef struct connection_t {
   /** Unique identifier for this connection on this Tor instance. */
   uint64_t global_identifier;
 
-  /* XXXX022 move this field, and all the listener-only fields (just
+  /* XXXX023 move this field, and all the listener-only fields (just
      socket_family, I think), into a new listener_connection_t subtype. */
   /** If the connection is a CONN_TYPE_AP_DNS_LISTENER, this field points
    * to the evdns_server_port is uses to listen to and answer connections. */
diff --git a/src/or/reasons.c b/src/or/reasons.c
index 304ea9fcfa..27c947edff 100644
--- a/src/or/reasons.c
+++ b/src/or/reasons.c
@@ -174,7 +174,7 @@ errno_to_stream_end_reason(int e)
     S_CASE(ENETUNREACH):
       return END_STREAM_REASON_INTERNAL;
     S_CASE(EHOSTUNREACH):
-      /* XXXX022
+      /* XXXX023
        * The correct behavior is END_STREAM_REASON_NOROUTE, but older
        * clients don't recognize it.  So we're going to continue sending
        * "MISC" until 0.2.1.27 or later is "well established".
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 01b2b62d3a..8ac909fc80 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -599,7 +599,7 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request,
   log_info(LD_REND,"Got rendezvous ack. This circuit is now ready for "
            "rendezvous.");
   circ->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY;
-  /* XXXX022 This is a pretty brute-force approach. It'd be better to
+  /* XXXX023 This is a pretty brute-force approach. It'd be better to
    * attach only the connections that are waiting on this circuit, rather
    * than trying to attach them all. See comments bug 743. */
   /* If we already have the introduction circuit built, make sure we send
@@ -669,7 +669,7 @@ rend_client_receive_rendezvous(origin_circuit_t *circ, const uint8_t *request,
 
   onion_append_to_cpath(&circ->cpath, hop);
   circ->build_state->pending_final_cpath = NULL; /* prevent double-free */
-  /* XXXX022 This is a pretty brute-force approach. It'd be better to
+  /* XXXX023 This is a pretty brute-force approach. It'd be better to
    * attach only the connections that are waiting on this circuit, rather
    * than trying to attach them all. See comments bug 743. */
   connection_ap_attach_pending();
diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c
index 290e8f8389..f4c8888c04 100644
--- a/src/or/rendcommon.c
+++ b/src/or/rendcommon.c
@@ -932,7 +932,7 @@ rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e)
   if (!*e)
     return 0;
   tor_assert((*e)->parsed && (*e)->parsed->intro_nodes);
-  /* XXX022 hack for now, to return "not found" if there are no intro
+  /* XXX023 hack for now, to return "not found" if there are no intro
    * points remaining. See bug 997. */
   if (smartlist_len((*e)->parsed->intro_nodes) == 0)
     return 0;
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 58e8ff0a05..1bb8c6ee68 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -587,7 +587,7 @@ rep_hist_get_weighted_time_known(const char *id, time_t when)
 int
 rep_hist_have_measured_enough_stability(void)
 {
-  /* XXXX021 This doesn't do so well when we change our opinion
+  /* XXXX022 This doesn't do so well when we change our opinion
    * as to whether we're tracking router stability. */
   return started_tracking_stability < time(NULL) - 4*60*60;
 }
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 2dca57899a..4deff53a3c 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1770,7 +1770,7 @@ smartlist_choose_by_bandwidth_weights(smartlist_t *sl,
       sl_last_weighted_bw_of_me = weight*this_bw;
   }
 
-  /* XXXX022 this is a kludge to expose these values. */
+  /* XXXX023 this is a kludge to expose these values. */
   sl_last_total_weighted_bw = weighted_bw;
 
   log_debug(LD_CIRC, "Choosing node for rule %s based on weights "
@@ -1893,7 +1893,7 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule,
       if (status->has_bandwidth) {
         this_bw = kb_to_bytes(status->bandwidth);
       } else { /* guess */
-        /* XXX022 once consensuses always list bandwidths, we can take
+        /* XXX023 once consensuses always list bandwidths, we can take
          * this guessing business out. -RD */
         is_known = 0;
         flags = status->is_fast ? 1 : 0;
@@ -1912,7 +1912,7 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule,
       if (rs && rs->has_bandwidth) {
         this_bw = kb_to_bytes(rs->bandwidth);
       } else if (rs) { /* guess; don't trust the descriptor */
-        /* XXX022 once consensuses always list bandwidths, we can take
+        /* XXX023 once consensuses always list bandwidths, we can take
          * this guessing business out. -RD */
         is_known = 0;
         flags = router->is_fast ? 1 : 0;
@@ -2028,7 +2028,7 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule,
     }
   }
 
-  /* XXXX022 this is a kludge to expose these values. */
+  /* XXXX023 this is a kludge to expose these values. */
   sl_last_total_weighted_bw = total_bw;
 
   log_debug(LD_CIRC, "Total weighted bw = "U64_FORMAT
@@ -3395,7 +3395,7 @@ router_add_extrainfo_to_routerlist(extrainfo_t *ei, const char **msg,
   int inserted;
   (void)from_fetch;
   if (msg) *msg = NULL;
-  /*XXXX022 Do something with msg */
+  /*XXXX023 Do something with msg */
 
   inserted = extrainfo_insert(router_get_routerlist(), ei);
 
@@ -4591,7 +4591,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote,
 
 /** How often should we launch a server/authority request to be sure of getting
  * a guess for our IP? */
-/*XXXX021 this info should come from netinfo cells or something, or we should
+/*XXXX023 this info should come from netinfo cells or something, or we should
  * do this only when we aren't seeing incoming data. see bug 652. */
 #define DUMMY_DOWNLOAD_INTERVAL (20*60)
 
@@ -4609,7 +4609,7 @@ update_router_descriptor_downloads(time_t now)
   update_consensus_router_descriptor_downloads(now, 0,
     networkstatus_get_reasonably_live_consensus(now));
 
-  /* XXXX021 we could be smarter here; see notes on bug 652. */
+  /* XXXX023 we could be smarter here; see notes on bug 652. */
   /* If we're a server that doesn't have a configured address, we rely on
    * directory fetches to learn when our address changes.  So if we haven't
    * tried to get any routerdescs in a long time, try a dummy fetch now. */
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index e0605dcd4d..95e174e550 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1818,7 +1818,7 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
     struct in_addr in;
     char *address = NULL;
     tor_assert(tok->n_args);
-    /* XXX021 use tor_addr_port_parse() below instead. -RD */
+    /* XXX023 use tor_addr_port_parse() below instead. -RD */
     if (parse_addr_port(LOG_WARN, tok->args[0], &address, NULL,
                         &cert->dir_port)<0 ||
         tor_inet_aton(address, &in) == 0) {

From 6a5b94de6c51478ca638ec03c40a6d3c27f2d674 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 16:14:42 -0400
Subject: [PATCH 06/14] Look at the right errno when sending reason for
 connect() failure

In afe414 (tor-0.1.0.1-rc~173), when we moved to
connection_edge_end_errno(), we used it in handling errors from
connection_connect().  That's not so good, since by the time
connection_connect() returns, the socket is no longer set, and we're
supposed to be looking at the socket_errno return value from
connection_connect() instead.  So do what we should've done, and
look at the socket_errno value that we get from connection_connect().
---
 changes/connect_err_reporting | 6 ++++++
 src/or/connection_edge.c      | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)
 create mode 100644 changes/connect_err_reporting

diff --git a/changes/connect_err_reporting b/changes/connect_err_reporting
new file mode 100644
index 0000000000..61a46b6580
--- /dev/null
+++ b/changes/connect_err_reporting
@@ -0,0 +1,6 @@
+  o Minor bugfixes:
+    - Be more careful about reporting the correct error from a failed
+      connect() operation.  Under some circumstances, it was possible to
+      look at an incorrect value for errno when sending the end reason.
+      Bugfix on Tor-0.1.0.1-rc.
+
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 82b71395b5..c75c06bc60 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2823,13 +2823,13 @@ connection_exit_connect(edge_connection_t *edge_conn)
 
   log_debug(LD_EXIT,"about to try connecting");
   switch (connection_connect(conn, conn->address, addr, port, &socket_error)) {
-    case -1:
-      /* XXX022 use socket_error below rather than trying to piece things
-       * together from the current errno, which may have been clobbered. */
-      connection_edge_end_errno(edge_conn);
+    case -1: {
+      int reason = errno_to_stream_end_reason(socket_error);
+      connection_edge_end(edge_conn, reason);
       circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn);
       connection_free(conn);
       return;
+    }
     case 0:
       conn->state = EXIT_CONN_STATE_CONNECTING;
 

From dddd333a80ee2e9bb731cb3c127ace3741d49673 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 16:45:25 -0400
Subject: [PATCH 07/14] Fix some 'impossible' overflow bugs in byte counting

The first was genuinely impossible, I think: it could only happen
when the amount we read differed from the amount we wanted to read
by more than INT_MAX.

The second is just very unlikely: it would give incorrect results to
the controller if you somehow wrote or read more than 4GB on one
edge conn in one second.  That one is a bugfix on 0.1.2.8-beta.
---
 changes/count_overflow |  5 +++++
 src/or/connection.c    | 36 ++++++++++++++++++++++--------------
 2 files changed, 27 insertions(+), 14 deletions(-)
 create mode 100644 changes/count_overflow

diff --git a/changes/count_overflow b/changes/count_overflow
new file mode 100644
index 0000000000..f302ff2d71
--- /dev/null
+++ b/changes/count_overflow
@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Correctly handle an "impossible" overflow cases in connection
+      byte counting, where we write or read more than 4GB on an edge
+      connection in single second.  Bugfix on 0.1.2.8-beta.
+
diff --git a/src/or/connection.c b/src/or/connection.c
index 7fa6cd9c17..084237dea1 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -51,7 +51,7 @@ static int connection_finished_flushing(connection_t *conn);
 static int connection_flushed_some(connection_t *conn);
 static int connection_finished_connecting(connection_t *conn);
 static int connection_reached_eof(connection_t *conn);
-static int connection_read_to_buf(connection_t *conn, int *max_to_read,
+static int connection_read_to_buf(connection_t *conn, ssize_t *max_to_read,
                                   int *socket_error);
 static int connection_process_inbuf(connection_t *conn, int package_partial);
 static void client_check_address_changed(int sock);
@@ -2338,7 +2338,7 @@ connection_bucket_should_increase(int bucket, or_connection_t *conn)
 static int
 connection_handle_read_impl(connection_t *conn)
 {
-  int max_to_read=-1, try_to_read;
+  ssize_t max_to_read=-1, try_to_read;
   size_t before, n_read = 0;
   int socket_error = 0;
 
@@ -2456,7 +2456,8 @@ connection_handle_read(connection_t *conn)
  * Return -1 if we want to break conn, else return 0.
  */
 static int
-connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error)
+connection_read_to_buf(connection_t *conn, ssize_t *max_to_read,
+                       int *socket_error)
 {
   int result;
   ssize_t at_most = *max_to_read;
@@ -2574,15 +2575,19 @@ connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error)
     n_read = (size_t) result;
   }
 
-  if (n_read > 0) { /* change *max_to_read */
-    /*XXXX022 check for overflow*/
-    *max_to_read = (int)(at_most - n_read);
-  }
+  if (n_read > 0) {
+    /* change *max_to_read */
+    *max_to_read = at_most - n_read;
 
-  if (conn->type == CONN_TYPE_AP) {
-    edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    /*XXXX022 check for overflow*/
-    edge_conn->n_read += (int)n_read;
+    /* Update edge_conn->n_read */
+    if (conn->type == CONN_TYPE_AP) {
+      edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
+      /* Check for overflow: */
+      if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_read > n_read))
+        edge_conn->n_read += (int)n_read;
+      else
+        edge_conn->n_read = UINT32_MAX;
+    }
   }
 
   connection_buckets_decrement(conn, approx_time(), n_read, n_written);
@@ -2781,10 +2786,13 @@ connection_handle_write_impl(connection_t *conn, int force)
     n_written = (size_t) result;
   }
 
-  if (conn->type == CONN_TYPE_AP) {
+  if (n_written && conn->type == CONN_TYPE_AP) {
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    /*XXXX022 check for overflow.*/
-    edge_conn->n_written += (int)n_written;
+    /* Check for overflow: */
+    if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written))
+      edge_conn->n_written += n_written;
+    else
+      edge_conn->n_written = UINT32_MAX;
   }
 
   connection_buckets_decrement(conn, approx_time(), n_read, n_written);

From 550749555cd57a1f82fe6c08e866ae14456ed439 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 17:11:04 -0400
Subject: [PATCH 08/14] Remove workaround code for bug539

We fixed bug 539 (where directories would say "503" but send data
anyway) back in 0.2.0.16-alpha/0.1.2.19.  Because most directory
versions were affected, we added workaround to make sure that we
examined the contents of 503-replies to make sure there wasn't any
data for them to find.  But now that such routers are nonexistent,
we can remove this code.  (Even if somebody fired up an 0.1.2.19
directory cache today, it would still be fine to ignore data in its
erroneous 503 replies.)
---
 changes/bug539_removal |  6 ++++++
 src/or/directory.c     | 31 ++++++++++++-------------------
 2 files changed, 18 insertions(+), 19 deletions(-)
 create mode 100644 changes/bug539_removal

diff --git a/changes/bug539_removal b/changes/bug539_removal
new file mode 100644
index 0000000000..dbff43de18
--- /dev/null
+++ b/changes/bug539_removal
@@ -0,0 +1,6 @@
+  o Removed code
+    - Removed workaround code to handle directory responses from
+      servers that had bug 539 (they would send HTTP status 503
+      responses _and_ send a body too).  Since only server versions
+      before 0.2.0.16-alpha/0.1.2.19 were affected, there is no longer
+      reason to keep the workaround in place.
diff --git a/src/or/directory.c b/src/or/directory.c
index 1347c8b4e9..8f33a608d4 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1539,26 +1539,19 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
   (void) skewed; /* skewed isn't used yet. */
 
   if (status_code == 503) {
-    if (body_len < 16) {
-      routerstatus_t *rs;
-      trusted_dir_server_t *ds;
-      log_info(LD_DIR,"Received http status code %d (%s) from server "
-               "'%s:%d'. I'll try again soon.",
-               status_code, escaped(reason), conn->_base.address,
-               conn->_base.port);
-      if ((rs = router_get_consensus_status_by_id(conn->identity_digest)))
-        rs->last_dir_503_at = now;
-      if ((ds = router_get_trusteddirserver_by_digest(conn->identity_digest)))
-        ds->fake_status.last_dir_503_at = now;
+    routerstatus_t *rs;
+    trusted_dir_server_t *ds;
+    log_info(LD_DIR,"Received http status code %d (%s) from server "
+             "'%s:%d'. I'll try again soon.",
+             status_code, escaped(reason), conn->_base.address,
+             conn->_base.port);
+    if ((rs = router_get_consensus_status_by_id(conn->identity_digest)))
+      rs->last_dir_503_at = now;
+    if ((ds = router_get_trusteddirserver_by_digest(conn->identity_digest)))
+      ds->fake_status.last_dir_503_at = now;
 
-      tor_free(body); tor_free(headers); tor_free(reason);
-      return -1;
-    }
-    /* XXXX022 Remove this once every server with bug 539 is obsolete. */
-    log_info(LD_DIR, "Server at '%s:%d' sent us a 503 response, but included "
-             "a body anyway.  We'll pretend it gave us a 200.",
-             conn->_base.address, conn->_base.port);
-    status_code = 200;
+    tor_free(body); tor_free(headers); tor_free(reason);
+    return -1;
   }
 
   plausible = body_is_plausible(body, body_len, conn->_base.purpose);

From d38030381b04263f76ce7f8ed2d6ceefc4ce363f Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 17:21:16 -0400
Subject: [PATCH 09/14] Clarify some documentation and comments wrt resetting
 OR token buckets

---
 src/or/connection_or.c | 6 +++++-
 src/or/networkstatus.c | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index ff863fd550..4b932ec51e 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -353,6 +353,9 @@ connection_or_digest_is_known_relay(const char *id_digest)
  * per-conn limits that are big enough they'll never matter. But if it's
  * not a known relay, first check if we set PerConnBwRate/Burst, then
  * check if the consensus sets them, else default to 'big enough'.
+ *
+ * If <b>reset</b> is true, set the bucket to be full.  Otherwise, just
+ * clip the bucket if it happens to be <em>too</em> full.
  */
 static void
 connection_or_update_token_buckets_helper(or_connection_t *conn, int reset,
@@ -392,7 +395,8 @@ connection_or_update_token_buckets_helper(or_connection_t *conn, int reset,
 }
 
 /** Either our set of relays or our per-conn rate limits have changed.
- * Go through all the OR connections and update their token buckets. */
+ * Go through all the OR connections and update their token buckets to make
+ * sure they don't exceed their maximum values. */
 void
 connection_or_update_token_buckets(smartlist_t *conns, or_options_t *options)
 {
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 18cb4779e0..4f6fe15409 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1747,7 +1747,8 @@ networkstatus_set_current_consensus(const char *consensus,
     routerstatus_list_update_named_server_map();
     cell_ewma_set_scale_factor(options, current_consensus);
 
-    /* XXX022 where is the right place to put this call? */
+    /* XXXX023 this call might be unnecessary here: can changing the
+     * current consensus really alter our view of any OR's rate limits? */
     connection_or_update_token_buckets(get_connection_array(), options);
 
     circuit_build_times_new_consensus_params(&circ_times, current_consensus);

From f3b89c114112987b74bb0632cfe47a3a17859adb Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 17:49:44 -0400
Subject: [PATCH 10/14] Add XXX023s for our timestamp_dirty abuse.

---
 src/or/circuitlist.c     |  3 +++
 src/or/connection_edge.c |  2 ++
 src/or/or.h              | 10 ++++++++--
 src/or/relay.c           |  2 ++
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index b4f5f45615..521c21e179 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1023,6 +1023,7 @@ circuit_mark_all_unused_circs(void)
  * This is useful for letting the user change pseudonyms, so new
  * streams will not be linkable to old streams.
  */
+/* XXX023 this is a bad name for what this function does */
 void
 circuit_expire_all_dirty_circs(void)
 {
@@ -1033,6 +1034,8 @@ circuit_expire_all_dirty_circs(void)
     if (CIRCUIT_IS_ORIGIN(circ) &&
         !circ->marked_for_close &&
         circ->timestamp_dirty)
+      /* XXXX023 This is a screwed-up way to say "This is too dirty
+       * for new circuits. */
       circ->timestamp_dirty -= options->MaxCircuitDirtiness;
   }
 }
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index c75c06bc60..af0cfbe14c 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -517,6 +517,7 @@ connection_ap_expire_beginning(void)
     /* kludge to make us not try this circuit again, yet to allow
      * current streams on it to survive if they can: make it
      * unattractive to use for new streams */
+    /* XXXX023 this is a kludgy way to do this. */
     tor_assert(circ->timestamp_dirty);
     circ->timestamp_dirty -= options->MaxCircuitDirtiness;
     /* give our stream another 'cutoff' seconds to try */
@@ -2164,6 +2165,7 @@ connection_ap_handshake_send_begin(edge_connection_t *ap_conn)
   ap_conn->stream_id = get_unique_stream_id_by_circ(circ);
   if (ap_conn->stream_id==0) {
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
+    /*XXXX022 _close_ the circuit because it's full?  That sounds dumb. */
     circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);
     return -1;
   }
diff --git a/src/or/or.h b/src/or/or.h
index 124892ce7b..e44c626d73 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2127,8 +2127,14 @@ typedef struct circuit_t {
   char *n_conn_onionskin;
 
   time_t timestamp_created; /**< When was this circuit created? */
-  time_t timestamp_dirty; /**< When the circuit was first used, or 0 if the
-                           * circuit is clean. */
+  /** When the circuit was first used, or 0 if the circuit is clean.
+   *
+   * XXXX023 Note that some code will artifically adjust this value backward
+   * in time in order to indicate that a circuit shouldn't be used for new
+   * streams, but that it can stay alive as long as it has streams on it.
+   * That's a kludge we should fix.
+   */
+  time_t timestamp_dirty;
   struct timeval highres_created; /**< When exactly was the circuit created? */
 
   uint16_t marked_for_close; /**< Should we close this circuit at the end of
diff --git a/src/or/relay.c b/src/or/relay.c
index 076b46b934..807cb3d435 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -791,6 +791,8 @@ connection_ap_process_end_not_open(
             < MAX_RESOLVE_FAILURES) {
           /* We haven't retried too many times; reattach the connection. */
           circuit_log_path(LOG_INFO,LD_APP,circ);
+          /* Mark this circuit "unusable for new streams". */
+          /* XXXX023 this is a kludgy way to do this. */
           tor_assert(circ->_base.timestamp_dirty);
           circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
 

From 432734279d3688fafb466a23f43585ff509ff693 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 17:57:15 -0400
Subject: [PATCH 11/14] Fix handling of StreamID exhaustion.

Since svn r1475/git 5b6099e8 in tor-0.0.6, we have responded to an
exhaustion of all 65535 stream IDs on a circuit by marking that
circuit for close.  That's not the right response.  Instead, we
should mark the circuit as "too dirty for new circuits".

Of course in reality this isn't really right either.  If somebody
has managed to cram 65535 streams onto a circuit, the circuit is
probably not going to work well for any of those streams, so maybe
we should be limiting the number of streams on an origin circuit
concurrently.

Also, closing the stream in this case is probably the wrong thing to
do as well, but fixing that can also wait.
---
 changes/full_ap_circuits |  6 ++++++
 src/or/connection_edge.c | 18 ++++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 changes/full_ap_circuits

diff --git a/changes/full_ap_circuits b/changes/full_ap_circuits
new file mode 100644
index 0000000000..379a1a1b73
--- /dev/null
+++ b/changes/full_ap_circuits
@@ -0,0 +1,6 @@
+  o Minor bugfixes
+    - When a client finds that an origin circuit has run out of 16-bit
+      stream IDs, we now mark it as unusable for new streams.
+      Previously, we would try to close the entire circuit.  Bugfix on
+      Tor version 0.0.6.
+
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index af0cfbe14c..72e2c8a409 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2164,9 +2164,14 @@ connection_ap_handshake_send_begin(edge_connection_t *ap_conn)
 
   ap_conn->stream_id = get_unique_stream_id_by_circ(circ);
   if (ap_conn->stream_id==0) {
+    /* XXXX023 Instead of closing this stream, we should make it get
+     * retried on another circuit. */
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
-    /*XXXX022 _close_ the circuit because it's full?  That sounds dumb. */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);
+
+    /* Mark this circuit "unusable for new streams". */
+    /* XXXX023 this is a kludgy way to do this. */
+    tor_assert(circ->_base.timestamp_dirty);
+    circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
     return -1;
   }
 
@@ -2224,9 +2229,14 @@ connection_ap_handshake_send_resolve(edge_connection_t *ap_conn)
 
   ap_conn->stream_id = get_unique_stream_id_by_circ(circ);
   if (ap_conn->stream_id==0) {
+    /* XXXX023 Instead of closing this stream, we should make it get
+     * retried on another circuit. */
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
-    /*XXXX022 _close_ the circuit because it's full?  That sounds dumb. */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);
+
+    /* Mark this circuit "unusable for new streams". */
+    /* XXXX023 this is a kludgy way to do this. */
+    tor_assert(circ->_base.timestamp_dirty);
+    circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
     return -1;
   }
 

From 8d81831d715adf9cb374100e23cc2b118efe3101 Mon Sep 17 00:00:00 2001
From: Sebastian Hahn <sebastian@torproject.org>
Date: Mon, 28 Mar 2011 18:20:50 +0200
Subject: [PATCH 12/14] Add a missing cast to silence the compiler

---
 src/or/connection.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/or/connection.c b/src/or/connection.c
index 084237dea1..6e7bbd5bad 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2790,7 +2790,7 @@ connection_handle_write_impl(connection_t *conn, int force)
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
     /* Check for overflow: */
     if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written))
-      edge_conn->n_written += n_written;
+      edge_conn->n_written += (int)n_written;
     else
       edge_conn->n_written = UINT32_MAX;
   }

From 9facf8918f959cc9d9c2674afbd0630718460dc3 Mon Sep 17 00:00:00 2001
From: Sebastian Hahn <sebastian@torproject.org>
Date: Mon, 28 Mar 2011 19:28:04 +0200
Subject: [PATCH 13/14] Improve a few comments

---
 src/common/address.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/common/address.c b/src/common/address.c
index a089260de8..adc0ef0f7c 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -51,11 +51,11 @@
 
 /** Convert the tor_addr_t in <b>a</b>, with port in <b>port</b>, into a
  * sockaddr object in *<b>sa_out</b> of object size <b>len</b>.  If not enough
- * room is available in sa_out, or on error, return 0  On success, return
+ * room is available in sa_out, or on error, return 0.  On success, return
  * the length of the sockaddr.
  *
- * Interface note: ordinarly, we return -1 for error.  We can't do that here,
- * since socklen is unsigned on some platforms.
+ * Interface note: ordinarily, we return -1 for error.  We can't do that here,
+ * since socklen_t is unsigned on some platforms.
  **/
 socklen_t
 tor_addr_to_sockaddr(const tor_addr_t *a,

From fc647832783cab352bebba63fe0210d7be395058 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 28 Mar 2011 13:42:59 -0400
Subject: [PATCH 14/14] Send END_STREAM_REASON_NOROUTE: clients that didn't
 grok it are now obsolete

---
 changes/noroute  | 5 +++++
 src/or/reasons.c | 8 +-------
 2 files changed, 6 insertions(+), 7 deletions(-)
 create mode 100644 changes/noroute

diff --git a/changes/noroute b/changes/noroute
new file mode 100644
index 0000000000..644deec453
--- /dev/null
+++ b/changes/noroute
@@ -0,0 +1,5 @@
+  - Minor features
+    - Send END_STREAM_REASON_NOROUTE in response to EHOSTUNREACH errors.
+      Clients before 0.2.1.27 didn't handle NOROUTE correctly, but
+      such clients are already deprecated because of security bugs.
+
diff --git a/src/or/reasons.c b/src/or/reasons.c
index 27c947edff..319e6c055a 100644
--- a/src/or/reasons.c
+++ b/src/or/reasons.c
@@ -174,13 +174,7 @@ errno_to_stream_end_reason(int e)
     S_CASE(ENETUNREACH):
       return END_STREAM_REASON_INTERNAL;
     S_CASE(EHOSTUNREACH):
-      /* XXXX023
-       * The correct behavior is END_STREAM_REASON_NOROUTE, but older
-       * clients don't recognize it.  So we're going to continue sending
-       * "MISC" until 0.2.1.27 or later is "well established".
-       */
-      /* return END_STREAM_REASON_NOROUTE; */
-      return END_STREAM_REASON_MISC;
+      return END_STREAM_REASON_NOROUTE;
     S_CASE(ECONNREFUSED):
       return END_STREAM_REASON_CONNECTREFUSED;
     S_CASE(ECONNRESET):