From 59dac38ebe4e6489c2a06d07470023ed5fbdd0aa Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@torproject.org>
Date: Tue, 7 Jan 2020 15:03:24 -0500
Subject: [PATCH] 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(-)
create mode 100644 changes/ticket32892
diff --git a/changes/ticket32892 b/changes/ticket32892
new file mode 100644
index 0000000000..f6360d2883
--- /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 2faf638756..eaf2ce311e 100644
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -1066,23 +1066,6 @@ channel_get_cell_handler(channel_t *chan)
return NULL;
}
-/**
- * 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.
*
@@ -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 0f6fa228dd..6360cf3f4f 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 1c97437769..2f3459f1ea 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 8c90e1de6f..c0af6f26d1 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 0353032796..c66170c46f 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;
--
GitLab