commit 4f4785a8c113806d00c6a31154956edb86c1639f Author: Nick Mathewson nickm@torproject.org Date: Tue Jul 14 14:26:24 2020 -0400
Refactor channel description internals.
Now that we've clarified that these functions only need to describe the peer in a human-readable way, we can have them delegate to connection_describe_peer(). --- src/core/or/channel.c | 19 ++++++------------ src/core/or/channel.h | 8 +++----- src/core/or/channeltls.c | 51 +++++++++++------------------------------------- src/test/test_channel.c | 5 ++--- 4 files changed, 22 insertions(+), 61 deletions(-)
diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 5ea38256ac..2de164b39c 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2794,31 +2794,24 @@ channel_listener_dump_transport_statistics(channel_listener_t *chan_l, const char * channel_get_actual_remote_descr(channel_t *chan) { - tor_assert(chan); - tor_assert(chan->get_remote_descr); - - /* Param 1 indicates the actual description */ - return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL); + return channel_get_canonical_remote_descr(chan); }
/** * Return text description of the remote endpoint canonical address. * - * This function return a test provided by the lower layer of the remote - * endpoint for this channel; it should use the known canonical address for - * this OR's identity digest if possible. + * This function returns a human-readable string for logging; nothing + * should parse it or rely on a particular format. * - * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} - * may invalidate the return value from this function. + * Subsequent calls to this function may invalidate its return value. */ MOCK_IMPL(const char *, channel_get_canonical_remote_descr,(channel_t *chan)) { tor_assert(chan); - tor_assert(chan->get_remote_descr); + tor_assert(chan->describe_peer);
- /* Param 0 indicates the canonicalized description */ - return chan->get_remote_descr(chan, 0); + return chan->describe_peer(chan); }
/** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index f047d24f66..917941a0b6 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -336,13 +336,11 @@ struct channel_t { int (*get_remote_addr)(const channel_t *, tor_addr_t *); int (*get_transport_name)(channel_t *chan, char **transport_out);
-#define GRD_FLAG_ORIGINAL 1 /** - * Get a text description of the remote endpoint; canonicalized if the flag - * GRD_FLAG_ORIGINAL is not set, or the one we originally connected - * to/received from if it is. + * Get a human-readable text description of the remote endpoint, for + * logging. */ - const char * (*get_remote_descr)(channel_t *, int); + const char * (*describe_peer)(const channel_t *); /** Check if the lower layer has queued writes */ int (*has_queued_writes)(channel_t *); /** diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 3a10b2c27d..ade26684b8 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -107,8 +107,7 @@ static int channel_tls_get_remote_addr_method(const channel_t *chan, tor_addr_t *addr_out); static int channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); -static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int flags); +static const char *channel_tls_describe_peer_method(const channel_t *chan); static int channel_tls_has_queued_writes_method(channel_t *chan); static int channel_tls_is_canonical_method(channel_t *chan, int req); static int @@ -164,7 +163,7 @@ channel_tls_common_init(channel_tls_t *tlschan) chan->free_fn = channel_tls_free_method; chan->get_overhead_estimate = channel_tls_get_overhead_estimate_method; chan->get_remote_addr = channel_tls_get_remote_addr_method; - chan->get_remote_descr = channel_tls_get_remote_descr_method; + chan->describe_peer = channel_tls_describe_peer_method; chan->get_transport_name = channel_tls_get_transport_name_method; chan->has_queued_writes = channel_tls_has_queued_writes_method; chan->is_canonical = channel_tls_is_canonical_method; @@ -567,50 +566,22 @@ channel_tls_get_transport_name_method(channel_t *chan, char **transport_out) }
/** - * Get endpoint description of a channel_tls_t. + * Get a human-readable endpoint description of a channel_tls_t. * - * This implements the get_remote_descr method for channel_tls_t; it returns - * a text description of the remote endpoint of the channel suitable for use - * in log messages. The req parameter is 0 for the canonical address or 1 for - * the actual address seen. + * This format is intended for logging, and may change in the future; + * nothing should parse or rely on its particular details. */ static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int flags) +channel_tls_describe_peer_method(const channel_t *chan) { - static char buf[TOR_ADDRPORT_BUF_LEN]; - channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); - connection_t *conn; - const char *answer = NULL; - char *addr_str; - + const channel_tls_t *tlschan = BASE_CHAN_TO_TLS((channel_t*)chan); tor_assert(tlschan);
if (tlschan->conn) { - conn = TO_CONN(tlschan->conn); - switch (flags) { - case 0: - /* Canonical address with port*/ - tor_snprintf(buf, TOR_ADDRPORT_BUF_LEN, - "%s:%u", conn->address, conn->port); - answer = buf; - break; - case GRD_FLAG_ORIGINAL: - /* Actual address with port */ - addr_str = tor_addr_to_str_dup(&(tlschan->conn->real_addr)); - tor_snprintf(buf, TOR_ADDRPORT_BUF_LEN, "%s:%u", addr_str, conn->port); - tor_free(addr_str); - answer = buf; - break; - default: - /* Something's broken in channel.c */ - tor_assert_nonfatal_unreached_once(); - } + return connection_describe_peer(TO_CONN(tlschan->conn)); } else { - strlcpy(buf, "(No connection)", sizeof(buf)); - answer = buf; + return "(No connection)"; } - - return answer; }
/** @@ -1903,8 +1874,8 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
if (me && !TLS_CHAN_TO_BASE(chan)->is_canonical_to_peer && channel_is_canonical(TLS_CHAN_TO_BASE(chan))) { - const char *descr = - TLS_CHAN_TO_BASE(chan)->get_remote_descr(TLS_CHAN_TO_BASE(chan), 0); + const char *descr = channel_get_actual_remote_descr( + TLS_CHAN_TO_BASE(chan)); log_info(LD_OR, "We made a connection to a relay at %s (fp=%s) but we think " "they will not consider this connection canonical. They " diff --git a/src/test/test_channel.c b/src/test/test_channel.c index efe195eac4..14d8a4eae7 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -163,10 +163,9 @@ chan_test_finish_close(channel_t *ch) }
static const char * -chan_test_get_remote_descr(channel_t *ch, int flags) +chan_test_describe_peer(const channel_t *ch) { tt_assert(ch); - tt_int_op(flags & ~(GRD_FLAG_ORIGINAL), OP_EQ, 0);
done: return "Fake channel for unit tests; no real endpoint"; @@ -276,7 +275,7 @@ new_fake_channel(void)
chan->close = chan_test_close; chan->num_cells_writeable = chan_test_num_cells_writeable; - chan->get_remote_descr = chan_test_get_remote_descr; + chan->describe_peer = chan_test_describe_peer; chan->get_remote_addr = chan_test_get_remote_addr; chan->write_packed_cell = chan_test_write_packed_cell; chan->write_var_cell = chan_test_write_var_cell;