[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] r10227: point out two remote crash bugs, a memory leak, and a few ot (in tor/trunk/src: common or)



Author: arma
Date: 2007-05-20 10:15:23 -0400 (Sun, 20 May 2007)
New Revision: 10227

Modified:
   tor/trunk/src/common/mempool.c
   tor/trunk/src/or/config.c
   tor/trunk/src/or/dns.c
   tor/trunk/src/or/main.c
   tor/trunk/src/or/routerlist.c
   tor/trunk/src/or/routerparse.c
Log:
point out two remote crash bugs, a memory leak, and a few other
items we should probably look into.


Modified: tor/trunk/src/common/mempool.c
===================================================================
--- tor/trunk/src/common/mempool.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/common/mempool.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -388,6 +388,8 @@
     n = pool->min_empty_chunks + (-n);
     if (n < pool->n_empty_chunks)
       pool->min_empty_chunks = n;
+    /* XXX020 don't we want some sort of return here, given the
+     * assert that follows? -RD */
   }
   ASSERT(n>=0);
 

Modified: tor/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/or/config.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -3119,6 +3119,8 @@
 
   if (argc > 1 && (!strcmp(argv[1],"--version"))) {
     char vbuf[128];
+    // XXX020 below, tor_svn_revision will always be defined, right?
+    // So we can take out that check? Also in router.c. -RD
     if (tor_svn_revision != NULL && strlen(tor_svn_revision)) {
       tor_snprintf(vbuf, sizeof(vbuf), " (r%s)", tor_svn_revision);
     } else {

Modified: tor/trunk/src/or/dns.c
===================================================================
--- tor/trunk/src/or/dns.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/or/dns.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -559,8 +559,10 @@
       }
       //circuit_detach_stream(TO_CIRCUIT(oncirc), exitconn);
       exitconn->on_circuit = NULL;
-      if (!exitconn->_base.marked_for_close)
+      if (!exitconn->_base.marked_for_close) {
         connection_free(TO_CONN(exitconn));
+        //XXX020 ... and we just leak exitconn otherwise? -RD
+      }
       break;
     default:
       tor_assert(0);

Modified: tor/trunk/src/or/main.c
===================================================================
--- tor/trunk/src/or/main.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/or/main.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -68,7 +68,7 @@
 static int signewnym_is_pending = 0;
 
 /** Array of all open connections.  The first n_conns elements are valid. */
-/*XXXX020 Should we just use a smartlist here? */
+/*XXXX020 Should we just use a smartlist here? -NM Sure. -RD */
 static connection_t *connection_array[MAXCONNECTIONS+1] =
         { NULL };
 /** List of connections that have been marked for close and need to be freed

Modified: tor/trunk/src/or/routerlist.c
===================================================================
--- tor/trunk/src/or/routerlist.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/or/routerlist.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -4857,6 +4857,8 @@
   if (ei->bad_sig)
     return 1;
 
+  /* XXX020 below we should explain why this is strcmp and not strcasecmp,
+   * since it differs from how we usually compare nicknames. -RD */
   if (strcmp(ri->nickname, ei->nickname) ||
       memcmp(ri->cache_info.identity_digest, ei->cache_info.identity_digest,
              DIGEST_LEN))

Modified: tor/trunk/src/or/routerparse.c
===================================================================
--- tor/trunk/src/or/routerparse.c	2007-05-20 05:15:53 UTC (rev 10226)
+++ tor/trunk/src/or/routerparse.c	2007-05-20 14:15:23 UTC (rev 10227)
@@ -2142,7 +2142,8 @@
   for (i = 0; table[i].t; ++i) {
     if (counts[table[i].v] < table[i].min_cnt) {
       log_warn(LD_DIR, "Parse error: missing %s element.", table[i].t);
-      tor_assert(0);
+      tor_assert(0); /* XXX020 is this assert a remote crash waiting to
+                      * happen? -RD */
       return -1;
     }
     if (counts[table[i].v] > table[i].max_cnt) {