commit ec4142abdf7fa4efaa281413e31acc93bf22ea4a Author: Nick Mathewson nickm@torproject.org Date: Wed Oct 19 17:07:23 2016 -0400

Unify code in channel_write_*cell()

Patch from pingl; patch for 13827. --- changes/bug13827 | 3 ++ src/or/channel.c | 93 +++++++++++++++++++------------------------------------- src/or/channel.h | 3 ++ 3 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/changes/bug13827 b/changes/bug13827 new file mode 100644 index 0000000..2235a3f --- /dev/null +++ b/changes/bug13827 @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Remove duplicate code in the channel_write_*cell() functions. + Closes ticket 13827; patch from Pingl. diff --git a/src/or/channel.c b/src/or/channel.c index f547aea..75aab3c 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -1838,45 +1838,58 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q) } }

-/** - * Write a cell to a channel +/** Write a generic cell type to a channel * - * Write a fixed-length cell to a channel using the write_cell() method. - * This is equivalent to the pre-channels connection_or_write_cell_to_buf(); - * it is called by the transport-independent code to deliver a cell to a - * channel for transmission. + * Write a generic cell to a channel. It is called by channel_write_cell(), + * channel_write_var_cell() and channel_write_packed_cell() in order to reduce + * code duplication. Notice that it takes cell as pointer of type void, + * this can be dangerous because no type check is performed. */

void -channel_write_cell(channel_t *chan, cell_t *cell) +channel_write_cell_generic_(channel_t *chan, const char *cell_type, + void *cell, cell_queue_entry_t *q) { - cell_queue_entry_t q;

tor_assert(chan); tor_assert(cell);

if (CHANNEL_IS_CLOSING(chan)) { - log_debug(LD_CHANNEL, "Discarding cell_t %p on closing channel %p with " - "global ID "U64_FORMAT, cell, chan, + log_debug(LD_CHANNEL, "Discarding %c %p on closing channel %p with " + "global ID "U64_FORMAT, *cell_type, cell, chan, U64_PRINTF_ARG(chan->global_identifier)); tor_free(cell); return; } - log_debug(LD_CHANNEL, - "Writing cell_t %p to channel %p with global ID " - U64_FORMAT, + "Writing %c %p to channel %p with global ID " + U64_FORMAT, *cell_type, cell, chan, U64_PRINTF_ARG(chan->global_identifier));

- q.type = CELL_QUEUE_FIXED; - q.u.fixed.cell = cell; - channel_write_cell_queue_entry(chan, &q); - + channel_write_cell_queue_entry(chan, q); /* Update the queue size estimate */ channel_update_xmit_queue_size(chan); }

/** + * Write a cell to a channel + * + * Write a fixed-length cell to a channel using the write_cell() method. + * This is equivalent to the pre-channels connection_or_write_cell_to_buf(); + * it is called by the transport-independent code to deliver a cell to a + * channel for transmission. + */ + +void +channel_write_cell(channel_t *chan, cell_t *cell) +{ + cell_queue_entry_t q; + q.type = CELL_QUEUE_FIXED; + q.u.fixed.cell = cell; + channel_write_cell_generic_(chan, "cell_t", cell, &q); +} + +/** * Write a packed cell to a channel * * Write a packed cell to a channel using the write_cell() method. This is @@ -1888,30 +1901,9 @@ void channel_write_packed_cell(channel_t *chan, packed_cell_t *packed_cell) { cell_queue_entry_t q; - - tor_assert(chan); - tor_assert(packed_cell); - - if (CHANNEL_IS_CLOSING(chan)) { - log_debug(LD_CHANNEL, "Discarding packed_cell_t %p on closing channel %p " - "with global ID "U64_FORMAT, packed_cell, chan, - U64_PRINTF_ARG(chan->global_identifier)); - packed_cell_free(packed_cell); - return; - } - - log_debug(LD_CHANNEL, - "Writing packed_cell_t %p to channel %p with global ID " - U64_FORMAT, - packed_cell, chan, - U64_PRINTF_ARG(chan->global_identifier)); - q.type = CELL_QUEUE_PACKED; q.u.packed.packed_cell = packed_cell; - channel_write_cell_queue_entry(chan, &q); - - /* Update the queue size estimate */ - channel_update_xmit_queue_size(chan); + channel_write_cell_generic_(chan, "packed_cell_t", packed_cell, &q); }

/** @@ -1927,30 +1919,9 @@ void channel_write_var_cell(channel_t *chan, var_cell_t *var_cell) { cell_queue_entry_t q; - - tor_assert(chan); - tor_assert(var_cell); - - if (CHANNEL_IS_CLOSING(chan)) { - log_debug(LD_CHANNEL, "Discarding var_cell_t %p on closing channel %p " - "with global ID "U64_FORMAT, var_cell, chan, - U64_PRINTF_ARG(chan->global_identifier)); - var_cell_free(var_cell); - return; - } - - log_debug(LD_CHANNEL, - "Writing var_cell_t %p to channel %p with global ID " - U64_FORMAT, - var_cell, chan, - U64_PRINTF_ARG(chan->global_identifier)); - q.type = CELL_QUEUE_VAR; q.u.var.var_cell = var_cell; - channel_write_cell_queue_entry(chan, &q); - - /* Update the queue size estimate */ - channel_update_xmit_queue_size(chan); + channel_write_cell_generic_(chan, "var_cell_t", var_cell, &q); }

/** diff --git a/src/or/channel.h b/src/or/channel.h index a711b56..44a3901 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -382,6 +382,9 @@ struct cell_queue_entry_s { STATIC int chan_cell_queue_len(const chan_cell_queue_t *queue);

STATIC void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off); + +void channel_write_cell_generic_(channel_t *chan, const char *cell_type, + void *cell, cell_queue_entry_t *q); #endif

/* Channel operations for subclasses and internal use only */