r17936@catbus: nickm | 2008-02-06 00:31:11 -0500

Fix/downgrade some more XXX020s.


svn:r13397

src/or/buffers.c

@@ -176,7 +176,8 @@ chunk_new_with_alloc_size(size_t alloc)
       freelist->lowest_length = freelist->cur_length;
     ++freelist->n_hit;
   } else {
-    /* XXXX020 take advantage of tor_malloc_roundup. */
+    /* XXXX020 take advantage of tor_malloc_roundup, once we know how that
+     * affects freelists. */
     if (freelist)
       ++freelist->n_alloc;
     else
@@ -950,8 +951,8 @@ fetch_var_cell_from_buf(buf_t *buf, var_cell_t **out)
 int
 move_buf_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen)
 {
-  /* XXXX020 we can do way better here.  See if this turns up in the
-   */
+  /* XXXX we can do way better here, but this doesn't turn up in any
+   * profiles. */
   char b[4096];
   size_t cp, len;
   len = *buf_flushlen;

src/or/command.c

@@ -78,6 +78,7 @@ command_time_process_cell(cell_t *cell, or_connection_t *conn, int *time,
 void
 command_process_cell(cell_t *cell, or_connection_t *conn)
 {
+  int handshaking = (conn->_base.state == OR_CONN_STATE_OR_HANDSHAKING);
 #ifdef KEEP_TIMING_STATS
   /* how many of each cell have we seen so far this second? needs better
    * name. */
@@ -117,8 +118,10 @@ command_process_cell(cell_t *cell, or_connection_t *conn)
 #define PROCESS_CELL(tp, cl, cn) command_process_ ## tp ## _cell(cl, cn)
 #endif
 
-  /*XXXX020 reject all but VERSIONS, NETINFO, CERT, LINK_AUTH when
-   * handshaking. */
+  /* Reject all but VERSIONS when handshaking. */
+  if (handshaking && cell->command != CELL_VERSIONS)
+    return;
+
   switch (cell->command) {
     case CELL_PADDING:
       ++stats_n_padding_cells_processed;
@@ -187,7 +190,10 @@ command_process_var_cell(var_cell_t *cell, or_connection_t *conn)
   }
 #endif
 
-  /*XXXX020 reject all when not handshaking. */
+  /* reject all when not handshaking. */
+  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING)
+    return;
+
   switch (cell->command) {
     case CELL_VERSIONS:
       ++stats_n_versions_cells_processed;
@@ -556,43 +562,3 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
   conn->handshake_state->received_netinfo = 1;
 }
 
-/*XXXX020 move to connection_or.c */
-/** DOCDOC Called when we're done authenticating; act on stuff we
- * learned in netinfo. */
-int
-connection_or_act_on_netinfo(or_connection_t *conn)
-{
-  long delta;
-  if (!conn->handshake_state)
-    return -1;
-
-  tor_assert(conn->handshake_state->received_versions != 0);
-
-  delta = conn->handshake_state->apparent_skew;
-  /*XXXX020 magic number 3600 */
-  if (abs(delta) > 3600 &&
-      router_get_by_digest(conn->identity_digest)) {
-    char dbuf[64];
-    /*XXXX020 not always warn!*/
-    format_time_interval(dbuf, sizeof(dbuf), delta);
-    log_fn(LOG_WARN, LD_HTTP, "Received NETINFO cell with skewed time from "
-           "server at %s:%d.  It seems that our clock is %s by %s, or "
-           "that theirs is %s. Tor requires an accurate clock to work: "
-           "please check your time and date settings.",
-           conn->_base.address, (int)conn->_base.port,
-           delta>0 ? "ahead" : "behind", dbuf,
-           delta>0 ? "behind" : "ahead");
-    control_event_general_status(LOG_WARN,
-                                 "CLOCK_SKEW SKEW=%ld SOURCE=OR:%s:%d",
-                                 delta, conn->_base.address, conn->_base.port);
-  }
-
-  /* XXX020 possibly, learn my address from my_apparent_addr */
-
-  if (conn->handshake_state->apparently_canonical) {
-    conn->is_canonical = 1;
-  }
-
-  return 0;
-}
-

src/or/connection_or.c

@@ -1089,3 +1089,43 @@ connection_or_send_netinfo(or_connection_t *conn)
   return 0;
 }
 
+/** DOCDOC Called when we're done authenticating; act on stuff we
+ * learned in netinfo. */
+int
+connection_or_act_on_netinfo(or_connection_t *conn)
+{
+  long delta;
+  if (!conn->handshake_state)
+    return -1;
+
+  tor_assert(conn->handshake_state->received_versions != 0);
+
+  delta = conn->handshake_state->apparent_skew;
+/** Warn when we get a netinfo skew with at least this value. */
+#define NETINFO_NOTICE_SKEW 3600
+  if (abs(delta) > NETINFO_NOTICE_SKEW &&
+      router_get_by_digest(conn->identity_digest)) {
+    char dbuf[64];
+    /*XXXX020 not always warn!*/
+    format_time_interval(dbuf, sizeof(dbuf), delta);
+    log_fn(LOG_WARN, LD_HTTP, "Received NETINFO cell with skewed time from "
+           "server at %s:%d.  It seems that our clock is %s by %s, or "
+           "that theirs is %s. Tor requires an accurate clock to work: "
+           "please check your time and date settings.",
+           conn->_base.address, (int)conn->_base.port,
+           delta>0 ? "ahead" : "behind", dbuf,
+           delta>0 ? "behind" : "ahead");
+    control_event_general_status(LOG_WARN,
+                                 "CLOCK_SKEW SKEW=%ld SOURCE=OR:%s:%d",
+                                 delta, conn->_base.address, conn->_base.port);
+  }
+
+  /* XXX020 possibly, learn my address from my_apparent_addr */
+
+  if (conn->handshake_state->apparently_canonical) {
+    conn->is_canonical = 1;
+  }
+
+  return 0;
+}
+

src/or/main.c

@@ -416,8 +416,9 @@ connection_stop_reading_from_linked_conn(connection_t *conn)
 
   if (conn->active_on_link) {
     conn->active_on_link = 0;
-    /* XXXX020 maybe we should keep an index here so we can smartlist_del
-     * cleanly. */
+    /* FFFF We could keep an index here so we can smartlist_del
+     * cleanly.  On the other hand, this doesn't show up on profiles,
+     * so let's leave it alone for now. */
     smartlist_remove(active_linked_connection_lst, conn);
   } else {
     tor_assert(!smartlist_isin(active_linked_connection_lst, conn));
@@ -552,8 +553,7 @@ conn_close_if_marked(int i)
          * we're gone. */
         connection_start_reading_from_linked_conn(conn->linked_conn);
       }
-      /* XXXX020 Downgrade to debug. */
-      log_info(LD_GENERAL, "Flushed last %d bytes from a linked conn; "
+      log_debug(LD_GENERAL, "Flushed last %d bytes from a linked conn; "
                "%d left; flushlen %d; wants-to-flush==%d", retval,
                (int)buf_datalen(conn->outbuf),
                (int)conn->outbuf_flushlen,
@@ -795,6 +795,17 @@ run_connection_housekeeping(int i, time_t now)
   }
 }
 
+/** Honor a NEWNYM request: make future requests unlinkability to past
+ * requests. */
+static void
+signewnym_impl(time_t now)
+{
+  circuit_expire_all_dirty_circs();
+  addressmap_clear_transient();
+  time_of_last_signewnym = now;
+  signewnym_is_pending = 0;
+}
+
 /** Perform regular maintenance tasks.  This function gets run once per
  * second by prepare_for_poll.
  */
@@ -834,10 +845,7 @@ run_scheduled_events(time_t now)
   if (signewnym_is_pending &&
       time_of_last_signewnym + MAX_SIGNEWNYM_RATE <= now) {
     log(LOG_INFO, LD_CONTROL, "Honoring delayed NEWNYM request");
-    circuit_expire_all_dirty_circs();
-    addressmap_clear_transient();
-    time_of_last_signewnym = now;
-    signewnym_is_pending = 0;
+    signewnym_impl(now);
   }
 
   /** 1a. Every MIN_ONION_KEY_LIFETIME seconds, rotate the onion keys,
@@ -1471,7 +1479,7 @@ do_main_loop(void)
 
     /* refilling buckets and sending cells happens at the beginning of the
      * next iteration of the loop, inside prepare_for_poll()
-     * XXXX020 No longer so; fix comment.
+     * DOCDOC No longer so; fix comment.
      */
   }
 }
