Refactor: suppress duplicated functions from router.c and encapsulate NODE_DESC_BUF_LEN constant.

Also encapsulates format_node_description().

Closes ticket 25432.

changes/ticket25432

@@ -0,0 +1,6 @@
+  o Code simplification and refactoring:
+    - Merge functions used for describing nodes and suppress the functions
+      that do not allocate memory for the output buffer string.
+    - NODE_DESC_BUF_LEN constant and format_node_description() function
+      cannot be used externally from router.c module anymore.
+    - Closes ticket 25432. Patch by valentecaio.

src/or/dirserv.c

@@ -858,13 +858,13 @@ directory_remove_invalid(void)
 
   SMARTLIST_FOREACH_BEGIN(nodes, node_t *, node) {
     const char *msg = NULL;
+    const char *description;
     routerinfo_t *ent = node->ri;
-    char description[NODE_DESC_BUF_LEN];
     uint32_t r;
     if (!ent)
       continue;
     r = dirserv_router_get_status(ent, &msg, LOG_INFO);
-    router_get_description(description, ent);
+    description = router_describe(ent);
     if (r & FP_REJECT) {
       log_info(LD_DIRSERV, "Router %s is now rejected: %s",
                description, msg?msg:"");

src/or/or.h

@@ -2343,10 +2343,10 @@ typedef struct routerstatus_t {
    * If it's a descriptor, we only use the first DIGEST_LEN bytes. */
   char descriptor_digest[DIGEST256_LEN];
   uint32_t addr; /**< IPv4 address for this router, in host order. */
-  uint16_t or_port; /**< OR port for this router. */
+  uint16_t or_port; /**< IPv4 OR port for this router. */
   uint16_t dir_port; /**< Directory port for this router. */
   tor_addr_t ipv6_addr; /**< IPv6 address for this router. */
-  uint16_t ipv6_orport; /**<IPV6 OR port for this router. */
+  uint16_t ipv6_orport; /**< IPv6 OR port for this router. */
   unsigned int is_authority:1; /**< True iff this router is an authority. */
   unsigned int is_exit:1; /**< True iff this router is a good exit. */
   unsigned int is_stable:1; /**< True iff this router stays up a long time. */

src/or/router.c

@@ -103,6 +103,13 @@ static authority_cert_t *legacy_key_certificate = NULL;
  * used by tor-gencert to sign new signing keys and make new key
  * certificates. */
 
+const char *format_node_description(char *buf,
+                                    const char *id_digest,
+                                    int is_named,
+                                    const char *nickname,
+                                    const tor_addr_t *addr,
+                                    uint32_t addr32h);
+
 /** Replace the current onion key with <b>k</b>.  Does not affect
  * lastonionkey; to update lastonionkey correctly, call rotate_onion_key().
  */
@@ -3453,6 +3460,15 @@ is_legal_hexdigest(const char *s)
           strspn(s,HEX_CHARACTERS)==HEX_DIGEST_LEN);
 }
 
+/**
+ * Longest allowed output of format_node_description, plus 1 character for
+ * NUL.  This allows space for:
+ * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at"
+ * " [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]"
+ * plus a terminating NUL.
+ */
+#define NODE_DESC_BUF_LEN (MAX_VERBOSE_NICKNAME_LEN+4+TOR_ADDR_BUF_LEN)
+
 /** Use <b>buf</b> (which must be at least NODE_DESC_BUF_LEN bytes long) to
  * hold a human-readable description of a node with identity digest
  * <b>id_digest</b>, named-status <b>is_named</b>, nickname <b>nickname</b>,
@@ -3498,15 +3514,16 @@ format_node_description(char *buf,
   return buf;
 }
 
-/** Use <b>buf</b> (which must be at least NODE_DESC_BUF_LEN bytes long) to
+/** Return a human-readable description of the routerinfo_t <b>ri</b>.
- * hold a human-readable description of <b>ri</b>.
  *
- *
+ * This function is not thread-safe.  Each call to this function invalidates
- * Return a pointer to the front of <b>buf</b>.
+ * previous values returned by this function.
  */
 const char *
-router_get_description(char *buf, const routerinfo_t *ri)
+router_describe(const routerinfo_t *ri)
 {
+  static char buf[NODE_DESC_BUF_LEN];
+
   if (!ri)
     return "<null>";
   return format_node_description(buf,
@@ -3517,14 +3534,15 @@ router_get_description(char *buf, const routerinfo_t *ri)
                                  ri->addr);
 }
 
-/** Use <b>buf</b> (which must be at least NODE_DESC_BUF_LEN bytes long) to
+/** Return a human-readable description of the node_t <b>node</b>.
- * hold a human-readable description of <b>node</b>.
  *
- * Return a pointer to the front of <b>buf</b>.
+ * This function is not thread-safe.  Each call to this function invalidates
+ * previous values returned by this function.
  */
 const char *
-node_get_description(char *buf, const node_t *node)
+node_describe(const node_t *node)
 {
+  static char buf[NODE_DESC_BUF_LEN];
   const char *nickname = NULL;
   uint32_t addr32h = 0;
   int is_named = 0;
@@ -3549,66 +3567,6 @@ node_get_description(char *buf, const node_t *node)
                                  addr32h);
 }
 
