Downlgrade tweak, and answer lots of XXX021s. No actual code fixes in this patch.

svn:r17686

src/common/address.c

@@ -995,7 +995,7 @@ get_interface_address6(int severity, sa_family_t family, tor_addr_t *addr)
 
 /* ======
  * IPv4 helpers
- * XXXX021 IPv6 deprecate some of these.
+ * XXXX022 IPv6 deprecate some of these.
  */
 
 /** Return true iff <b>ip</b> (in host order) is an IP reserved to localhost,

src/common/tortls.h

@@ -22,7 +22,7 @@ typedef struct tor_tls_t tor_tls_t;
 /* Possible return values for most tor_tls_* functions. */
 #define _MIN_TOR_TLS_ERROR_VAL     -9
 #define TOR_TLS_ERROR_MISC         -9
-/* Rename to unexpected close or something. XXXX021 */
+/* Rename to unexpected close or something. XXXX */
 #define TOR_TLS_ERROR_IO           -8
 #define TOR_TLS_ERROR_CONNREFUSED  -7
 #define TOR_TLS_ERROR_CONNRESET    -6

src/common/util.c

@@ -1286,7 +1286,7 @@ format_time_interval(char *out, size_t out_len, long interval)
 
 /* =====
  * Fuzzy time
- * XXXX021 Use this consistently or rip it out.
+ * XXXX022 Use this consistently or rip it out.
  * ===== */
 
 /* In a perfect world, everybody would run ntp, and ntp would be perfect, so

src/or/buffers.c

@@ -186,7 +186,7 @@ chunk_new_with_alloc_size(size_t alloc)
       freelist->lowest_length = freelist->cur_length;
     ++freelist->n_hit;
   } else {
-    /* XXXX021 take advantage of tor_malloc_roundup, once we know how that
+    /* XXXX take advantage of tor_malloc_roundup, once we know how that
      * affects freelists. */
     if (freelist)
       ++freelist->n_alloc;

src/or/circuitbuild.c

@@ -1688,8 +1688,8 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
     routerlist_add_family(excluded, r);
   }
   if (firewall_is_fascist_or()) {
-    /*XXXX021 This can slow things down a lot; use a smarter implementation */
-    /* exclude all ORs that listen on the wrong port */
+    /*XXXX This could slow things down a lot; use a smarter implementation */
+    /* exclude all ORs that listen on the wrong port, if anybody notices. */
     routerlist_t *rl = router_get_routerlist();
     int i;
 
@@ -1898,7 +1898,7 @@ build_state_get_exit_nickname(cpath_build_state_t *state)
  *
  * If it's not usable, set *<b>reason</b> to a static string explaining why.
  */
-/*XXXX021 take a routerstatus, not a routerinfo. */
+/*XXXX take a routerstatus, not a routerinfo. */
 static int
 entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri,
                        time_t now, or_options_t *options, const char **reason)

src/or/circuitlist.c

@@ -872,7 +872,7 @@ origin_circuit_t *
 circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
                             int flags)
 {
-  /*XXXX021 arma: The purpose argument is ignored.  Can that possibly be
+  /* XXXX021 arma: The purpose argument is ignored.  Can that possibly be
    * right? */
   /* XXXX <arma> i don't know of any actual bugs that this causes. since i
    * think we only call the function for purposes where we want it to do what

src/or/circuituse.c

@@ -1084,8 +1084,7 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn,
         return -1;
       }
     } else {
-      /* XXXX021 Duplicates checks in connection_ap_handshake_attach_circuit
-       * XXXX021 Fix this, then backport it? */
+      /* XXXX021 Duplicates checks in connection_ap_handshake_attach_circuit */
       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)) {
@@ -1363,7 +1362,7 @@ connection_ap_handshake_attach_chosen_circuit(edge_connection_t *conn,
  * Otherwise, associate conn with a safe live circuit, do the
  * right next step, and return 1.
  */
-/* XXXX021 this function should mark for close whenever it returns -1;
+/* XXXX this function should mark for close whenever it returns -1;
  * its callers shouldn't have to worry about that. */
 int
 connection_ap_handshake_attach_circuit(edge_connection_t *conn)

src/or/command.c

@@ -597,7 +597,7 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
       router_get_by_digest(conn->identity_digest)) {
     char dbuf[64];
     int severity;
-    /*XXXX021 be smarter about when everybody says we are skewed. */
+    /*XXXX be smarter about when everybody says we are skewed. */
     if (router_digest_is_trusted_dir(conn->identity_digest))
       severity = LOG_WARN;
     else
@@ -615,7 +615,7 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
                         apparent_skew, conn->_base.address, conn->_base.port);
   }
 
