From 8775c93a996fd864b1747c04eebe51534425ff51 Mon Sep 17 00:00:00 2001 From: Caio Valente Date: Tue, 6 Mar 2018 20:42:32 +0100 Subject: [PATCH 1/2] 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 | 116 +++++++++++++++++--------------------------- src/or/router.h | 18 ------- 5 files changed, 55 insertions(+), 93 deletions(-) create mode 100644 changes/ticket25432 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; /**k. 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 buf (which must be at least NODE_DESC_BUF_LEN bytes long) to * hold a human-readable description of a node with identity digest * id_digest, named-status is_named, nickname nickname, @@ -3498,15 +3514,16 @@ format_node_description(char *buf, return buf; } -/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to - * hold a human-readable description of ri. +/** Return a human-readable description of the routerinfo_t ri. * - * - * Return a pointer to the front of buf. + * 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 ""; return format_node_description(buf, @@ -3517,14 +3534,15 @@ router_get_description(char *buf, const routerinfo_t *ri) ri->addr); } -/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to - * hold a human-readable description of node. +/** Return a human-readable description of the node_t node. * - * Return a pointer to the front of buf. + * 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 buf (which must be at least NODE_DESC_BUF_LEN bytes long) to - * hold a human-readable description of rs. - * - * Return a pointer to the front of buf. - */ -const char * -routerstatus_get_description(char *buf, const routerstatus_t *rs) -{ - if (!rs) - return ""; - return format_node_description(buf, - rs->identity_digest, - rs->is_named, - rs->nickname, - NULL, - rs->addr); -} - -/** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to - * hold a human-readable description of ei. - * - * Return a pointer to the front of buf. - */ -const char * -extend_info_get_description(char *buf, const extend_info_t *ei) -{ - if (!ei) - return ""; - return format_node_description(buf, - ei->identity_digest, - 0, - ei->nickname, - &ei->addr, - 0); -} - -/** Return a human-readable description of the routerinfo_t ri. - * - * 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 node. - * - * 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 rs. * * 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 ""; + 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 ei. @@ -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 ""; + return format_node_description(buf, + ei->identity_digest, + 0, + ei->nickname, + &ei->addr, + 0); } /** Set buf (which must have MAX_VERBOSE_NICKNAME_LEN+1 bytes) to the 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); From 216bc353d3c724e8348408800f389c0c76c2bbc6 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Sun, 25 Mar 2018 20:13:00 +0300 Subject: [PATCH 2/2] fixup! Refactor: suppress duplicated functions from router.c and encapsulate NODE_DESC_BUF_LEN constant. --- changes/ticket25432 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changes/ticket25432 b/changes/ticket25432 index 2179770f6..21ca20134 100644 --- a/changes/ticket25432 +++ b/changes/ticket25432 @@ -1,6 +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 + 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. + Closes ticket 25432. Patch by valentecaio.