Refactor use of connection_new so that we get more verifiable typesafety.

svn:r16785

7 files changed, 92 insertions, 39 deletions

diff --git a/ChangeLog b/ChangeLog
index fabbbae104..77170aadb8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -13,6 +13,10 @@ Changes in version 0.2.1.6-alpha - 2008-09-xx
     - Use a lockfile to make sure that two Tor processes are not
       simultaneously running with the same datadir.
 
+  o Code simplifications and refactoring:
+    - Revise the connection_new functions so that a more typesafe variant
+      exists.  This will lower false positives from some scanning tools.
+
 
 Changes in version 0.2.1.5-alpha - 2008-08-31
   o Major features:
diff --git a/src/or/connection.c b/src/or/connection.c
index b96ac15d29..7365e77c7c 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -18,6 +18,8 @@ const char connection_c_id[] =
 static connection_t *connection_create_listener(
                                struct sockaddr *listensockaddr, int type,
                                char* address);
+static void connection_init(time_t now, connection_t *conn, int type,
+                            int socket_family);
 static int connection_init_accepted_conn(connection_t *conn,
                                          uint8_t listener_type);
 static int connection_handle_listener_read(connection_t *conn, int new_type);
@@ -150,8 +152,72 @@ conn_state_to_string(int type, int state)
   return buf;
 }
 
-/** Allocate space for a new connection_t. This function just initializes
- * conn; you must call connection_add() to link it into the main array.
+dir_connection_t *
+dir_connection_new(int socket_family)
+{
+  dir_connection_t *dir_conn = tor_malloc_zero(sizeof(dir_connection_t));
+  connection_init(time(NULL), TO_CONN(dir_conn), CONN_TYPE_DIR, socket_family);
+  return dir_conn;
+}
+or_connection_t *
+or_connection_new(int socket_family)
+{
+  or_connection_t *or_conn = tor_malloc_zero(sizeof(or_connection_t));
+  time_t now = time(NULL);
+  connection_init(now, TO_CONN(or_conn), CONN_TYPE_OR, socket_family);
+
+  or_conn->timestamp_last_added_nonpadding = time(NULL);
+  or_conn->next_circ_id = crypto_rand_int(1<<15);
+
+  return or_conn;
+}
+edge_connection_t *
+edge_connection_new(int type, int socket_family)
+{
+  edge_connection_t *edge_conn = tor_malloc_zero(sizeof(edge_connection_t));
+  tor_assert(type == CONN_TYPE_EXIT || type == CONN_TYPE_AP);
+  connection_init(time(NULL), TO_CONN(edge_conn), type, socket_family);
+  if (type == CONN_TYPE_AP)
+    edge_conn->socks_request = tor_malloc_zero(sizeof(socks_request_t));
+  return edge_conn;
+}
+control_connection_t *
+control_connection_new(int socket_family)
+{
+  control_connection_t *control_conn =
+    tor_malloc_zero(sizeof(control_connection_t));
+  connection_init(time(NULL),
+                  TO_CONN(control_conn), CONN_TYPE_CONTROL, socket_family);
+  return control_conn;
+}
+
+connection_t *
+connection_new(int type, int socket_family)
+{
+  switch (type) {
+    case CONN_TYPE_OR:
+      return TO_CONN(or_connection_new(socket_family));
+
+    case CONN_TYPE_EXIT:
+    case CONN_TYPE_AP:
+      return TO_CONN(edge_connection_new(type, socket_family));
+
+    case CONN_TYPE_DIR:
+      return TO_CONN(dir_connection_new(socket_family));
+
+    case CONN_TYPE_CONTROL:
+      return TO_CONN(control_connection_new(socket_family));
+
+    default: {
+      connection_t *conn = tor_malloc_zero(sizeof(connection_t));
+      connection_init(time(NULL), conn, type, socket_family);
+      return conn;
+    }
+  }
+}
+
+/** Initializes conn. (you must call connection_add() to link it into the main
+ * array).
  *
  * Set conn-\>type to <b>type</b>. Set conn-\>s and conn-\>conn_array_index to
  * -1 to signify they are not yet assigned.
@@ -163,42 +229,30 @@ conn_state_to_string(int type, int state)
  *
  * Initialize conn's timestamps to now.
  */