-/** Use <b>buf</b> (which must be at least NODE_DESC_BUF_LEN bytes long) to
- * hold a human-readable description of <b>rs</b>.
- *
- * Return a pointer to the front of <b>buf</b>.
- */
-const char *
-routerstatus_get_description(char *buf, const routerstatus_t *rs)
-{
-  if (!rs)
-    return "<null>";
-  return format_node_description(buf,
-                                 rs->identity_digest,
-                                 rs->is_named,
-                                 rs->nickname,
-                                 NULL,
-                                 rs->addr);
-}
-
-/** Use <b>buf</b> (which must be at least NODE_DESC_BUF_LEN bytes long) to
- * hold a human-readable description of <b>ei</b>.
- *
- * Return a pointer to the front of <b>buf</b>.
- */
-const char *
-extend_info_get_description(char *buf, const extend_info_t *ei)
-{
-  if (!ei)
-    return "<null>";
-  return format_node_description(buf,
-                                 ei->identity_digest,
-                                 0,
-                                 ei->nickname,
-                                 &ei->addr,
-                                 0);
-}
-
-/** Return a human-readable description of the routerinfo_t <b>ri</b>.
- *
- * This function is not thread-safe.  Each call to this function invalidates
- * previous values returned by this function.
- */
-const char *
-router_describe(const routerinfo_t *ri)
-{
-  static char buf[NODE_DESC_BUF_LEN];
-  return router_get_description(buf, ri);
-}
-
-/** Return a human-readable description of the node_t <b>node</b>.
- *
- * This function is not thread-safe.  Each call to this function invalidates
- * previous values returned by this function.
- */
-const char *
-node_describe(const node_t *node)
-{
-  static char buf[NODE_DESC_BUF_LEN];
-  return node_get_description(buf, node);
-}
-
 /** Return a human-readable description of the routerstatus_t <b>rs</b>.
  *
  * This function is not thread-safe.  Each call to this function invalidates
@@ -3618,7 +3576,15 @@ const char *
 routerstatus_describe(const routerstatus_t *rs)
 {
   static char buf[NODE_DESC_BUF_LEN];
-  return routerstatus_get_description(buf, rs);
+
+  if (!rs)
+    return "<null>";
+  return format_node_description(buf,
+                                 rs->identity_digest,
+                                 rs->is_named,
+                                 rs->nickname,
+                                 NULL,
+                                 rs->addr);
 }
 
 /** Return a human-readable description of the extend_info_t <b>ei</b>.
@@ -3630,7 +3596,15 @@ const char *
 extend_info_describe(const extend_info_t *ei)
 {
   static char buf[NODE_DESC_BUF_LEN];
-  return extend_info_get_description(buf, ei);
+
+  if (!ei)
+    return "<null>";
+  return format_node_description(buf,
+                                 ei->identity_digest,
+                                 0,
+                                 ei->nickname,
+                                 &ei->addr,
+                                 0);
 }
 
 /** Set <b>buf</b> (which must have MAX_VERBOSE_NICKNAME_LEN+1 bytes) to the

src/or/router.h

@@ -123,24 +123,6 @@ int is_legal_nickname(const char *s);
 int is_legal_nickname_or_hexdigest(const char *s);
 int is_legal_hexdigest(const char *s);
 
-/**
- * Longest allowed output of format_node_description, plus 1 character for
- * NUL.  This allows space for:
- * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at"
- * " [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]"
- * plus a terminating NUL.
- */
-#define NODE_DESC_BUF_LEN (MAX_VERBOSE_NICKNAME_LEN+4+TOR_ADDR_BUF_LEN)
-const char *format_node_description(char *buf,
-                                    const char *id_digest,
-                                    int is_named,
-                                    const char *nickname,
-                                    const tor_addr_t *addr,
-                                    uint32_t addr32h);
-const char *router_get_description(char *buf, const routerinfo_t *ri);
-const char *node_get_description(char *buf, const node_t *node);
-const char *routerstatus_get_description(char *buf, const routerstatus_t *rs);
-const char *extend_info_get_description(char *buf, const extend_info_t *ei);
 const char *router_describe(const routerinfo_t *ri);
 const char *node_describe(const node_t *node);
 const char *routerstatus_describe(const routerstatus_t *ri);