r14182@tombo: nickm | 2008-02-15 17:20:51 -0500

Defer, downgrade, or address more XXX020s.  The remaining ones are all ones we should deal with before release.


svn:r13530

src/common/tortls.c

@@ -459,7 +459,7 @@ tor_tls_create_certificate(crypto_pk_env_t *rsa,
  * really exist; if I understand correctly, it's a bit of silliness that
  * netscape did on its own before any standard for what they wanted was
  * formally approved.  Nonetheless, Firefox still uses it, so we need to
- * fake it at some point soon. XXXX020 -NM */
+ * fake it at some point soon. XXXX021 -NM */
 #else
 /* Ug. We don't have as many ciphers with openssl 0.9.7 as we'd like.  Fix
  * this list into something that sucks less. */

src/or/command.c

@@ -591,7 +591,8 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
                         apparent_skew, conn->_base.address, conn->_base.port);
   }
 
-  /*XXX020 maybe act on my_apparent_addr */
+  /* XXX021 maybe act on my_apparent_addr, if the source is sufficiently
+   * trustworthy. */
 
   if (connection_or_set_state_open(conn)<0)
     connection_mark_for_close(TO_CONN(conn));

src/or/connection.c

@@ -613,12 +613,6 @@ _connection_mark_for_close(connection_t *conn, int line, const char *file)
   conn->marked_for_close_file = file;
   add_connection_to_closeable_list(conn);
 
-#if 0
-  /* XXXX020 Actually, I don't think this is right. */
-  if (conn->linked_conn && !conn->linked_conn->marked_for_close)
-    _connection_mark_for_close(conn->linked_conn, line, file);
-#endif
-
   /* in case we're going to be held-open-til-flushed, reset
    * the number of seconds since last successful write, so
    * we get our whole 15 seconds */
@@ -1681,7 +1675,8 @@ connection_bucket_refill_helper(int *bucket, int rate, int burst,
       if (*bucket > burst || *bucket < starting_bucket) {
         /* If we overflow the burst, or underflow our starting bucket,
          * cap the bucket value to burst. */
-        /* XXXX020 this might be redundant now. */
+        /* XXXX021 this might be redundant now, but it doesn't show up
+         * in profiles.  Remove it after analysis. */
         *bucket = burst;
       }
     }

src/or/directory.c

@@ -2335,7 +2335,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; /* XXXX020 should actually use vote intervals. */
+    int lifetime = 60; /* XXXX021 should actually use vote intervals. */
     url += strlen("/tor/status-vote/");
     current = !strcmpstart(url, "current/");
     url = strchr(url, '/');

src/or/dirserv.c

@@ -861,7 +861,8 @@ directory_set_dirty(void)
   time_t now = time(NULL);
   int set_v1_dirty=0;
 
-  /* Regenerate stubs only every 8 hours. XXXX020 */
+  /* Regenerate stubs only every 8 hours.
+   * XXXX021 It would be nice to generate less often. */
 #define STUB_REGENERATE_INTERVAL (8*60*60)
   if (!the_directory || !the_runningrouters.dir)
     set_v1_dirty = 1;
@@ -1430,7 +1431,7 @@ dirserv_clear_old_v1_info(time_t now)
   }
 }
 
-/** Helper: If we're an authority for the right directory version
+/** Helper: If we're an authority for the right directory version (v1 or v2)
  * (based on <b>auth_type</b>), try to regenerate
  * auth_src as appropriate and return it, falling back to cache_src on
  * failure.  If we're a cache, simply return cache_src.
@@ -1445,7 +1446,6 @@ dirserv_pick_cached_dir_obj(cached_dir_t *cache_src,
   or_options_t *options = get_options();
   int authority = (auth_type == V1_AUTHORITY && authdir_mode_v1(options)) ||
                   (auth_type == V2_AUTHORITY && authdir_mode_v2(options));
-  /* XXX020 eventually use authdir_mode_publishes_statuses() here */
 
   if (!authority || authdir_mode_bridge(options)) {
     return cache_src;
@@ -1814,7 +1814,7 @@ version_from_platform(const char *platform)
   if (platform && !strcmpstart(platform, "Tor ")) {
     const char *eos = find_whitespace(platform+4);
     if (eos && !strcmpstart(eos, " (r")) {
-      /* XXXX020 Unify this logic with the other version extraction
+      /* XXXX021 Unify this logic with the other version extraction
        * logic */
       eos = find_whitespace(eos+1);
     }
@@ -2618,8 +2618,8 @@ dirserv_get_routerdesc_fingerprints(smartlist_t *fps_out, const char *key,
  * If -1 is returned *<b>msg</b> will be set to an appropriate error
  * message.
  *
- * XXXX020 rename this function.  IT's only called from the controller.
- * XXXX020 in fact, refactor this function, mergeing as much as possible.
+ * XXXX021 rename this function.  It's only called from the controller.
+ * XXXX021 in fact, refactor this function, mergeing as much as possible.
  */
 int
 dirserv_get_routerdescs(smartlist_t *descs_out, const char *key,

src/or/dns.c

@@ -851,8 +851,6 @@ dns_cancel_pending_resolve(const char *address)
   }
 
   if (!resolve->pending_connections) {
-    /* XXX this should never trigger, but sometimes it does */
-    /* XXXX020 is the above still true? -NM */
     log_warn(LD_BUG,
              "Address %s is pending but has no pending connections!",
              escaped_safe_str(address));
@@ -872,8 +870,7 @@ dns_cancel_pending_resolve(const char *address)
     assert_connection_ok(TO_CONN(pendconn), 0);
     tor_assert(pendconn->_base.s == -1);
     if (!pendconn->_base.marked_for_close) {
-      /* XXXX020 RESOURCELIMIT?  Not RESOLVEFAILED??? */
-      connection_edge_end(pendconn, END_STREAM_REASON_RESOURCELIMIT);
+      connection_edge_end(pendconn, END_STREAM_REASON_RESOLVEFAILED);
     }
     circ = circuit_get_by_edge_conn(pendconn);
     if (circ)
@@ -1269,7 +1266,7 @@ launch_resolve(edge_connection_t *exitconn)
     log_info(LD_EXIT, "Launching eventdns request for %s",
              escaped_safe_str(exitconn->_base.address));
     r = evdns_resolve_ipv4(exitconn->_base.address, options,
-                              evdns_callback, addr);
+                           evdns_callback, addr);
   } else if (r == 1) {
     log_info(LD_EXIT, "Launching eventdns reverse request for %s",
              escaped_safe_str(exitconn->_base.address));
@@ -1425,13 +1422,14 @@ launch_wildcard_check(int min_len, int max_len, const char *suffix)
   log_info(LD_EXIT, "Testing whether our DNS server is hijacking nonexistent "
            "domains with request for bogus hostname \"%s\"", addr);
 
-  r = evdns_resolve_ipv4(addr, DNS_QUERY_NO_SEARCH,
-                         evdns_wildcard_check_callback, addr);
-  if (r)
+  r = evdns_resolve_ipv4(/* This "addr" tells us which address to resolve */
+                         addr,
+                         DNS_QUERY_NO_SEARCH, evdns_wildcard_check_callback,
+                         /* This "addr" is an argument to the callback*/ addr);
+  if (r) {
+    /* There is no evdns request in progress; stop addr from getting leaked */
     tor_free(addr);
-  /* XXX020 Nick, the above "if" needs some explanation. Plus the fact
-   * that we're sending addr twice. Given that evdns_resolve_ipv4() has
-   * no doxygen documentation. -RD */
+  }
 }
 
 /** Launch attempts to resolve a bunch of known-good addresses (configured in