r12330@catbus: nickm | 2007-04-09 19:15:42 -0400

 Split type of "packed cell" from "parsed cell"; pack cells before queueing them on circuits.  This will help us avoid dumb errors when we confuse the two types.


svn:r9935

3 files changed, 38 insertions, 29 deletions

diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 7a3c6f2918..202f62b742 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -124,9 +124,10 @@ connection_or_set_identity_digest(or_connection_t *conn, const char *digest)
  * in the buffer <b>dest</b>. See tor-spec.txt for details about the
  * wire format.
  */
-static void
-cell_pack(char *dest, const cell_t *src)
+void
+cell_pack(packed_cell_t *dst, const cell_t *src)
 {
+  char *dest = dst->body;
   *(uint16_t*)dest    = htons(src->circ_id);
   *(uint8_t*)(dest+2) = src->command;
   memcpy(dest+3, src->payload, CELL_PAYLOAD_SIZE);
@@ -738,15 +739,14 @@ connection_tls_finish_handshake(or_connection_t *conn)
 void
 connection_or_write_cell_to_buf(const cell_t *cell, or_connection_t *conn)
 {
-  char networkcell[CELL_NETWORK_SIZE];
-  char *n = networkcell;
+  packed_cell_t networkcell;
 
   tor_assert(cell);
   tor_assert(conn);
 
-  cell_pack(n, cell);
+  cell_pack(&networkcell, cell);
 
-  connection_write_to_buf(n, CELL_NETWORK_SIZE, TO_CONN(conn));
+  connection_write_to_buf(networkcell.body, CELL_NETWORK_SIZE, TO_CONN(conn));
 }
 
 /** Process cells from <b>conn</b>'s inbuf.
diff --git a/src/or/or.h b/src/or/or.h
index 590c255408..7345d27ba0 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -678,18 +678,24 @@ typedef struct cell_t cell_t;
 /** Parsed onion routing cell.  All communication between nodes
  * is via cells. */
 struct cell_t {
-  struct cell_t *next; /**< Next cell queued on a this circuit. */
   uint16_t circ_id; /**< Circuit which received the cell. */
   uint8_t command; /**< Type of the cell: one of PADDING, CREATE, RELAY,
                     * or DESTROY. */
   char payload[CELL_PAYLOAD_SIZE]; /**< Cell body. */
 };
 
+typedef struct packed_cell_t packed_cell_t;
+/** A cell as packed for writing to the network. */
+struct packed_cell_t {
+  struct packed_cell_t *next; /**< Next cell queued on a this circuit. */
+  char body[CELL_NETWORK_SIZE]; /**< Cell as packed for network. */
+};
+
 /** A queue of cells on a circuit, waiting to be added to the
  * or_connection_t's outbuf. */
 typedef struct cell_queue_t {
-  cell_t *head; /**< The first cell, or NULL if the queue is empty */
-  cell_t *tail; /**< The last cell, or NULL if the queue is empty */
+  packed_cell_t *head; /**< The first cell, or NULL if the queue is empty */
+  packed_cell_t *tail; /**< The last cell, or NULL if the queue is empty */
   int n; /**< The number of cells in the queue */
 } cell_queue_t;
 
@@ -2299,6 +2305,8 @@ void connection_or_write_cell_to_buf(const cell_t *cell,
 int connection_or_send_destroy(uint16_t circ_id, or_connection_t *conn,
                                int reason);
 
+void cell_pack(packed_cell_t *dest, const cell_t *src);
+
 /********************************* control.c ***************************/
 
 typedef enum circuit_status_event_t {
@@ -2656,8 +2664,8 @@ extern uint64_t stats_n_data_cells_received;
 extern uint64_t stats_n_data_bytes_received;
 
 void cell_queue_clear(cell_queue_t *queue);
-void cell_queue_append(cell_queue_t *queue, cell_t *cell);
-void cell_queue_append_copy(cell_queue_t *queue, const cell_t *cell);
+void cell_queue_append(cell_queue_t *queue, packed_cell_t *cell);
+void cell_queue_append_packed_copy(cell_queue_t *queue, const cell_t *cell);
 
 void append_cell_to_circuit_queue(circuit_t *circ, or_connection_t *orconn,
                                   cell_t *cell, int direction);
diff --git a/src/or/relay.c b/src/or/relay.c
index 7d853124a6..2b8fe8d2e8 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -323,7 +323,7 @@ relay_crypt(circuit_t *circ, cell_t *cell, int cell_direction,
 
 /** Package a relay cell from an edge:
  *  - Encrypt it to the right layer
- *  - Append it to the appropriate cell_queue on <b>circ</b>
+ *  - Append it to the appropriate cell_queue on <b>circ</b>.
  */
 static int
 circuit_package_relay_cell(cell_t *cell, circuit_t *circ,
@@ -1479,24 +1479,24 @@ circuit_consider_sending_sendme(circuit_t *circ, crypt_path_t *layer_hint)
 
 /** Release storage held by <b>cell</b> */
 static INLINE void
-cell_free(cell_t *cell)
+packed_cell_free(packed_cell_t *cell)
 {
   tor_free(cell);
 }
 
-/** Allocate a new copy of <b>cell</b>. */
-static INLINE cell_t *
-cell_copy(const cell_t *cell)
+/** Allocate a new copy of packed <b>cell</b>. */
+static INLINE packed_cell_t *
+packed_cell_copy(const cell_t *cell)
 {
-  cell_t *c = tor_malloc(sizeof(cell_t));
-  memcpy(c, cell, sizeof(cell_t));
+  packed_cell_t *c = tor_malloc(sizeof(packed_cell_t));
+  cell_pack(c, cell);
   c->next = NULL;
   return c;
 }
 
 /** Append <b>cell</b> to the end of <b>queue</b>. */
 void
-cell_queue_append(cell_queue_t *queue, cell_t *cell)
+cell_queue_append(cell_queue_t *queue, packed_cell_t *cell)
 {
   if (queue->tail) {
     tor_assert(!queue->tail->next);
@@ -1511,20 +1511,20 @@ cell_queue_append(cell_queue_t *queue, cell_t *cell)
 
 /** Append a newly allocated copy of <b>cell</b> to the end of <b>queue</b> */
 void
-cell_queue_append_copy(cell_queue_t *queue, const cell_t *cell)
+cell_queue_append_packed_copy(cell_queue_t *queue, const cell_t *cell)
 {
-  cell_queue_append(queue, cell_copy(cell));
+  cell_queue_append(queue, packed_cell_copy(cell));
 }
 
 /** Remove and free every cell in <b>queue</b>. */
 void
 cell_queue_clear(cell_queue_t *queue)
 {
-  cell_t *cell, *next;
+  packed_cell_t *cell, *next;
   cell = queue->head;
   while (cell) {
     next = cell->next;
-    cell_free(cell);
+    packed_cell_free(cell);
     cell = next;
   }
   queue->head = queue->tail = NULL;
@@ -1533,10 +1533,10 @@ cell_queue_clear(cell_queue_t *queue)
 
 /** Extract and return the cell at the head of <b>queue</b>; return NULL if
  * <b>queue</b> is empty. */
-static INLINE cell_t *
+static INLINE packed_cell_t *
 cell_queue_pop(cell_queue_t *queue)
 {
-  cell_t *cell = queue->head;
+  packed_cell_t *cell = queue->head;
   if (!cell)
     return NULL;
   queue->head = cell->next;
@@ -1702,11 +1702,12 @@ connection_or_flush_from_first_active_circuit(or_connection_t *conn, int max)
   tor_assert(*next_circ_on_conn_p(circ,conn));
 
   for (n_flushed = 0; n_flushed < max && queue->head; ++n_flushed) {
-    cell_t *cell = cell_queue_pop(queue);
+    packed_cell_t *cell = cell_queue_pop(queue);
     tor_assert(*next_circ_on_conn_p(circ,conn));
 
-    connection_or_write_cell_to_buf(cell, conn);
-    cell_free(cell);
+    connection_write_to_buf(cell->body, CELL_NETWORK_SIZE, TO_CONN(conn));
+
+    packed_cell_free(cell);
     ++n_flushed;
     if (circ != conn->active_circuits) {
       /* If this happens, the current circuit just got made inactive by
@@ -1752,7 +1753,7 @@ append_cell_to_circuit_queue(circuit_t *circ, or_connection_t *orconn,
     streams_blocked = circ->streams_blocked_on_p_conn;
   }
 
-  cell_queue_append_copy(queue, cell);
+  cell_queue_append_packed_copy(queue, cell);
 
   /* If we have too many cells on the circuit, we should stop reading from
    * the edge streams for a while. */