commit 8775c93a996fd864b1747c04eebe51534425ff51 Author: Caio Valente valentecaio95@gmail.com Date: Tue Mar 6 20:42:32 2018 +0100

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

Also encapsulates format_node_description().

Closes ticket 25432. --- changes/ticket25432 | 6 +++ src/or/dirserv.c | 4 +- src/or/or.h | 4 +- src/or/router.c | 104 ++++++++++++++++++++-------------------------------- src/or/router.h | 18 --------- 5 files changed, 49 insertions(+), 87 deletions(-)

diff --git a/changes/ticket25432 b/changes/ticket25432 new file mode 100644 index 000000000..2179770f6 --- /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 7a693b9d4..7dae5ee9d 100644 --- 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 045cdd9e1..ecee534fe 100644 --- 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 991c7138c..e5996f665 100644 --- 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 7f5030895..e5efe577e 100644 --- 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);