@@ -1574,12 +1582,7 @@ signal_callback(int fd, short events, void *arg)
             "Rate limiting NEWNYM request: delaying by %d second(s)",
             (int)(MAX_SIGNEWNYM_RATE+time_of_last_signewnym-now));
       } else {
-        /* XXX020 refactor someday: these two calls are in
-         * run_scheduled_events() above too, and they should be in just
-         * one place. */
-        circuit_expire_all_dirty_circs();
-        addressmap_clear_transient();
-        time_of_last_signewnym = now;
+        signewnym_impl(now);
       }
       break;
     }

src/or/or.h

@@ -2613,7 +2613,6 @@ int connection_ap_handshake_attach_circuit(edge_connection_t *conn);
 
 void command_process_cell(cell_t *cell, or_connection_t *conn);
 void command_process_var_cell(var_cell_t *cell, or_connection_t *conn);
-int connection_or_act_on_netinfo(or_connection_t *conn);
 
 extern uint64_t stats_n_padding_cells_processed;
 extern uint64_t stats_n_create_cells_processed;
@@ -2864,6 +2863,7 @@ void cell_pack(packed_cell_t *dest, const cell_t *src);
 void var_cell_pack_header(const var_cell_t *cell, char *hdr_out);
 var_cell_t *var_cell_new(uint16_t payload_len);
 void var_cell_free(var_cell_t *cell);
