Remove orconn_ext_or_id_map and related functions

changes/ticket33383

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Remove the orconn_ext_or_id_map structure and related functions. Nothing
+      outside of unit tests looks up anything in this structure. Closes ticket
+      33383. Patch by Neel Chauhan.

src/core/mainloop/connection.c

@@ -948,7 +948,6 @@ connection_free_minimal(connection_t *conn)
     connection_or_clear_identity(TO_OR_CONN(conn));
   }
   if (conn->type == CONN_TYPE_OR || conn->type == CONN_TYPE_EXT_OR) {
-    connection_or_remove_from_ext_or_id_map(TO_OR_CONN(conn));
     tor_free(TO_OR_CONN(conn)->ext_or_conn_id);
     tor_free(TO_OR_CONN(conn)->ext_or_auth_correct_client_hash);
     tor_free(TO_OR_CONN(conn)->ext_or_transport);
@@ -5824,7 +5823,6 @@ connection_free_all(void)
 
   /* Unlink everything from the identity map. */
   connection_or_clear_identity_map();
-  connection_or_clear_ext_or_id_map();
 
   /* Clear out our list of broken connections */
   clear_broken_connection_map(0);

src/feature/relay/ext_orport.c

@@ -656,75 +656,17 @@ connection_ext_or_start_auth(or_connection_t *or_conn)
   return 0;
 }
 
-/** Global map between Extended ORPort identifiers and OR
- *  connections. */
-static digestmap_t *orconn_ext_or_id_map = NULL;
-
-/** Remove the Extended ORPort identifier of <b>conn</b> from the
- *  global identifier list. Also, clear the identifier from the
- *  connection itself. */
-void
-connection_or_remove_from_ext_or_id_map(or_connection_t *conn)
-{
-  or_connection_t *tmp;
-  if (!orconn_ext_or_id_map)
-    return;
-  if (!conn->ext_or_conn_id)
-    return;
-
-  tmp = digestmap_remove(orconn_ext_or_id_map, conn->ext_or_conn_id);
-  if (!tor_digest_is_zero(conn->ext_or_conn_id))
-    tor_assert(tmp == conn);
-
-  memset(conn->ext_or_conn_id, 0, EXT_OR_CONN_ID_LEN);
-}
-
-#ifdef TOR_UNIT_TESTS
-/** Return the connection whose ext_or_id is <b>id</b>. Return NULL if no such
- * connection is found. */
-or_connection_t *
-connection_or_get_by_ext_or_id(const char *id)
-{
-  if (!orconn_ext_or_id_map)
-    return NULL;
-  return digestmap_get(orconn_ext_or_id_map, id);
-}
-#endif /* defined(TOR_UNIT_TESTS) */
-
-/** Deallocate the global Extended ORPort identifier list */
-void
-connection_or_clear_ext_or_id_map(void)
-{
-  digestmap_free(orconn_ext_or_id_map, NULL);
-  orconn_ext_or_id_map = NULL;
-}
-
 /** Creates an Extended ORPort identifier for <b>conn</b> and deposits
  *  it into the global list of identifiers. */
 void
 connection_or_set_ext_or_identifier(or_connection_t *conn)
 {
   char random_id[EXT_OR_CONN_ID_LEN];
-  or_connection_t *tmp;
-
-  if (!orconn_ext_or_id_map)
-    orconn_ext_or_id_map = digestmap_new();
-
-  /* Remove any previous identifiers: */
-  if (conn->ext_or_conn_id && !tor_digest_is_zero(conn->ext_or_conn_id))
-      connection_or_remove_from_ext_or_id_map(conn);
-
-  do {
-    crypto_rand(random_id, sizeof(random_id));
-  } while (digestmap_get(orconn_ext_or_id_map, random_id));
 
   if (!conn->ext_or_conn_id)
     conn->ext_or_conn_id = tor_malloc_zero(EXT_OR_CONN_ID_LEN);
 
   memcpy(conn->ext_or_conn_id, random_id, EXT_OR_CONN_ID_LEN);
-
-  tmp = digestmap_set(orconn_ext_or_id_map, random_id, conn);
-  tor_assert(!tmp);
 }
 
 /** Free any leftover allocated memory of the ext_orport.c subsystem. */

src/feature/relay/ext_orport.h

@@ -36,8 +36,6 @@
 int connection_ext_or_start_auth(or_connection_t *or_conn);
 
 void connection_or_set_ext_or_identifier(or_connection_t *conn);
-void connection_or_remove_from_ext_or_id_map(or_connection_t *conn);
-void connection_or_clear_ext_or_id_map(void);
 int connection_ext_or_finished_flushing(or_connection_t *conn);
 int connection_ext_or_process_inbuf(or_connection_t *or_conn);
 char *get_ext_or_auth_cookie_file_name(void);
