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().

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;