[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[or-cvs] r10315: Review XXXX comments without a version; upgrade some to XXXX (in tor/trunk: . src/common src/or)
Author: nickm
Date: 2007-05-24 14:12:52 -0400 (Thu, 24 May 2007)
New Revision: 10315
Modified:
tor/trunk/
tor/trunk/src/common/tortls.c
tor/trunk/src/or/connection_edge.c
tor/trunk/src/or/control.c
tor/trunk/src/or/directory.c
tor/trunk/src/or/dns.c
tor/trunk/src/or/main.c
tor/trunk/src/or/relay.c
tor/trunk/src/or/routerlist.c
Log:
r12936@catbus: nickm | 2007-05-24 14:12:34 -0400
Review XXXX comments without a version; upgrade some to XXXX020.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r12936] on 8246c3cf-6607-4228-993b-4d95d33730f1
Modified: tor/trunk/src/common/tortls.c
===================================================================
--- tor/trunk/src/common/tortls.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/common/tortls.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -183,7 +183,7 @@
return _TOR_TLS_ZERORETURN;
log(severity, LD_NET, "TLS error: Zero return");
tls_log_errors(severity, doing);
- /* XXXX Actually, a 'zero return' error has a pretty specific meaning:
+ /* XXXX020 Actually, a 'zero return' error has a pretty specific meaning:
* the connection has been closed cleanly. */
return TOR_TLS_ERROR_MISC;
default:
Modified: tor/trunk/src/or/connection_edge.c
===================================================================
--- tor/trunk/src/or/connection_edge.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/connection_edge.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -1884,7 +1884,7 @@
digest, DIGEST_LEN);
}
- conn->_base.address = tor_strdup("(local bridge)"); /*XXXX020 no "bridge"*/
+ conn->_base.address = tor_strdup("(local link)");
conn->_base.addr = 0;
conn->_base.port = 0;
@@ -2445,7 +2445,7 @@
connection_edge_is_rendezvous_stream(edge_connection_t *conn)
{
tor_assert(conn);
- if (*conn->rend_query) /* XXX */
+ if (*conn->rend_query) /* XXX */ /* XXXX Why is this XXX? -NM */
return 1;
return 0;
}
Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/control.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -876,7 +876,9 @@
else if (!strcasecmp(ev, "GUARD"))
event_code = EVENT_GUARD;
else if (!strcasecmp(ev, "GUARDS")) {
- /* XXX tolerate buggy spec in 0.1.2.5-alpha through 0.1.2.10-rc */
+ /* XXXX021 This check is here to tolerate the controllers that
+ * depended on the buggy spec in 0.1.2.5-alpha through 0.1.2.10-rc.
+ * Once those versions are obsolete, stop supporting this. */
log_warn(LD_CONTROL, "Controller used obsolete 'GUARDS' event name; "
"use GUARD instead.");
event_code = EVENT_GUARD;
@@ -3083,7 +3085,7 @@
smartlist_add(strs, s);
});
smartlist_add(strs, tor_strdup("\r\n.\r\n"));
- /* XXX the above strdup has an extra \r\n in it, resulting in
+ /* XXX020 the above strdup has an extra \r\n in it, resulting in
* a blank line in the NS output. Can we remove it, or is that
* bad since the output of networkstatus_getinfo_helper_single()
* only adds \n, not \r\n? */
Modified: tor/trunk/src/or/directory.c
===================================================================
--- tor/trunk/src/or/directory.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/directory.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -1140,7 +1140,7 @@
log_info(LD_DIR,"Received networkstatus objects (size %d) from server "
"'%s:%d'",(int) body_len, conn->_base.address, conn->_base.port);
if (status_code != 200) {
- /* XXXX This warning tends to freak out clients who get a 403. */
+ /* XXXX020 This warning tends to freak out clients who get a 403. */
log_warn(LD_DIR,
"Received http status code %d (%s) from server "
"'%s:%d' while fetching \"/tor/status/%s\". I'll try again soon.",
@@ -1223,7 +1223,7 @@
(status_code == 400 && !strcmp(reason, "Servers unavailable."));
/* 404 means that it didn't have them; no big deal.
* Older (pre-0.1.1.8) servers said 400 Servers unavailable instead. */
- /* XXXX This warning tends to freak out clients who get a 403. */
+ /* XXXX020 This warning tends to freak out clients who get a 403. */
log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR,
"Received http status code %d (%s) from server '%s:%d' "
"while fetching \"/tor/server/%s\". I'll try again soon.",
Modified: tor/trunk/src/or/dns.c
===================================================================
--- tor/trunk/src/or/dns.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/dns.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -565,6 +565,8 @@
// If it's marked for close, it's on closeable_connection_lst in
// main.c. If it's on the closeable list, it will get freed from
// main.c. -NM
+ // "<armadev> If that's true, there are other bugs arond, where we
+ // don't check if it's marked, and will end up double-freeing."
}
break;
default:
@@ -814,6 +816,7 @@
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));
@@ -940,7 +943,7 @@
assert_resolve_ok(resolve);
if (resolve->state != CACHE_STATE_PENDING) {
- /* XXXX Maybe update addr? or check addr for consistency? Or let
+ /* XXXX020 Maybe update addr? or check addr for consistency? Or let
* VALID replace FAILED? */
int is_test_addr = is_test_address(address);
if (!is_test_addr)
Modified: tor/trunk/src/or/main.c
===================================================================
--- tor/trunk/src/or/main.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/main.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -100,7 +100,7 @@
SERVICE_STATUS service_status;
SERVICE_STATUS_HANDLE hStatus;
-/* XXXX This 'backup argv' and 'backup argc' business is an ugly hack. This
+/* XXXX020 This 'backup argv' and 'backup argc' business is an ugly hack. This
* is a job for arguments, not globals. */
static char **backup_argv;
static int backup_argc;
@@ -510,7 +510,7 @@
edge_conn->end_reason = END_STREAM_REASON_INTERNAL;
conn->edge_has_sent_end = 1;
}
- /* XXX do we need a close-immediate here, so we don't try to flush? */
+ /* XXX020 do we need a close-immediate here, so we don't try to flush? */
connection_mark_for_close(conn);
}
}
@@ -583,7 +583,7 @@
LOG_FN_CONN(conn, (LOG_INFO,LD_NET,
"Holding conn (fd %d) open for more flushing.",
conn->s));
- /* XXX should we reset timestamp_lastwritten here? */
+ /* XXX020 should we reset timestamp_lastwritten here? */
return 0;
}
if (connection_wants_to_flush(conn)) {
@@ -902,7 +902,7 @@
if (time_to_fetch_directory < now) {
/* Only caches actually need to fetch directories now. */
if (options->DirPort && !authdir_mode_v1(options)) {
- /* XXX actually, we should only do this if we want to advertise
+ /* XXX020 actually, we should only do this if we want to advertise
* our dirport. not simply if we configured one. -RD */
if (any_trusted_dir_is_v1_authority())
directory_get_from_dirserver(DIR_PURPOSE_FETCH_DIR, NULL, 1);
@@ -1380,7 +1380,7 @@
/* refilling buckets and sending cells happens at the beginning of the
* next iteration of the loop, inside prepare_for_poll()
- * XXXX No longer so.
+ * XXXX020 No longer so; fix comment.
*/
}
}
@@ -1483,7 +1483,7 @@
"Rate limiting NEWNYM request: delaying by %d second(s)",
(int)(MAX_SIGNEWNYM_RATE+time_of_last_signewnym-now));
} else {
- /* XXX refactor someday: these two calls are in
+ /* 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();
@@ -1775,7 +1775,10 @@
or_state_mark_dirty(get_or_state(), 0); /* force an immediate save. */
or_state_save(time(NULL));
}
- tor_free_all(0); /* move tor_free_all back into the ifdef below later. XXX*/
+ tor_free_all(0); /* We could move tor_free_all back into the ifdef below
+ later, if it makes shutdown unacceptably slow. But for
+ now, leave it here: it's helped us catch bugs in the
+ past. */
crypto_global_cleanup();
#ifdef USE_DMALLOC
dmalloc_log_unfreed();
Modified: tor/trunk/src/or/relay.c
===================================================================
--- tor/trunk/src/or/relay.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/relay.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -358,7 +358,8 @@
} else { /* incoming cell */
or_circuit_t *or_circ;
if (CIRCUIT_IS_ORIGIN(circ)) {
- /* XXXX RD This is a bug, right? */
+ /* We should never package an _incoming_ cell from the circuit
+ * origin; that means we messed up somewhere. */
log_warn(LD_BUG,"incoming relay cell at origin circuit. Dropping.");
assert_circuit_ok(circ);
return 0; /* just drop it */
Modified: tor/trunk/src/or/routerlist.c
===================================================================
--- tor/trunk/src/or/routerlist.c 2007-05-24 18:12:44 UTC (rev 10314)
+++ tor/trunk/src/or/routerlist.c 2007-05-24 18:12:52 UTC (rev 10315)
@@ -1467,7 +1467,7 @@
if (router_hex_digest_matches(router, nickname))
return router;
else
- best_match = router; // XXXX NM not exactly right.
+ best_match = router; // XXXX020 NM not exactly right.
}
});