commit 59dac38ebe4e6489c2a06d07470023ed5fbdd0aa Author: David Goulet <dgoulet@torproject.org> Date: Tue Jan 7 15:03:24 2020 -0500 chan: Remove dead var cell handler from channel_t The variable lenght cells are handled directly by channel_tls_handle_var_cell() from an OR connection reading its inbuf. The channel var cell handler (agnostic) was never used. Closes #32892 Signed-off-by: David Goulet <dgoulet@torproject.org> --- changes/ticket32892 | 3 +++ src/core/or/channel.c | 25 +------------------------ src/core/or/channel.h | 9 +-------- src/core/or/command.c | 20 +------------------- src/core/or/command.h | 1 - src/test/test_channel.c | 27 ++------------------------- 6 files changed, 8 insertions(+), 77 deletions(-) diff --git a/changes/ticket32892 b/changes/ticket32892 new file mode 100644 index 000000000..f6360d288 --- /dev/null +++ b/changes/ticket32892 @@ -0,0 +1,3 @@ + o Code simplification and refactoring (channel): + - Channel layer had a variable length cell handler that was not used and + thus removed. Closes ticket 32892. diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 2faf63875..eaf2ce311 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -1067,23 +1067,6 @@ channel_get_cell_handler(channel_t *chan) } /** - * Return the variable-length cell handler for a channel. - * - * This function gets the handler for incoming variable-length cells - * installed on a channel. - */ -channel_var_cell_handler_fn_ptr -channel_get_var_cell_handler(channel_t *chan) -{ - tor_assert(chan); - - if (CHANNEL_CAN_HANDLE_CELLS(chan)) - return chan->var_cell_handler; - - return NULL; -} - -/** * Set both cell handlers for a channel. * * This function sets both the fixed-length and variable length cell handlers @@ -1091,9 +1074,7 @@ channel_get_var_cell_handler(channel_t *chan) */ void channel_set_cell_handlers(channel_t *chan, - channel_cell_handler_fn_ptr cell_handler, - channel_var_cell_handler_fn_ptr - var_cell_handler) + channel_cell_handler_fn_ptr cell_handler) { tor_assert(chan); tor_assert(CHANNEL_CAN_HANDLE_CELLS(chan)); @@ -1101,13 +1082,9 @@ channel_set_cell_handlers(channel_t *chan, log_debug(LD_CHANNEL, "Setting cell_handler callback for channel %p to %p", chan, cell_handler); - log_debug(LD_CHANNEL, - "Setting var_cell_handler callback for channel %p to %p", - chan, var_cell_handler); /* Change them */ chan->cell_handler = cell_handler; - chan->var_cell_handler = var_cell_handler; } /* diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 0f6fa228d..6360cf3f4 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -22,7 +22,6 @@ struct tor_timer_t; /* Channel handler function pointer typedefs */ typedef void (*channel_listener_fn_ptr)(channel_listener_t *, channel_t *); typedef void (*channel_cell_handler_fn_ptr)(channel_t *, cell_t *); -typedef void (*channel_var_cell_handler_fn_ptr)(channel_t *, var_cell_t *); /** * This enum is used by channelpadding to decide when to pad channels. @@ -320,7 +319,6 @@ struct channel_t { /** Registered handlers for incoming cells */ channel_cell_handler_fn_ptr cell_handler; - channel_var_cell_handler_fn_ptr var_cell_handler; /* Methods implemented by the lower layer */ @@ -542,13 +540,8 @@ void channel_listener_set_listener_fn(channel_listener_t *chan, /* Incoming cell callbacks */ channel_cell_handler_fn_ptr channel_get_cell_handler(channel_t *chan); -channel_var_cell_handler_fn_ptr -channel_get_var_cell_handler(channel_t *chan); - void channel_set_cell_handlers(channel_t *chan, - channel_cell_handler_fn_ptr cell_handler, - channel_var_cell_handler_fn_ptr - var_cell_handler); + channel_cell_handler_fn_ptr cell_handler); /* Clean up closed channels and channel listeners periodically; these are * called from run_scheduled_events() in main.c. diff --git a/src/core/or/command.c b/src/core/or/command.c index 1c9743776..2f3459f1e 100644 --- a/src/core/or/command.c +++ b/src/core/or/command.c @@ -217,23 +217,6 @@ command_process_cell(channel_t *chan, cell_t *cell) } } -/** Process an incoming var_cell from a channel; in the current protocol all - * the var_cells are handshake-related and handled below the channel layer, - * so this just logs a warning and drops the cell. - */ - -void -command_process_var_cell(channel_t *chan, var_cell_t *var_cell) -{ - tor_assert(chan); - tor_assert(var_cell); - - log_info(LD_PROTOCOL, - "Received unexpected var_cell above the channel layer of type %d" - "; dropping it.", - var_cell->command); -} - /** Process a 'create' <b>cell</b> that just arrived from <b>chan</b>. Make a * new circuit with the p_circ_id specified in cell. Put the circuit in state * onionskin_pending, and pass the onionskin to the cpuworker. Circ will get @@ -685,8 +668,7 @@ command_setup_channel(channel_t *chan) tor_assert(chan); channel_set_cell_handlers(chan, - command_process_cell, - command_process_var_cell); + command_process_cell); } /** Given a listener, install the right handler to process incoming diff --git a/src/core/or/command.h b/src/core/or/command.h index 8c90e1de6..c0af6f26d 100644 --- a/src/core/or/command.h +++ b/src/core/or/command.h @@ -15,7 +15,6 @@ #include "core/or/channel.h" void command_process_cell(channel_t *chan, cell_t *cell); -void command_process_var_cell(channel_t *chan, var_cell_t *cell); void command_setup_channel(channel_t *chan); void command_setup_listener(channel_listener_t *chan_l); diff --git a/src/test/test_channel.c b/src/test/test_channel.c index 035303279..c66170c46 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -34,8 +34,6 @@ static int test_chan_accept_cells = 0; static int test_chan_fixed_cells_recved = 0; static cell_t * test_chan_last_seen_fixed_cell_ptr = NULL; -static int test_chan_var_cells_recved = 0; -static var_cell_t * test_chan_last_seen_var_cell_ptr = NULL; static int test_cells_written = 0; static int test_doesnt_want_writes_count = 0; static int test_dumpstats_calls = 0; @@ -113,24 +111,6 @@ chan_test_dumpstats(channel_t *ch, int severity) return; } -/* - * Handle an incoming variable-size cell for unit tests - */ - -static void -chan_test_var_cell_handler(channel_t *ch, - var_cell_t *var_cell) -{ - tt_assert(ch); - tt_assert(var_cell); - - test_chan_last_seen_var_cell_ptr = var_cell; - ++test_chan_var_cells_recved; - - done: - return; -} - static void chan_test_close(channel_t *ch) { @@ -492,11 +472,8 @@ test_channel_dumpstats(void *arg) /* Receive path */ channel_set_cell_handlers(ch, - chan_test_cell_handler, - chan_test_var_cell_handler); + chan_test_cell_handler); tt_ptr_op(channel_get_cell_handler(ch), OP_EQ, chan_test_cell_handler); - tt_ptr_op(channel_get_var_cell_handler(ch), OP_EQ, - chan_test_var_cell_handler); cell = tor_malloc_zero(sizeof(*cell)); old_count = test_chan_fixed_cells_recved; channel_process_cell(ch, cell); @@ -722,7 +699,7 @@ test_channel_inbound_cell(void *arg) /* Setup incoming cell handlers. We don't care about var cell, the channel * layers is not handling those. */ - channel_set_cell_handlers(chan, chan_test_cell_handler, NULL); + channel_set_cell_handlers(chan, chan_test_cell_handler); tt_ptr_op(chan->cell_handler, OP_EQ, chan_test_cell_handler); /* Now process the cell, we should see it. */ old_count = test_chan_fixed_cells_recved;