[tor-commits] [tor/master] Refactor node lookup APIs to take flags
nickm at torproject.org
nickm at torproject.org
Fri Sep 8 16:22:17 UTC 2017
commit 80d3887360548b28fe2bd06501f0d51d0a1ba4f0
Author: Nick Mathewson <nickm at torproject.org>
Date: Tue Aug 22 19:04:31 2017 -0400
Refactor node lookup APIs to take flags
Right now there's a single warn_if_unnamed flag for
router_get_consensus_status_by_nickname() and
node_get_by_nickname(), that is nearly always 1. I've turned it
into an 'unsigned' bitfield, and inverted its sense. I've added the
flags argument to node_get_by_hex_id() too, though it does nothing
there right now.
I've removed the router_get_consensus_status_by_nickname() function,
since it was only used in once place.
This patch changes the warning behavior of GETINFO ns/name/<name>,
since all other name lookups from the controller currently warn.
Later I'm going to add more flags, for ed25519 support.
---
src/or/addressmap.c | 2 +-
src/or/circuituse.c | 6 +++---
src/or/connection_edge.c | 9 +++++----
src/or/control.c | 16 ++++++++--------
src/or/networkstatus.c | 18 ++----------------
src/or/networkstatus.h | 3 ---
src/or/nodelist.c | 20 ++++++++++++--------
src/or/nodelist.h | 8 ++++++--
src/or/rendservice.c | 2 +-
src/or/router.c | 2 +-
src/or/routerset.c | 2 +-
src/test/test_routerset.c | 18 +++++++++---------
12 files changed, 49 insertions(+), 57 deletions(-)
diff --git a/src/or/addressmap.c b/src/or/addressmap.c
index c92af3825..f278564e8 100644
--- a/src/or/addressmap.c
+++ b/src/or/addressmap.c
@@ -214,7 +214,7 @@ addressmap_clear_excluded_trackexithosts(const or_options_t *options)
dot--;
if (*dot == '.') dot++;
nodename = tor_strndup(dot, len-5-(dot-target));;
- node = node_get_by_nickname(nodename, 0);
+ node = node_get_by_nickname(nodename, NNF_NO_WARN_UNNAMED);
tor_free(nodename);
if (!node ||
(allow_nodes && !routerset_contains_node(allow_nodes, node)) ||
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 21cc9c540..240e25293 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -2119,7 +2119,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
} else {
/* XXXX Duplicates checks in connection_ap_handshake_attach_circuit:
* refactor into a single function. */
- const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1);
+ const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0);
int opt = conn->chosen_exit_optional;
if (node && !connection_ap_can_use_exit(conn, node)) {
log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP,
@@ -2199,7 +2199,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
if (conn->chosen_exit_name) {
const node_t *r;
int opt = conn->chosen_exit_optional;
- r = node_get_by_nickname(conn->chosen_exit_name, 1);
+ r = node_get_by_nickname(conn->chosen_exit_name, 0);
if (r && node_has_descriptor(r)) {
/* We might want to connect to an IPv6 bridge for loading
descriptors so we use the preferred address rather than
@@ -2598,7 +2598,7 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn)
* open to that exit. See what exit we meant, and whether we can use it.
*/
if (conn->chosen_exit_name) {
- const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1);
+ const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0);
int opt = conn->chosen_exit_optional;
if (!node && !want_onehop) {
/* We ran into this warning when trying to extend a circuit to a
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 12ddc7e82..c42adbf73 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1074,7 +1074,8 @@ circuit_discard_optional_exit_enclaves(extend_info_t *info)
if (!entry_conn->chosen_exit_optional &&
!entry_conn->chosen_exit_retries)
continue;
- r1 = node_get_by_nickname(entry_conn->chosen_exit_name, 0);
+ r1 = node_get_by_nickname(entry_conn->chosen_exit_name,
+ NNF_NO_WARN_UNNAMED);
r2 = node_get_by_id(info->identity_digest);
if (!r1 || !r2 || r1 != r2)
continue;
@@ -1508,7 +1509,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
if (s[1] != '\0') {
/* Looks like a real .exit one. */
conn->chosen_exit_name = tor_strdup(s+1);
- node = node_get_by_nickname(conn->chosen_exit_name, 1);
+ node = node_get_by_nickname(conn->chosen_exit_name, 0);
if (exit_source == ADDRMAPSRC_TRACKEXIT) {
/* We 5 tries before it expires the addressmap */
@@ -1529,7 +1530,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
* form that means (foo's address).foo.exit. */
conn->chosen_exit_name = tor_strdup(socks->address);
- node = node_get_by_nickname(conn->chosen_exit_name, 1);
+ node = node_get_by_nickname(conn->chosen_exit_name, 0);
if (node) {
*socks->address = 0;
node_get_address_string(node, socks->address, sizeof(socks->address));
@@ -3630,7 +3631,7 @@ connection_ap_can_use_exit(const entry_connection_t *conn,
*/
if (conn->chosen_exit_name) {
const node_t *chosen_exit =
- node_get_by_nickname(conn->chosen_exit_name, 1);
+ node_get_by_nickname(conn->chosen_exit_name, 0);
if (!chosen_exit || tor_memneq(chosen_exit->identity,
exit_node->identity, DIGEST_LEN)) {
/* doesn't match */
diff --git a/src/or/control.c b/src/or/control.c
index 724d4b35c..889436c67 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -1886,7 +1886,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
(void) control_conn;
if (!strcmpstart(question, "desc/id/")) {
const routerinfo_t *ri = NULL;
- const node_t *node = node_get_by_hex_id(question+strlen("desc/id/"));
+ const node_t *node = node_get_by_hex_id(question+strlen("desc/id/"), 0);
if (node)
ri = node->ri;
if (ri) {
@@ -1905,7 +1905,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
/* XXX Setting 'warn_if_unnamed' here is a bit silly -- the
* warning goes to the user, not to the controller. */
const node_t *node =
- node_get_by_nickname(question+strlen("desc/name/"), 1);
+ node_get_by_nickname(question+strlen("desc/name/"), 0);
if (node)
ri = node->ri;
if (ri) {
@@ -1991,7 +1991,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
return -1;
}
} else if (!strcmpstart(question, "md/id/")) {
- const node_t *node = node_get_by_hex_id(question+strlen("md/id/"));
+ const node_t *node = node_get_by_hex_id(question+strlen("md/id/"), 0);
const microdesc_t *md = NULL;
if (node) md = node->md;
if (md && md->body) {
@@ -2000,7 +2000,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
} else if (!strcmpstart(question, "md/name/")) {
/* XXX Setting 'warn_if_unnamed' here is a bit silly -- the
* warning goes to the user, not to the controller. */
- const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 1);
+ const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 0);
/* XXXX duplicated code */
const microdesc_t *md = NULL;
if (node) md = node->md;
@@ -2013,7 +2013,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
} else if (!strcmpstart(question, "desc-annotations/id/")) {
const routerinfo_t *ri = NULL;
const node_t *node =
- node_get_by_hex_id(question+strlen("desc-annotations/id/"));
+ node_get_by_hex_id(question+strlen("desc-annotations/id/"), 0);
if (node)
ri = node->ri;
if (ri) {
@@ -3394,7 +3394,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
nodes = smartlist_new();
SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) {
- const node_t *node = node_get_by_nickname(n, 1);
+ const node_t *node = node_get_by_nickname(n, 0);
if (!node) {
connection_printf_to_buf(conn, "552 No such router \"%s\"\r\n", n);
goto done;
@@ -4158,7 +4158,7 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
const char *server;
server = arg + strlen(opt_server);
- node = node_get_by_hex_id(server);
+ node = node_get_by_hex_id(server, 0);
if (!node) {
connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
server);
@@ -4239,7 +4239,7 @@ handle_control_hspost(control_connection_t *conn,
SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {
if (!strcasecmpstart(arg, opt_server)) {
const char *server = arg + strlen(opt_server);
- const node_t *node = node_get_by_hex_id(server);
+ const node_t *node = node_get_by_hex_id(server, 0);
if (!node || !node->rs) {
connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 69bff55cf..6263be36c 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -794,21 +794,6 @@ router_get_consensus_status_by_id(const char *digest)
return router_get_mutable_consensus_status_by_id(digest);
}
-/** Given a nickname (possibly verbose, possibly a hexadecimal digest), return
- * the corresponding routerstatus_t, or NULL if none exists. Warn the
- * user if <b>warn_if_unnamed</b> is set, and they have specified a router by
- * nickname, but the Named flag isn't set for that router. */
-const routerstatus_t *
-router_get_consensus_status_by_nickname(const char *nickname,
- int warn_if_unnamed)
-{
- const node_t *node = node_get_by_nickname(nickname, warn_if_unnamed);
- if (node)
- return node->rs;
- else
- return NULL;
-}
-
/** Return the identity digest that's mapped to officially by
* <b>nickname</b>. */
const char *
@@ -2555,7 +2540,8 @@ getinfo_helper_networkstatus(control_connection_t *conn,
}
status = router_get_consensus_status_by_id(d);
} else if (!strcmpstart(question, "ns/name/")) {
- status = router_get_consensus_status_by_nickname(question+8, 0);
+ const node_t *n = node_get_by_nickname(question+8, 0);
+ status = n ? n->rs : NULL;
} else if (!strcmpstart(question, "ns/purpose/")) {
*answer = networkstatus_getinfo_by_purpose(question+11, time(NULL));
return *answer ? 0 : -1;
diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index f9320747d..145be37d1 100644
--- a/src/or/networkstatus.h
+++ b/src/or/networkstatus.h
@@ -62,9 +62,6 @@ const routerstatus_t *router_get_consensus_status_by_descriptor_digest(
MOCK_DECL(routerstatus_t *,
router_get_mutable_consensus_status_by_descriptor_digest,
(networkstatus_t *consensus, const char *digest));
-const routerstatus_t *router_get_consensus_status_by_nickname(
- const char *nickname,
- int warn_if_unnamed);
const char *networkstatus_get_router_digest_by_nickname(const char *nickname);
int networkstatus_nickname_is_unnamed(const char *nickname);
int we_want_to_fetch_flavor(const or_options_t *options, int flavor);
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index b1cb8d6b5..f7cae9281 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -762,14 +762,16 @@ nodelist_get_list,(void))
/** Given a hex-encoded nickname of the format DIGEST, $DIGEST, $DIGEST=name,
* or $DIGEST~name, return the node with the matching identity digest and
* nickname (if any). Return NULL if no such node exists, or if <b>hex_id</b>
- * is not well-formed. */
+ * is not well-formed. DOCDOC flags */
const node_t *
-node_get_by_hex_id(const char *hex_id)
+node_get_by_hex_id(const char *hex_id, unsigned flags)
{
char digest_buf[DIGEST_LEN];
char nn_buf[MAX_NICKNAME_LEN+1];
char nn_char='\0';
+ (void) flags; // XXXX
+
if (hex_digest_nickname_decode(hex_id, digest_buf, &nn_char, nn_buf)==0) {
const node_t *node = node_get_by_id(digest_buf);
if (!node)
@@ -785,19 +787,21 @@ node_get_by_hex_id(const char *hex_id)
}
/** Given a nickname (possibly verbose, possibly a hexadecimal digest), return
- * the corresponding node_t, or NULL if none exists. Warn the user if
- * <b>warn_if_unnamed</b> is set, and they have specified a router by
- * nickname, but the Named flag isn't set for that router. */
+ * the corresponding node_t, or NULL if none exists. Warn the user if they
+ * have specified a router by nickname, unless the NNF_NO_WARN_UNNAMED bit is
+ * set in <b>flags</b>. */
MOCK_IMPL(const node_t *,
-node_get_by_nickname,(const char *nickname, int warn_if_unnamed))
+node_get_by_nickname,(const char *nickname, unsigned flags))
{
+ const int warn_if_unnamed = !(flags & NNF_NO_WARN_UNNAMED);
+
if (!the_nodelist)
return NULL;
/* Handle these cases: DIGEST, $DIGEST, $DIGEST=name, $DIGEST~name. */
{
const node_t *node;
- if ((node = node_get_by_hex_id(nickname)) != NULL)
+ if ((node = node_get_by_hex_id(nickname, flags)) != NULL)
return node;
}
@@ -1719,7 +1723,7 @@ nodelist_add_node_and_family(smartlist_t *sl, const node_t *node)
SMARTLIST_FOREACH_BEGIN(declared_family, const char *, name) {
const node_t *node2;
const smartlist_t *family2;
- if (!(node2 = node_get_by_nickname(name, 0)))
+ if (!(node2 = node_get_by_nickname(name, NNF_NO_WARN_UNNAMED)))
continue;
if (!(family2 = node_get_declared_family(node2)))
continue;
diff --git a/src/or/nodelist.h b/src/or/nodelist.h
index c5805c55f..79ff720c9 100644
--- a/src/or/nodelist.h
+++ b/src/or/nodelist.h
@@ -21,7 +21,11 @@ MOCK_DECL(const node_t *, node_get_by_id, (const char *identity_digest));
node_t *node_get_mutable_by_ed25519_id(const ed25519_public_key_t *ed_id);
MOCK_DECL(const node_t *, node_get_by_ed25519_id,
(const ed25519_public_key_t *ed_id));
-const node_t *node_get_by_hex_id(const char *identity_digest);
+
+#define NNF_NO_WARN_UNNAMED (1u<<0)
+
+const node_t *node_get_by_hex_id(const char *identity_digest,
+ unsigned flags);
node_t *nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out);
node_t *nodelist_add_microdesc(microdesc_t *md);
void nodelist_set_consensus(networkstatus_t *ns);
@@ -35,7 +39,7 @@ void nodelist_free_all(void);
void nodelist_assert_ok(void);
MOCK_DECL(const node_t *, node_get_by_nickname,
- (const char *nickname, int warn_if_unnamed));
+ (const char *nickname, unsigned flags));
void node_get_verbose_nickname(const node_t *node,
char *verbose_name_out);
void node_get_verbose_nickname_by_id(const char *id_digest,
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index a205b00c6..925373883 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -2112,7 +2112,7 @@ find_rp_for_intro(const rend_intro_cell_t *intro,
if (intro->version == 0 || intro->version == 1) {
rp_nickname = (const char *)(intro->u.v0_v1.rp);
- node = node_get_by_nickname(rp_nickname, 0);
+ node = node_get_by_nickname(rp_nickname, NNF_NO_WARN_UNNAMED);
if (!node) {
if (err_msg_out) {
tor_asprintf(&err_msg,
diff --git a/src/or/router.c b/src/or/router.c
index 7fad57265..8629168c8 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -2279,7 +2279,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
if (!strcasecmp(name, options->Nickname))
continue; /* Don't list ourself, that's redundant */
else
- member = node_get_by_nickname(name, 1);
+ member = node_get_by_nickname(name, 0);
if (!member) {
int is_legal = is_legal_nickname_or_hexdigest(name);
if (!smartlist_contains_string(warned_nonexistent_family, name) &&
diff --git a/src/or/routerset.c b/src/or/routerset.c
index 4906c6a51..54e26ef94 100644
--- a/src/or/routerset.c
+++ b/src/or/routerset.c
@@ -362,7 +362,7 @@ routerset_get_all_nodes(smartlist_t *out, const routerset_t *routerset,
/* No routers are specified by type; all are given by name or digest.
* we can do a lookup in O(len(routerset)). */
SMARTLIST_FOREACH(routerset->list, const char *, name, {
- const node_t *node = node_get_by_nickname(name, 1);
+ const node_t *node = node_get_by_nickname(name, 0);
if (node) {
if (!running_only || node->is_running)
if (!routerset_contains_node(excludeset, node))
diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c
index 7efd042ed..c9c69911d 100644
--- a/src/test/test_routerset.c
+++ b/src/test/test_routerset.c
@@ -1602,7 +1602,7 @@ NS(test_main)(void *arg)
*/
NS_DECL(const node_t *, node_get_by_nickname,
- (const char *nickname, int warn_if_unused));
+ (const char *nickname, unsigned flags));
static const char *NS(mock_nickname);
static void
@@ -1632,11 +1632,11 @@ NS(test_main)(void *arg)
}
const node_t *
-NS(node_get_by_nickname)(const char *nickname, int warn_if_unused)
+NS(node_get_by_nickname)(const char *nickname, unsigned flags)
{
CALLED(node_get_by_nickname)++;
tt_str_op(nickname, OP_EQ, NS(mock_nickname));
- tt_int_op(warn_if_unused, OP_EQ, 1);
+ tt_uint_op(flags, OP_EQ, 0);
done:
return NULL;
@@ -1651,7 +1651,7 @@ NS(node_get_by_nickname)(const char *nickname, int warn_if_unused)
*/
NS_DECL(const node_t *, node_get_by_nickname,
- (const char *nickname, int warn_if_unused));
+ (const char *nickname, unsigned flags));
static const char *NS(mock_nickname);
static node_t NS(mock_node);
@@ -1683,11 +1683,11 @@ NS(test_main)(void *arg)
}
const node_t *
-NS(node_get_by_nickname)(const char *nickname, int warn_if_unused)
+NS(node_get_by_nickname)(const char *nickname, unsigned flags)
{
CALLED(node_get_by_nickname)++;
tt_str_op(nickname, OP_EQ, NS(mock_nickname));
- tt_int_op(warn_if_unused, OP_EQ, 1);
+ tt_int_op(flags, OP_EQ, 0);
done:
return &NS(mock_node);
@@ -1701,7 +1701,7 @@ NS(node_get_by_nickname)(const char *nickname, int warn_if_unused)
*/
NS_DECL(const node_t *, node_get_by_nickname,
- (const char *nickname, int warn_if_unused));
+ (const char *nickname, unsigned flags));
static char *NS(mock_nickname);
static node_t NS(mock_node);
@@ -1735,11 +1735,11 @@ NS(test_main)(void *arg)
}
const node_t *
-NS(node_get_by_nickname)(const char *nickname, int warn_if_unused)
+NS(node_get_by_nickname)(const char *nickname, unsigned flags)
{
CALLED(node_get_by_nickname)++;
tt_str_op(nickname, OP_EQ, NS(mock_nickname));
- tt_int_op(warn_if_unused, OP_EQ, 1);
+ tt_int_op(flags, OP_EQ, 0);
done:
return &NS(mock_node);
More information about the tor-commits
mailing list