-  /* XXX021 maybe act on my_apparent_addr, if the source is sufficiently
+  /* XXX maybe act on my_apparent_addr, if the source is sufficiently
    * trustworthy. */
 
   if (connection_or_set_state_open(conn)<0)

src/or/config.c

@@ -1349,9 +1349,9 @@ options_act(or_options_t *old_options)
   if (options->GeoIPFile &&
       ((!old_options || !opt_streq(old_options->GeoIPFile, options->GeoIPFile))
        || !geoip_is_loaded())) {
-    /** XXXX021 Don't use this "<default>" junk; make our filename options
+    /* XXXX Don't use this "<default>" junk; make our filename options
      * understand prefixes somehow. -NM */
-    /** XXXX021 Reload GeoIPFile on SIGHUP. -NM */
+    /* XXXX021 Reload GeoIPFile on SIGHUP. -NM */
     char *actual_fname = tor_strdup(options->GeoIPFile);
 #ifdef WIN32
     if (!strcmp(actual_fname, "<default>")) {
@@ -2151,7 +2151,7 @@ options_trial_assign(config_line_t *list, int use_defaults,
 
   if (options_validate(get_options(), trial_options, 1, msg) < 0) {
     config_free(&options_format, trial_options);
-    return SETOPT_ERR_PARSE; /*XXX021 make this separate. */
+    return SETOPT_ERR_PARSE; /*XXX make this a separate return value. */
   }
 
   if (options_transition_allowed(get_options(), trial_options, msg) < 0) {
@@ -3038,7 +3038,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
     COMPLAIN("StrictEntryNodes set, but no EntryNodes listed.");
 
   if (options->EntryNodes && !routerset_is_list(options->EntryNodes)) {
-    /** XXXX021 fix this; see entry_guards_prepend_from_config(). */
+    /* XXXX fix this; see entry_guards_prepend_from_config(). */
     REJECT("IPs or countries are not yet supported in EntryNodes.");
   }
 
@@ -3672,7 +3672,7 @@ static int
 options_transition_affects_descriptor(or_options_t *old_options,
                                       or_options_t *new_options)
 {
-  /* XXX021 We can be smarter here. If your DirPort isn't being
+  /* XXX We can be smarter here. If your DirPort isn't being
    * published and you just turned it off, no need to republish. If
    * you changed your bandwidthrate but maxadvertisedbandwidth still
    * trumps, no need to republish. Etc. */
@@ -4022,7 +4022,7 @@ options_init_from_string(const char *cf,
    * for a list of dependent config options, re-initialize newoptions
    * with the new defaults, and assign all options to it second time. */
   if (newoptions->TestingTorNetwork) {
-    /* XXXX021 this is a bit of a kludge.  perhaps there's a better way to do
+    /* XXXX this is a bit of a kludge.  perhaps there's a better way to do
      * this?  We could, for example, make the parsing algorithm do two passes
      * over the configuration.  If it finds any "suite" options like
      * TestingTorNetwork, it could change the defaults before its second pass.
@@ -4070,7 +4070,7 @@ options_init_from_string(const char *cf,
 
   /* Validate newoptions */
   if (options_validate(oldoptions, newoptions, 0, msg) < 0) {
-    err = SETOPT_ERR_PARSE; /*XXX021 make this separate.*/
+    err = SETOPT_ERR_PARSE; /*XXX make this a separate return value.*/
     goto err;
   }
 

src/or/connection.c

@@ -563,7 +563,8 @@ connection_about_to_close_connection(connection_t *conn)
             int reason = tls_error_to_orconn_end_reason(or_conn->tls_error);
             control_event_or_conn_status(or_conn, OR_CONN_EVENT_FAILED,
                                          reason);
-            /* XXX021 come up with a better string for the first arg */
+            /* XXX021 come up with a better string for the first arg -RD */
+            /* What did you have in mind? -NM */
             if (!authdir_mode_tests_reachability(options))
               control_event_bootstrap_problem(
                 orconn_end_reason_to_control_string(reason), reason);
@@ -1800,7 +1801,7 @@ 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. */
-        /* XXXX021 this might be redundant now, but it doesn't show up
+        /* XXXX this might be redundant now, but it doesn't show up
          * in profiles.  Remove it after analysis. */
         *bucket = burst;
       }
@@ -2033,7 +2034,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error)
 
   if (at_most == -1) { /* we need to initialize it */
     /* how many bytes are we allowed to read? */
-    /* XXXX too many calls to time(). Do they hurt? */
+    /* XXXX021 too many calls to time(). Do they hurt? */
     at_most = connection_bucket_read_limit(conn, time(NULL));
   }
 

src/or/connection_or.c

@@ -398,14 +398,14 @@ connection_or_init_conn_from_address(or_connection_t *conn,
   tor_addr_copy(&conn->_base.addr, addr);
   tor_addr_copy(&conn->real_addr, addr);
   if (r) {
-    /* XXXX021 proposal 118 will make this more complex. */
+    /* XXXX proposal 118 will make this more complex. */
     if (tor_addr_eq_ipv4h(&conn->_base.addr, r->addr))
       conn->is_canonical = 1;
     if (!started_here) {
       /* Override the addr/port, so our log messages will make sense.
        * This is dangerous, since if we ever try looking up a conn by
        * its actual addr/port, we won't remember. Careful! */
-      /* XXXX021 arma: this is stupid, and it's the reason we need real_addr
+      /* XXXX arma: this is stupid, and it's the reason we need real_addr
        * to track is_canonical properly.  What requires it? */
       /* XXXX <arma> i believe the reason we did this, originally, is because
        * we wanted to log what OR a connection was to, and if we logged the
@@ -929,7 +929,7 @@ connection_or_set_state_open(or_connection_t *conn)
     /* only report it to the geoip module if it's not a known router */
     if (!router_get_by_digest(conn->identity_digest)) {
       if (tor_addr_family(&TO_CONN(conn)->addr) == AF_INET) {
-        /*XXXX021 IP6 support ipv6 geoip.*/
+        /*XXXX IP6 support ipv6 geoip.*/
         uint32_t a = tor_addr_to_ipv4h(&TO_CONN(conn)->addr);
         geoip_note_client_seen(GEOIP_CLIENT_CONNECT, a, now);
       }

src/or/directory.c

@@ -540,7 +540,7 @@ directory_conn_is_self_reachability_test(dir_connection_t *conn)
     routerinfo_t *me = router_get_my_routerinfo();
     if (me &&
         router_digest_is_me(conn->identity_digest) &&
-        tor_addr_eq_ipv4h(&conn->_base.addr, me->addr) && /*XXXX021 prop 118*/
+        tor_addr_eq_ipv4h(&conn->_base.addr, me->addr) && /*XXXX prop 118*/
         me->dir_port == conn->_base.port)
       return 1;
   }
@@ -1804,8 +1804,13 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
           int rejected = 0;
           if (rejected_hdr) {
             if (!strcmp(rejected_hdr, "Yes")) {
-              /* XXXX021 use this information; be sure to upload next one
+              /* XXXX use this information; be sure to upload next one
                * sooner. -NM */
+              /* XXXX021 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
+               * do. -NM */
               rejected = 1;
             }
             tor_free(rejected_hdr);

src/or/dirserv.c

@@ -887,7 +887,8 @@ directory_set_dirty(void)
   int set_v1_dirty=0;
 
   /* Regenerate stubs only every 8 hours.
-   * XXXX021 It would be nice to generate less often. */
+   * XXXX It would be nice to generate less often, but these are just
+   * stubs: it doesn't matter. */
 #define STUB_REGENERATE_INTERVAL (8*60*60)
   if (!the_directory || !the_runningrouters.dir)
     set_v1_dirty = 1;
@@ -2727,8 +2728,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.
  *
- * XXXX021 rename this function.  It's only called from the controller.
- * XXXX021 in fact, refactor this function, mergeing as much as possible.
+ * XXXX rename this function.  It's only called from the controller.
+ * XXXX 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

@@ -441,7 +441,7 @@ send_resolved_cell(edge_connection_t *conn, uint8_t answer_type)
       set_uint32(buf+6, htonl(ttl));
       buflen = 10;
       break;
-    /*XXXX021 IP6 need ipv6 implementation */
+    /*XXXX IP6 need ipv6 implementation */
     case RESOLVED_TYPE_ERROR_TRANSIENT:
     case RESOLVED_TYPE_ERROR:
       {
@@ -499,7 +499,7 @@ send_resolved_hostname_cell(edge_connection_t *conn, const char *hostname)
  * parse it and place the address in <b>in</b> if present. Return 1 on success;
  * 0 if the address is not in in-addr.arpa format, and -1 if the address is
  * malformed. */
-/* XXXX021 move this to util.c. */
+/* XXXX021 move this to address.c; unify with logic in connection_edge.c */
 int
 parse_inaddr_arpa_address(const char *address, struct in_addr *in)
 {

src/or/dnsserv.c

@@ -258,7 +258,7 @@ dnsserv_resolved(edge_connection_t *conn,
   name = evdns_get_orig_address(req, answer_type,
                                 conn->socks_request->address);
 
-  /* XXXX021 Re-do; this is dumb. */
+  /* XXXX Re-do; this is dumb. */
   if (ttl < 60)
     ttl = 60;
 

src/or/networkstatus.c

@@ -908,7 +908,7 @@ router_get_consensus_status_by_nickname(const char *nickname,
 
   /* This name is not canonical for any server; go through the list and
    * see who it matches. */
-  /*XXXX021 This is inefficient. */
+  /*XXXX This is inefficient; optimize it if it matters. */
   matches = smartlist_create();
   SMARTLIST_FOREACH(current_consensus->routerstatus_list,
                     routerstatus_t *, lrs,
@@ -1746,7 +1746,7 @@ routers_update_status_from_consensus_networkstatus(smartlist_t *routers,
   } SMARTLIST_FOREACH_JOIN_END(rs, router);
 
   /* Now update last_listed_as_valid_until from v2 networkstatuses. */
-  /* XXXX021 If this is slow, we need to rethink the code. */
+  /* XXXX If this is slow, we need to rethink the code. */
   SMARTLIST_FOREACH(networkstatus_v2_list, networkstatus_v2_t *, ns, {
     time_t live_until = ns->published_on + V2_NETWORKSTATUS_LIFETIME;
     SMARTLIST_FOREACH_JOIN(ns->entries, routerstatus_t *, rs,

src/or/or.h

@@ -1342,7 +1342,7 @@ typedef struct signed_descriptor_t {
 #ifdef TRACK_SERVED_TIME
   /** The last time we served anybody this descriptor.  Used for internal
    * testing to see whether we're holding on to descriptors too long. */
-  time_t last_served_at; /*XXXX021 remove if not useful. */
+  time_t last_served_at; /*XXXX remove if not useful. */
 #endif
   /* If true, we do not ever try to save this object in the cache. */
   unsigned int do_not_cache : 1;
@@ -4290,11 +4290,11 @@ smartlist_t *router_get_trusted_dir_servers(void);
  *
  * Passed to router_pick_directory_server (et al)
  *
- * [XXXX021 NOTE: This option is only implemented for pick_trusteddirserver,
+ * [XXXX NOTE: This option is only implemented for pick_trusteddirserver,
  *  not pick_directory_server.  If we make it work on pick_directory_server
  *  too, we could conservatively make it only prevent multiple fetches to
  *  the same authority, or we could aggressively make it prevent multiple
- *  fetches to _any_ directory server.]
+ *  fetches to _any_ single directory server.]
  */
 #define PDS_NO_EXISTING_SERVERDESC_FETCH (1<<3)
 #define _PDS_PREFER_TUNNELED_DIR_CONNS (1<<16)
@@ -4334,9 +4334,9 @@ typedef enum {
   CRN_NEED_CAPACITY = 1<<1,
   CRN_NEED_GUARD = 1<<2,
   CRN_ALLOW_INVALID = 1<<3,
-  /* XXXX021 not used, apparently. */
+  /* XXXX not used, apparently. */
   CRN_STRICT_PREFERRED = 1<<4,
-  /* XXXX021 not used, apparently. */
+  /* XXXX not used, apparently. */
   CRN_WEIGHT_AS_EXIT = 1<<5
 } router_crn_flags_t;
 

src/or/policies.c

@@ -69,7 +69,7 @@ policy_expand_private(smartlist_t **policy)
   int i;
   smartlist_t *tmp;
 
-  if (!*policy) /*XXXX021 disallow NULL policies */
+  if (!*policy) /*XXXX disallow NULL policies? */
     return;
 
   tmp = smartlist_create();
@@ -235,7 +235,7 @@ addr_policy_permits_tor_addr(const tor_addr_t *addr, uint16_t port,
   }
 }
 
-/* DOCDOC XXXX021 deprecate? */
+/* DOCDOC XXXX deprecate when possible. */
 static int
 addr_policy_permits_address(uint32_t addr, uint16_t port,
                             smartlist_t *policy)
@@ -258,7 +258,7 @@ fascist_firewall_allows_address_or(const tor_addr_t *addr, uint16_t port)
 int
 fascist_firewall_allows_or(routerinfo_t *ri)
 {
-  /* XXXX021 proposal 118 */
+  /* XXXX proposal 118 */
   tor_addr_t addr;
   tor_addr_from_ipv4h(&addr, ri->addr);
   return fascist_firewall_allows_address_or(&addr, ri->or_port);
@@ -556,7 +556,7 @@ addr_policy_get_canonical_entry(addr_policy_t *e)
 addr_policy_result_t
 compare_addr_to_addr_policy(uint32_t addr, uint16_t port, smartlist_t *policy)
 {
-  /*XXXX021 deprecate this function? */
+  /*XXXX deprecate this function when possible. */
   tor_addr_t a;
   tor_addr_from_ipv4h(&a, addr);
   return compare_tor_addr_to_addr_policy(&a, port, policy);
@@ -825,7 +825,7 @@ exit_policy_is_general_exit(smartlist_t *policy)
   static const int ports[] = { 80, 443, 6667 };
   int n_allowed = 0;
   int i;
-  if (!policy) /*XXXX021 disallow NULL policies */
+  if (!policy) /*XXXX disallow NULL policies? */
     return 0;
 
   for (i = 0; i < 3; ++i) {
@@ -851,7 +851,7 @@ exit_policy_is_general_exit(smartlist_t *policy)
 int
 policy_is_reject_star(smartlist_t *policy)
 {
-  if (!policy) /*XXXX021 disallow NULL policies */
+  if (!policy) /*XXXX disallow NULL policies? */
     return 1;
   SMARTLIST_FOREACH(policy, addr_policy_t *, p, {
     if (p->policy_type == ADDR_POLICY_ACCEPT)

src/or/reasons.c

@@ -248,8 +248,6 @@ tls_error_to_orconn_end_reason(int e)
 
 /** Given an errno from a failed ORConn connection, return a reason code
  * appropriate for use in the controller orconn events. */
-/* XXX021 somebody should think about whether the assignments I've made
- * are accurate or useful. -RD */
 int
 errno_to_orconn_end_reason(int e)
 {

src/or/rendclient.c

@@ -601,9 +601,9 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const char *request,
   log_info(LD_REND,"Got rendezvous ack. This circuit is now ready for "
            "rendezvous.");
   circ->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY;
-  /*XXXX021 This is a pretty brute approach. It'd be better to
+  /*XXXX This is a pretty brute approach. It'd be better to
    * attach only the connections that are waiting on this circuit, rather
-   * than trying to attach them all. */
+   * than trying to attach them all. See bug 743. */
   /* If we already have the introduction circuit built, make sure we send
    * the INTRODUCE cell _now_ */
   connection_ap_attach_pending();
@@ -669,9 +669,9 @@ rend_client_receive_rendezvous(origin_circuit_t *circ, const char *request,
 
   onion_append_to_cpath(&circ->cpath, hop);
   circ->build_state->pending_final_cpath = NULL; /* prevent double-free */
-  /*XXXX021 This is a pretty brute approach. It'd be better to
+  /*XXXX This is a pretty brute approach. It'd be better to
    * attach only the connections that are waiting on this circuit, rather
-   * than trying to attach them all. */
+   * than trying to attach them all. See bug 743.*/
   /* */
   connection_ap_attach_pending();
   return 0;