Refactor: suppress duplicated functions from router.c and encapsulate NODE_DESC_BUF_L...

Also encapsulates format_node_description().

Closes ticket 25432.

diff --git a/changes/ticket25432 b/changes/ticket25432
new file mode 100644 (file)
index 0000000..2179770
--- /dev/null
+++ b/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.

diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 7a693b9d437f414735081bece7df9ef504452817..7dae5ee9d1e74c8e995fb0606ab75f5f976e746e 100644 (file)
--- a/src/or/dirserv.c
+++ b/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:"");

diff --git a/src/or/or.h b/src/or/or.h
index 045cdd9e140ddde02685b0ef78221a85fd41f1e7..ecee534fee3fa5d6a3cc9a68f4d87b8e57720e4a 100644 (file)
--- a/src/or/or.h
+++ b/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. */

diff --git a/src/or/router.c b/src/or/router.c
index 991c7138c877185a6a4ccbdeb92e045c76f15ddd..e5996f665eafd39906de67f8be319db7f8e93e54 100644 (file)
--- a/src/or/router.c
+++ b/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
- * hold a human-readable description of <b>ri</b>.
- *
+/** Return a human-readable description of the routerinfo_t <b>ri</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 *
-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
- * hold a human-readable description of <b>node</b>.
+/** Return a human-readable description of the node_t <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,14 +3567,16 @@ 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 human-readable description of the routerstatus_t <b>rs</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 *
-routerstatus_get_description(char *buf, const routerstatus_t *rs)
+routerstatus_describe(const routerstatus_t *rs)
 {
+  static char buf[NODE_DESC_BUF_LEN];
+
   if (!rs)
     return "<null>";
   return format_node_description(buf,
@@ -3567,14 +3587,16 @@ routerstatus_get_description(char *buf, const routerstatus_t *rs)
                                  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 human-readable description of the extend_info_t <b>ei</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 *
-extend_info_get_description(char *buf, const extend_info_t *ei)
+extend_info_describe(const extend_info_t *ei)
 {
+  static char buf[NODE_DESC_BUF_LEN];
+
   if (!ei)
     return "<null>";
   return format_node_description(buf,
@@ -3585,54 +3607,6 @@ extend_info_get_description(char *buf, const extend_info_t *ei)
                                  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
- * previous values returned by this function.
- */
-const char *
-routerstatus_describe(const routerstatus_t *rs)
-{
-  static char buf[NODE_DESC_BUF_LEN];
-  return routerstatus_get_description(buf, rs);
-}
-
-/** Return a human-readable description of the extend_info_t <b>ei</b>.
- *
- * This function is not thread-safe.  Each call to this function invalidates
- * previous values returned by this function.
- */
-const char *
-extend_info_describe(const extend_info_t *ei)
-{
-  static char buf[NODE_DESC_BUF_LEN];
-  return extend_info_get_description(buf, ei);
-}
-
 /** Set <b>buf</b> (which must have MAX_VERBOSE_NICKNAME_LEN+1 bytes) to the
  * verbose representation of the identity of <b>router</b>.  The format is:
  *  A dollar sign.

diff --git a/src/or/router.h b/src/or/router.h
index 7f50308956288c0f19f604b5273f1b87f65e117b..e5efe577e3b484254e523a58881c0eb968fbb28d 100644 (file)
--- a/src/or/router.h
+++ b/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);