+int connection_or_act_on_netinfo(or_connection_t *conn);
 
 /********************************* control.c ***************************/
 

src/or/routerlist.c

@@ -41,6 +41,13 @@ static void list_pending_downloads(digestmap_t *result,
 DECLARE_TYPED_DIGESTMAP_FNS(sdmap_, digest_sd_map_t, signed_descriptor_t)
 DECLARE_TYPED_DIGESTMAP_FNS(rimap_, digest_ri_map_t, routerinfo_t)
 DECLARE_TYPED_DIGESTMAP_FNS(eimap_, digest_ei_map_t, extrainfo_t)
+#define SDMAP_FOREACH(map, keyvar, valvar)                              \
+  DIGESTMAP_FOREACH(sdmap_to_digestmap(map), keyvar, signed_descriptor_t *, \
+                    valvar)
+#define RIMAP_FOREACH(map, keyvar, valvar) \
+  DIGESTMAP_FOREACH(rimap_to_digestmap(map), keyvar, routerinfo_t *, valvar)
+#define EIMAP_FOREACH(map, keyvar, valvar) \
+  DIGESTMAP_FOREACH(eimap_to_digestmap(map), keyvar, extrainfo_t *, valvar)
 
 /****************************************************************************/
 
@@ -222,7 +229,7 @@ trusted_dirs_flush_certs_to_disk(void)
             c->len = cert->cache_info.signed_descriptor_len;
             smartlist_add(chunks, c);
           });
-  } DIGESTMAP_FOREACH_END
+  } DIGESTMAP_FOREACH_END;
 
   filename = get_datadir_fname("cached-certs");
   if (write_chunks_to_file(filename, chunks, 0)) {
@@ -259,7 +266,7 @@ trusted_dirs_remove_old_certs(void)
             authority_cert_free(cert);
             trusted_dir_servers_certs_changed = 1;
           });
-  } DIGESTMAP_FOREACH_END
+  } DIGESTMAP_FOREACH_END;
 #undef OLD_CERT_LIFETIME
 
   trusted_dirs_flush_certs_to_disk();
@@ -299,7 +306,7 @@ authority_cert_get_by_sk_digest(const char *sk_digest)
       if (!memcmp(cert->signing_key_digest, sk_digest, DIGEST_LEN))
         return cert;
     });
-  } DIGESTMAP_FOREACH_END
+  } DIGESTMAP_FOREACH_END;
   return NULL;
 }
 
