Fix issues found by nickm.

* Document fmt_addr_impl() and friends.
* Parenthesize macro arguments.
* Rename get_first_listener_addrport_for_pt() to
  get_first_listener_addrport_string().
* Handle port_cfg_t with no_listen.
* Handle failure of router_get_active_listener_port_by_type().
* Add an XXX to router_get_active_listener_port_by_type().

6 files changed, 30 insertions, 12 deletions

diff --git a/src/common/address.c b/src/common/address.c
index 62cf16c03..7f78d1e4d 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -986,10 +986,15 @@ tor_dup_addr(const tor_addr_t *addr)
   }
 }
 
-/** Return a string representing the address <b>addr</b>.  This string is
- * statically allocated, and must not be freed.  Each call to
- * <b>fmt_addr</b> invalidates the last result of the function.  This
- * function is not thread-safe. */
+/** Return a string representing the address <b>addr</b>.  This string
+ * is statically allocated, and must not be freed.  Each call to
+ * <b>fmt_addr_impl</b> invalidates the last result of the function.
+ * This function is not thread-safe. If <b>decorate</b> is set, add
+ * brackets to IPv6 addresses.
+ *
+ * It's better to use the wrapper macros of this function:
+ * <b>fmt_addr()</b> and <b>fmt_and_decorate_addr()</b>.
+ */
 const char *
 fmt_addr_impl(const tor_addr_t *addr, int decorate)
 {
diff --git a/src/common/address.h b/src/common/address.h
index bdb14eb39..761eed661 100644
--- a/src/common/address.h
+++ b/src/common/address.h
@@ -135,8 +135,13 @@ tor_addr_eq_ipv4h(const tor_addr_t *a, uint32_t u)
 
 int tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr_out);
 char *tor_dup_addr(const tor_addr_t *addr) ATTR_MALLOC;
-#define fmt_addr(a) fmt_addr_impl(a, 0)
-#define fmt_and_decorate_addr(a) fmt_addr_impl(a, 1)
+
+/** Wrapper function of fmt_addr_impl(). It does not decorate IPv6
+ *  addresses. */
+#define fmt_addr(a) fmt_addr_impl((a), 0)
+/** Wrapper function of fmt_addr_impl(). It decorates IPv6
+ *  addresses. */
+#define fmt_and_decorate_addr(a) fmt_addr_impl((a), 1)
 const char *fmt_addr_impl(const tor_addr_t *addr, int decorate);
 const char * fmt_addr32(uint32_t addr);
 int get_interface_address6(int severity, sa_family_t family, tor_addr_t *addr);
diff --git a/src/or/config.c b/src/or/config.c
index 5ea1f5ede..3fc543c41 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -6033,9 +6033,10 @@ get_configured_ports(void)
  *  caller to free it after use.
  *
  *  This function is meant to be used by the pluggable transport proxy
- *  spawning code. */
+ *  spawning code, please make sure that it fits your purposes before
+ *  using it. */
 char *
-get_first_listener_addrport_for_pt(int listener_type)
+get_first_listener_addrport_string(int listener_type)
 {
   static const char *ipv4_localhost = "127.0.0.1";
   static const char *ipv6_localhost = "[::1]";
@@ -6047,6 +6048,8 @@ get_first_listener_addrport_for_pt(int listener_type)
     return NULL;
 
   SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) {
+    if (cfg->no_listen)
+      continue;
 
     if (cfg->type == listener_type &&
         tor_addr_family(&cfg->addr) != AF_UNSPEC) {
@@ -6064,10 +6067,13 @@ get_first_listener_addrport_for_pt(int listener_type)
       /* If a listener is configured with port 'auto', we are forced
          to iterate all listener connections and find out in which
          port it ended up listening: */
-      if (cfg->port == CFG_AUTO_PORT)
+      if (cfg->port == CFG_AUTO_PORT) {
         port = router_get_active_listener_port_by_type(listener_type);
-      else
+        if (!port)
+          return NULL;
+      } else {
         port = cfg->port;
+      }
 
       tor_asprintf(&string, "%s:%u", address, port);
 
diff --git a/src/or/config.h b/src/or/config.h
index fd84d9b60..049518651 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -72,7 +72,7 @@ int get_first_advertised_port_by_type_af(int listener_type,
 #define get_primary_dir_port() \
   (get_first_advertised_port_by_type_af(CONN_TYPE_DIR_LISTENER, AF_INET))
 
-char *get_first_listener_addrport_for_pt(int listener_type);
+char *get_first_listener_addrport_string(int listener_type);
 
 int options_need_geoip_info(const or_options_t *options,
                             const char **reason_out);
diff --git a/src/or/router.c b/src/or/router.c
index 67d26e13e..3b97d28d0 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1218,6 +1218,8 @@ consider_publishable_server(int force)
 
 /** Return the port of the first active listener of type
  *  <b>listener_type</b>. */
+/** XXX not a very good interface. it's not reliable when there are
+    multiple listeners. */
 uint16_t
 router_get_active_listener_port_by_type(int listener_type)
 {
diff --git a/src/or/transports.c b/src/or/transports.c
index 1f2149381..897645021 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -992,7 +992,7 @@ create_managed_proxy_environment(const managed_proxy_t *mp)
 
   if (mp->is_server) {
     {
-      char *orport_tmp = get_first_listener_addrport_for_pt(CONN_TYPE_OR_LISTENER);
+      char *orport_tmp = get_first_listener_addrport_string(CONN_TYPE_OR_LISTENER);
       smartlist_add_asprintf(envs, "TOR_PT_ORPORT=%s", orport_tmp);
       tor_free(orport_tmp);
     }