diff options
| -rw-r--r-- | src/or/channel.c | 24 | ||||
| -rw-r--r-- | src/or/channeltls.c | 11 | ||||
| -rw-r--r-- | src/or/relay.c | 24 | ||||
| -rw-r--r-- | src/test/test_channel.c | 2 |
4 files changed, 32 insertions, 29 deletions
diff --git a/src/or/channel.c b/src/or/channel.c index 94d7af47b5..9e79fc1a3f 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -1431,8 +1431,11 @@ channel_clear_remote_end(channel_t *chan) /** * Write to a channel the given packed cell. * - * Return 0 on success and the cell is freed so the caller MUST discard any - * reference to it. On error, -1 is returned and the cell is untouched. + * Return 0 on success or -1 on error. + * + * Two possible errors can happen. Either the channel is not opened or the + * lower layer (specialized channel) failed to write it. In both cases, it is + * the caller responsability to free the cell. */ static int write_packed_cell(channel_t *chan, packed_cell_t *cell) @@ -1483,10 +1486,15 @@ write_packed_cell(channel_t *chan, packed_cell_t *cell) * Write a packed cell to a channel using the write_cell() method. This is * called by the transport-independent code to deliver a packed cell to a * channel for transmission. + * + * Return 0 on success else a negative value. In both cases, the caller should + * not access the cell anymore, it is freed both on success and error. */ int channel_write_packed_cell(channel_t *chan, packed_cell_t *cell) { + int ret = -1; + tor_assert(chan); tor_assert(cell); @@ -1494,14 +1502,20 @@ channel_write_packed_cell(channel_t *chan, packed_cell_t *cell) log_debug(LD_CHANNEL, "Discarding %p on closing channel %p with " "global ID "U64_FORMAT, cell, chan, U64_PRINTF_ARG(chan->global_identifier)); - tor_free(cell); - return -1; + goto end; } log_debug(LD_CHANNEL, "Writing %p to channel %p with global ID " U64_FORMAT, cell, chan, U64_PRINTF_ARG(chan->global_identifier)); - return write_packed_cell(chan, cell); + ret = write_packed_cell(chan, cell); + + end: + /* Whatever happens, we free the cell. Either an error occured or the cell + * was put on the connection outbuf, both cases we have ownership of the + * cell and we free it. */ + packed_cell_free(cell); + return ret; } /** diff --git a/src/or/channeltls.c b/src/or/channeltls.c index e6ecc15381..023ccdefd3 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -832,6 +832,9 @@ channel_tls_write_cell_method(channel_t *chan, cell_t *cell) * * This implements the write_packed_cell method for channel_tls_t; given a * channel_tls_t and a packed_cell_t, transmit the packed_cell_t. + * + * Return 0 on success or negative value on error. The caller must free the + * packed cell. */ static int @@ -841,7 +844,6 @@ channel_tls_write_packed_cell_method(channel_t *chan, tor_assert(chan); channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); size_t cell_network_size = get_cell_network_size(chan->wide_circ_ids); - int written = 0; tor_assert(tlschan); tor_assert(packed_cell); @@ -849,18 +851,15 @@ channel_tls_write_packed_cell_method(channel_t *chan, if (tlschan->conn) { connection_buf_add(packed_cell->body, cell_network_size, TO_CONN(tlschan->conn)); - - /* This is where the cell is finished; used to be done from relay.c */ - packed_cell_free(packed_cell); - ++written; } else { log_info(LD_CHANNEL, "something called write_packed_cell on a tlschan " "(%p with ID " U64_FORMAT " but no conn", chan, U64_PRINTF_ARG(chan->global_identifier)); + return -1; } - return written; + return 0; } /** diff --git a/src/or/relay.c b/src/or/relay.c index f36fc78cda..ac91032844 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2521,17 +2521,6 @@ cell_queue_pop(cell_queue_t *queue) return cell; } -/** Insert <b>cell</b> as the head of the <b>queue</b>. */ -static void -cell_insert_head(cell_queue_t *queue, packed_cell_t *cell) -{ - tor_assert(queue); - tor_assert(cell); - - TOR_SIMPLEQ_INSERT_HEAD(&queue->head, cell, next); - ++queue->n; -} - /** Return the total number of bytes used for each packed_cell in a queue. * Approximate. */ size_t @@ -2757,8 +2746,11 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) /* this code is duplicated from some of the logic below. Ugly! XXXX */ tor_assert(destroy_queue->n > 0); cell = cell_queue_pop(destroy_queue); + /* Send the DESTROY cell. It is very unlikely that this fails but just + * in case, get rid of the channel. */ if (channel_write_packed_cell(chan, cell) < 0) { - cell_insert_head(destroy_queue, cell); + /* The cell has been freed. */ + channel_mark_for_close(chan); continue; } /* Update the cmux destroy counter */ @@ -2836,11 +2828,11 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) DIRREQ_TUNNELED, DIRREQ_CIRC_QUEUE_FLUSHED); - /* Now send the cell */ + /* Now send the cell. It is very unlikely that this fails but just in + * case, get rid of the channel. */ if (channel_write_packed_cell(chan, cell) < 0) { - /* Unable to send the cell, put it back at the start of the circuit - * queue so we can retry. */ - cell_insert_head(queue, cell); + /* The cell has been freed at this point. */ + channel_mark_for_close(chan); continue; } cell = NULL; diff --git a/src/test/test_channel.c b/src/test/test_channel.c index 88a13862bd..c3b99d87ac 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -199,7 +199,6 @@ chan_test_write_packed_cell(channel_t *ch, if (test_chan_accept_cells) { /* Free the cell and bump the counter */ - packed_cell_free(packed_cell); ++test_cells_written; rv = 1; } @@ -846,7 +845,6 @@ test_channel_lifecycle(void *arg) done: free_fake_channel(ch1); free_fake_channel(ch2); - tor_free(p_cell); UNMOCK(scheduler_channel_doesnt_want_writes); UNMOCK(scheduler_release_channel); |