-connection_t *
-connection_new(int type, int socket_family)
+static void
+connection_init(time_t now, connection_t *conn, int type, int socket_family)
 {
   static uint64_t n_connections_allocated = 1;
 
-  connection_t *conn;
-  time_t now = time(NULL);
-  size_t length;
-  uint32_t magic;
-
   switch (type) {
     case CONN_TYPE_OR:
-      length = sizeof(or_connection_t);
-      magic = OR_CONNECTION_MAGIC;
+      conn->magic = OR_CONNECTION_MAGIC;
       break;
     case CONN_TYPE_EXIT:
     case CONN_TYPE_AP:
-      length = sizeof(edge_connection_t);
-      magic = EDGE_CONNECTION_MAGIC;
+      conn->magic = EDGE_CONNECTION_MAGIC;
       break;
     case CONN_TYPE_DIR:
-      length = sizeof(dir_connection_t);
-      magic = DIR_CONNECTION_MAGIC;
+      conn->magic = DIR_CONNECTION_MAGIC;
       break;
     case CONN_TYPE_CONTROL:
-      length = sizeof(control_connection_t);
-      magic = CONTROL_CONNECTION_MAGIC;
+      conn->magic = CONTROL_CONNECTION_MAGIC;
       break;
     default:
-      length = sizeof(connection_t);
-      magic = BASE_CONNECTION_MAGIC;
+      conn->magic = BASE_CONNECTION_MAGIC;
       break;
   }
 
-  conn = tor_malloc_zero(length);
-  conn->magic = magic;
   conn->s = -1; /* give it a default of 'not used' */
   conn->conn_array_index = -1; /* also default to 'not used' */
   conn->global_identifier = n_connections_allocated++;
@@ -209,20 +263,10 @@ connection_new(int type, int socket_family)
     conn->inbuf = buf_new();
     conn->outbuf = buf_new();
   }
-  if (type == CONN_TYPE_AP) {
-    TO_EDGE_CONN(conn)->socks_request =
-      tor_malloc_zero(sizeof(socks_request_t));
-  }
-  if (type == CONN_TYPE_OR) {
-    TO_OR_CONN(conn)->timestamp_last_added_nonpadding = now;
-    TO_OR_CONN(conn)->next_circ_id = crypto_rand_int(1<<15);
-  }
 
   conn->timestamp_created = now;
   conn->timestamp_lastread = now;
   conn->timestamp_lastwritten = now;
-
-  return conn;
 }
 
 /** Create a link between <b>conn_a</b> and <b>conn_b</b>. */
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 2dc083291a..f799090de1 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2167,7 +2167,7 @@ connection_ap_make_link(char *address, uint16_t port,
   log_info(LD_APP,"Making internal %s tunnel to %s:%d ...",
            want_onehop ? "direct" : "anonymized" , safe_str(address),port);
 
-  conn = TO_EDGE_CONN(connection_new(CONN_TYPE_AP, AF_INET));
+  conn = edge_connection_new(CONN_TYPE_AP, AF_INET);
   conn->_base.linked = 1; /* so that we can add it safely below. */
 
   /* populate conn->socks_request */
@@ -2517,7 +2517,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   }
 
   log_debug(LD_EXIT,"Creating new exit connection.");
-  n_stream = TO_EDGE_CONN(connection_new(CONN_TYPE_EXIT, AF_INET));
+  n_stream = edge_connection_new(CONN_TYPE_EXIT, AF_INET);
   n_stream->_base.purpose = EXIT_PURPOSE_CONNECT;
 
   n_stream->stream_id = rh.stream_id;