@@ -71,10 +69,6 @@ connection_ext_or_process_inbuf(or_connection_t *conn)
 }
 #define connection_or_set_ext_or_identifier(conn) \
   ((void)(conn))
-#define connection_or_remove_from_ext_or_id_map(conn) \
-  ((void)(conn))
-#define connection_or_clear_ext_or_id_map() \
-  STMT_NIL
 
 #define get_ext_or_auth_cookie_file_name() \
   (NULL)
@@ -94,7 +88,6 @@ STATIC int handle_client_auth_nonce(const char *client_nonce,
 #ifdef TOR_UNIT_TESTS
 extern uint8_t *ext_or_auth_cookie;
 extern int ext_or_auth_cookie_is_set;
-or_connection_t *connection_or_get_by_ext_or_id(const char *id);
 #endif
 #endif /* defined(EXT_ORPORT_PRIVATE) */
 

src/test/test_extorport.c

@@ -24,60 +24,6 @@
 #include <sys/stat.h>
 #endif
 
-/* Test connection_or_remove_from_ext_or_id_map and
- * connection_or_set_ext_or_identifier */
-static void
-test_ext_or_id_map(void *arg)
-{
-  or_connection_t *c1 = NULL, *c2 = NULL, *c3 = NULL;
-  char *idp = NULL, *idp2 = NULL;
-  (void)arg;
-
-  /* pre-initialization */
-  tt_ptr_op(NULL, OP_EQ,
-            connection_or_get_by_ext_or_id("xxxxxxxxxxxxxxxxxxxx"));
-
-  c1 = or_connection_new(CONN_TYPE_EXT_OR, AF_INET);
-  c2 = or_connection_new(CONN_TYPE_EXT_OR, AF_INET);
-  c3 = or_connection_new(CONN_TYPE_OR, AF_INET);
-
-  tt_ptr_op(c1->ext_or_conn_id, OP_NE, NULL);
-  tt_ptr_op(c2->ext_or_conn_id, OP_NE, NULL);
-  tt_ptr_op(c3->ext_or_conn_id, OP_EQ, NULL);
-
-  tt_ptr_op(c1, OP_EQ, connection_or_get_by_ext_or_id(c1->ext_or_conn_id));
-  tt_ptr_op(c2, OP_EQ, connection_or_get_by_ext_or_id(c2->ext_or_conn_id));
-  tt_ptr_op(NULL, OP_EQ,
-            connection_or_get_by_ext_or_id("xxxxxxxxxxxxxxxxxxxx"));
-
-  idp = tor_memdup(c2->ext_or_conn_id, EXT_OR_CONN_ID_LEN);
-
-  /* Give c2 a new ID. */
-  connection_or_set_ext_or_identifier(c2);
-  tt_mem_op(idp, OP_NE, c2->ext_or_conn_id, EXT_OR_CONN_ID_LEN);
-  idp2 = tor_memdup(c2->ext_or_conn_id, EXT_OR_CONN_ID_LEN);
-  tt_assert(!tor_digest_is_zero(idp2));
-
-  tt_ptr_op(NULL, OP_EQ, connection_or_get_by_ext_or_id(idp));
-  tt_ptr_op(c2, OP_EQ, connection_or_get_by_ext_or_id(idp2));
-
-  /* Now remove it. */
-  connection_or_remove_from_ext_or_id_map(c2);
-  tt_ptr_op(NULL, OP_EQ, connection_or_get_by_ext_or_id(idp));
-  tt_ptr_op(NULL, OP_EQ, connection_or_get_by_ext_or_id(idp2));
-
- done:
-  if (c1)
-    connection_free_minimal(TO_CONN(c1));
-  if (c2)
-    connection_free_minimal(TO_CONN(c2));
-  if (c3)
-    connection_free_minimal(TO_CONN(c3));
-  tor_free(idp);
-  tor_free(idp2);
-  connection_or_clear_ext_or_id_map();
-}
-
 /* Simple connection_write_to_buf_impl_ replacement that unconditionally
  * writes to outbuf. */
 static void
@@ -581,7 +527,6 @@ test_ext_or_handshake(void *arg)
 }
 
 struct testcase_t extorport_tests[] = {
-  { "id_map", test_ext_or_id_map, TT_FORK, NULL, NULL },
   { "write_command", test_ext_or_write_command, TT_FORK, NULL, NULL },
   { "init_auth", test_ext_or_init_auth, TT_FORK, NULL, NULL },
   { "cookie_auth", test_ext_or_cookie_auth, TT_FORK, NULL, NULL },