@@ -331,7 +338,7 @@ authority_cert_get_all(smartlist_t *certs_out)
   DIGESTMAP_FOREACH(trusted_dir_certs, key, cert_list_t *, cl) {
     SMARTLIST_FOREACH(cl->certs, authority_cert_t *, c,
                       smartlist_add(certs_out, c));
-  } DIGESTMAP_FOREACH_END
+  } DIGESTMAP_FOREACH_END;
 }
 
 /** DOCDOC */
@@ -4300,7 +4307,6 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
 void
 routerlist_assert_ok(routerlist_t *rl)
 {
-  digestmap_iter_t *iter; /* XXXX020 use the appropriate iter type. */
   routerinfo_t *r2;
   signed_descriptor_t *sd2;
   if (!rl)
@@ -4355,46 +4361,19 @@ routerlist_assert_ok(routerlist_t *rl)
 #endif
   });
 
-  iter = digestmap_iter_init((digestmap_t*)rl->identity_map);
-  while (!digestmap_iter_done(iter)) {
-    const char *d;
-    void *_r;
-    routerinfo_t *r;
-    digestmap_iter_get(iter, &d, &_r);
-    r = _r;
+  RIMAP_FOREACH(rl->identity_map, d, r) {
     tor_assert(!memcmp(r->cache_info.identity_digest, d, DIGEST_LEN));
-    iter = digestmap_iter_next((digestmap_t*)rl->identity_map, iter);
-  }
-  iter = digestmap_iter_init((digestmap_t*)rl->desc_digest_map);
-  while (!digestmap_iter_done(iter)) {
-    const char *d;
-    void *_sd;
-    signed_descriptor_t *sd;
-    digestmap_iter_get(iter, &d, &_sd);
-    sd = _sd;
+  } DIGESTMAP_FOREACH_END;
+  SDMAP_FOREACH(rl->desc_digest_map, d, sd) {
     tor_assert(!memcmp(sd->signed_descriptor_digest, d, DIGEST_LEN));
-    iter = digestmap_iter_next((digestmap_t*)rl->desc_digest_map, iter);
-  }
-  iter = digestmap_iter_init((digestmap_t*)rl->desc_by_eid_map);
-  while (!digestmap_iter_done(iter)) {
-    const char *d;
-    void *_sd;
-    signed_descriptor_t *sd;
-    digestmap_iter_get(iter, &d, &_sd);
-    sd = _sd;
+  } DIGESTMAP_FOREACH_END;
+  SDMAP_FOREACH(rl->desc_by_eid_map, d, sd) {
     tor_assert(!tor_digest_is_zero(d));
     tor_assert(sd);
     tor_assert(!memcmp(sd->extra_info_digest, d, DIGEST_LEN));
-    iter = digestmap_iter_next((digestmap_t*)rl->desc_by_eid_map, iter);
-  }
-  iter = digestmap_iter_init((digestmap_t*)rl->extra_info_map);
-  while (!digestmap_iter_done(iter)) {
-    const char *d;
-    void *_ei;
-    extrainfo_t *ei;
+  } DIGESTMAP_FOREACH_END;
+  EIMAP_FOREACH(rl->extra_info_map, d, ei) {
     signed_descriptor_t *sd;
-    digestmap_iter_get(iter, &d, &_ei);
-    ei = _ei;
     tor_assert(!memcmp(ei->cache_info.signed_descriptor_digest,
                        d, DIGEST_LEN));
     sd = sdmap_get(rl->desc_by_eid_map,
@@ -4404,8 +4383,7 @@ routerlist_assert_ok(routerlist_t *rl)
       tor_assert(!memcmp(ei->cache_info.signed_descriptor_digest,
                          sd->extra_info_digest, DIGEST_LEN));
     }
-    iter = digestmap_iter_next((digestmap_t*)rl->extra_info_map, iter);
-  }
+  } DIGESTMAP_FOREACH_END;
 }
 
 /** Allocate and return a new string representing the contact info

src/or/routerparse.c

@@ -2343,7 +2343,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
 ns_detached_signatures_t *
 networkstatus_parse_detached_signatures(const char *s, const char *eos)
 {
-  /* XXXX020 there is too much duplicate code here. */
+  /* XXXX there is too much duplicate code here. */
   directory_token_t *tok;
 
   smartlist_t *tokens = smartlist_create();