@@ -2623,7 +2623,7 @@ connection_exit_begin_resolve(cell_t *cell, or_circuit_t *circ)
    * resolved; but if we didn't store them in a connection like this,
    * the housekeeping in dns.c would get way more complicated.)
    */
-  dummy_conn = TO_EDGE_CONN(connection_new(CONN_TYPE_EXIT, AF_INET));
+  dummy_conn = edge_connection_new(CONN_TYPE_EXIT, AF_INET);
   dummy_conn->stream_id = rh.stream_id;
   dummy_conn->_base.address = tor_strndup(cell->payload+RELAY_HEADER_SIZE,
                                           rh.length);
@@ -2765,7 +2765,7 @@ connection_exit_connect_dir(edge_connection_t *exitconn)
 
   exitconn->_base.state = EXIT_CONN_STATE_OPEN;
 
-  dirconn = TO_DIR_CONN(connection_new(CONN_TYPE_DIR, AF_INET));
+  dirconn = dir_connection_new(AF_INET);
 
   dirconn->_base.addr = exitconn->_base.addr;
   dirconn->_base.port = 0;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index f67f4768b3..96b2074c40 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -524,7 +524,7 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
     return NULL;
   }
 
-  conn = TO_OR_CONN(connection_new(CONN_TYPE_OR, AF_INET));
+  conn = or_connection_new(AF_INET);
 
   /* set up conn so it's got all the data we need to remember */
   connection_or_init_conn_from_address(conn, &addr, port, id_digest, 1);
diff --git a/src/or/directory.c b/src/or/directory.c
index 4f4d94af33..22f9678970 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -683,7 +683,7 @@ directory_initiate_command(const char *address, const tor_addr_t *_addr,
 
   log_debug(LD_DIR, "Initiating %s", dir_conn_purpose_to_string(dir_purpose));
 
-  conn = TO_DIR_CONN(connection_new(CONN_TYPE_DIR, AF_INET));
+  conn = dir_connection_new(AF_INET);
 
   /* set up conn so it's got all the data we need to remember */
   tor_addr_copy(&conn->_base.addr, &addr);
diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c
index c65dfe08ca..afd0f28364 100644
--- a/src/or/dnsserv.c
+++ b/src/or/dnsserv.c
@@ -108,7 +108,7 @@ evdns_server_callback(struct evdns_server_request *req, void *_data)
   }
 
   /* Make a new dummy AP connection, and attach the request to it. */
-  conn = TO_EDGE_CONN(connection_new(CONN_TYPE_AP, AF_INET));
+  conn = edge_connection_new(CONN_TYPE_AP, AF_INET);
   conn->_base.state = AP_CONN_STATE_RESOLVE_WAIT;
   conn->is_dns_request = 1;
 
@@ -161,7 +161,7 @@ dnsserv_launch_request(const char *name, int reverse)
   char *q_name;
 
   /* Make a new dummy AP connection, and attach the request to it. */
-  conn = TO_EDGE_CONN(connection_new(CONN_TYPE_AP, AF_INET));
+  conn = edge_connection_new(CONN_TYPE_AP, AF_INET);
   conn->_base.state = AP_CONN_STATE_RESOLVE_WAIT;
 
   if (reverse)
diff --git a/src/or/or.h b/src/or/or.h
index a84bd9b9d6..9e19eaf7cc 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2814,7 +2814,12 @@ or_options_t *options_new(void);
 const char *conn_type_to_string(int type);
 const char *conn_state_to_string(int type, int state);
 
+dir_connection_t *dir_connection_new(int socket_family);
+or_connection_t *or_connection_new(int socket_family);
+edge_connection_t *edge_connection_new(int type, int socket_family);
+control_connection_t *control_connection_new(int socket_family);
 connection_t *connection_new(int type, int socket_family);
+
 void connection_link_connections(connection_t *conn_a, connection_t *conn_b);
 void connection_unregister_events(connection_t *conn);
 void connection_free(connection_t *conn);