Use nodefamily_t in microdescriptors.

Closes ticket 27359.

changes/ticket27359

@@ -0,0 +1,3 @@
+  o Minor features (memory usage):
+    - Store microdescriptor family lists with a more compact representation
+      to save memory.  Closes ticket 27359.

src/feature/dirparse/microdesc_parse.c

@@ -18,6 +18,7 @@
 #include "feature/dirparse/routerparse.h"
 #include "feature/nodelist/microdesc.h"
 #include "feature/nodelist/nickname.h"
+#include "feature/nodelist/nodefamily.h"
 #include "feature/relay/router.h"
 #include "lib/crypt_ops/crypto_curve25519.h"
 #include "lib/crypt_ops/crypto_ed25519.h"
@@ -32,7 +33,7 @@ static token_rule_t microdesc_token_table[] = {
   T01("ntor-onion-key",        K_ONION_KEY_NTOR,   GE(1),       NO_OBJ ),
   T0N("id",                    K_ID,               GE(2),       NO_OBJ ),
   T0N("a",                     K_A,                GE(1),       NO_OBJ ),
-  T01("family",                K_FAMILY,           ARGS,        NO_OBJ ),
+  T01("family",                K_FAMILY,           CONCAT_ARGS, NO_OBJ ),
   T01("p",                     K_P,                CONCAT_ARGS, NO_OBJ ),
   T01("p6",                    K_P6,               CONCAT_ARGS, NO_OBJ ),
   A01("@last-listed",          A_LAST_LISTED,      CONCAT_ARGS, NO_OBJ ),
@@ -222,16 +223,9 @@ microdescs_parse_from_string(const char *s, const char *eos,
     }
 
     if ((tok = find_opt_by_keyword(tokens, K_FAMILY))) {
-      int i;
-      md->family = smartlist_new();
-      for (i=0;i<tok->n_args;++i) {
-        if (!is_legal_nickname_or_hexdigest(tok->args[i])) {
-          log_warn(LD_DIR, "Illegal nickname %s in family line",
-                   escaped(tok->args[i]));
-          goto next;
-        }
-        smartlist_add_strdup(md->family, tok->args[i]);
-      }
+      md->family = nodefamily_parse(tok->args[0],
+                                    NULL,
+                                    NF_WARN_MALFORMED);
     }
 
     if ((tok = find_opt_by_keyword(tokens, K_P))) {

src/feature/nodelist/microdesc.c

@@ -23,6 +23,7 @@
 #include "feature/nodelist/dirlist.h"
 #include "feature/nodelist/microdesc.h"
 #include "feature/nodelist/networkstatus.h"
+#include "feature/nodelist/nodefamily.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerlist.h"
 #include "feature/relay/router.h"
@@ -882,10 +883,7 @@ microdesc_free_(microdesc_t *md, const char *fname, int lineno)
   if (md->body && md->saved_location != SAVED_IN_CACHE)
     tor_free(md->body);
 
-  if (md->family) {
-    SMARTLIST_FOREACH(md->family, char *, cp, tor_free(cp));
-    smartlist_free(md->family);
-  }
+  nodefamily_free(md->family);
   short_policy_free(md->exit_policy);
   short_policy_free(md->ipv6_exit_policy);
 

src/feature/nodelist/microdesc_st.h

@@ -9,6 +9,7 @@
 
 struct curve25519_public_key_t;
 struct ed25519_public_key_t;
+struct nodefamily_t;
 struct short_policy_t;
 
 /** A microdescriptor is the smallest amount of information needed to build a
@@ -69,8 +70,8 @@ struct microdesc_t {
   tor_addr_t ipv6_addr;
   /** As routerinfo_t.ipv6_orport */
   uint16_t ipv6_orport;
-  /** As routerinfo_t.family */
-  smartlist_t *family;
+  /** As routerinfo_t.family, with readable members parsed. */
+  struct nodefamily_t *family;
   /** IPv4 exit policy summary */
   struct short_policy_t *exit_policy;
   /** IPv6 exit policy summary */

src/feature/nodelist/nodelist.c

@@ -59,6 +59,7 @@
 #include "feature/nodelist/microdesc.h"
 #include "feature/nodelist/networkstatus.h"
 #include "feature/nodelist/node_select.h"
+#include "feature/nodelist/nodefamily.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerlist.h"
 #include "feature/nodelist/routerset.h"
@@ -1504,19 +1505,6 @@ node_is_me(const node_t *node)
   return router_digest_is_me(node->identity);
 }
 
-/** Return <b>node</b> declared family (as a list of names), or NULL if
- * the node didn't declare a family. */
-const smartlist_t *
-node_get_declared_family(const node_t *node)
-{
-  if (node->ri && node->ri->declared_family)
-    return node->ri->declared_family;
-  else if (node->md && node->md->family)
-    return node->md->family;
-  else
-    return NULL;
-}
-
 /* Does this node have a valid IPv6 address?
  * Prefer node_has_ipv6_orport() or node_has_ipv6_dirport() for
  * checking specific ports. */
@@ -1905,6 +1893,61 @@ node_in_nickname_smartlist(const smartlist_t *lst, const node_t *node)
   return 0;
 }
 
+/** Return true iff n1's declared family contains n2. */
+static int
+node_family_contains(const node_t *n1, const node_t *n2)
+{
+  if (n1->ri && n1->ri->declared_family) {
+    return node_in_nickname_smartlist(n1->ri->declared_family, n2);
+  } else if (n1->md) {
+    return nodefamily_contains_node(n1->md->family, n2);
+  } else {
+    return 0;
+  }
+}
+
+/**
+ * Return true iff <b>node</b> has declared a nonempty family.
+ **/
+static bool
+node_has_declared_family(const node_t *node)
+{
+  if (node->ri && node->ri->declared_family &&
+      smartlist_len(node->ri->declared_family)) {
+    return true;
+  }
+
+  if (node->md && node->md->family) {
+    return true;
+  }
+
+  return false;
+}
+
+/**
+ * Add to <b>out</b> every node_t that is listed by <b>node</b> as being in
+ * its family.  (Note that these nodes are not in node's family unless they
+ * also agree that node is in their family.)
+ **/
+static void
+node_lookup_declared_family(smartlist_t *out, const node_t *node)
+{
+  if (node->ri && node->ri->declared_family &&
+      smartlist_len(node->ri->declared_family)) {
+    SMARTLIST_FOREACH_BEGIN(node->ri->declared_family, const char *, name) {
+      const node_t *n2 = node_get_by_nickname(name, NNF_NO_WARN_UNNAMED);
+      if (n2) {
+        smartlist_add(out, (node_t *)n2);
+      }
+    } SMARTLIST_FOREACH_END(name);
+    return;
+  }
+
+  if (node->md && node->md->family) {
+    nodefamily_add_nodes_to_smartlist(node->md->family, out);
+  }
+}
+
 /** Return true iff r1 and r2 are in the same family, but not the same
  * router. */
 int
@@ -1922,14 +1965,9 @@ nodes_in_same_family(const node_t *node1, const node_t *node2)
   }
 
   /* Are they in the same family because the agree they are? */
-  {
-    const smartlist_t *f1, *f2;
-    f1 = node_get_declared_family(node1);
-    f2 = node_get_declared_family(node2);
-    if (f1 && f2 &&
-        node_in_nickname_smartlist(f1, node2) &&
-        node_in_nickname_smartlist(f2, node1))
-      return 1;
+  if (node_family_contains(node1, node2) &&
+      node_family_contains(node2, node1)) {
+    return 1;
   }
 
   /* Are they in the same option because the user says they are? */
@@ -1957,13 +1995,10 @@ void
 nodelist_add_node_and_family(smartlist_t *sl, const node_t *node)
 {
   const smartlist_t *all_nodes = nodelist_get_list();
-  const smartlist_t *declared_family;
   const or_options_t *options = get_options();
 
   tor_assert(node);
 
-  declared_family = node_get_declared_family(node);
-
   /* Let's make sure that we have the node itself, if it's a real node. */
   {
     const node_t *real_node = node_get_by_id(node->identity);
@@ -1984,25 +2019,20 @@ nodelist_add_node_and_family(smartlist_t *sl, const node_t *node)
     } SMARTLIST_FOREACH_END(node2);
   }
 
-  /* Now, add all nodes in the declared_family of this node, if they
+  /* Now, add all nodes in the declared family of this node, if they
    * also declare this node to be in their family. */
-  if (declared_family) {
+  if (node_has_declared_family(node)) {
+    smartlist_t *declared_family = smartlist_new();
+    node_lookup_declared_family(declared_family, node);
+
     /* Add every r such that router declares familyness with node, and node
      * declares familyhood with router. */
-    SMARTLIST_FOREACH_BEGIN(declared_family, const char *, name) {
-      const node_t *node2;
-      const smartlist_t *family2;
-      if (!(node2 = node_get_by_nickname(name, NNF_NO_WARN_UNNAMED)))
-        continue;
-      if (!(family2 = node_get_declared_family(node2)))
-        continue;
-      SMARTLIST_FOREACH_BEGIN(family2, const char *, name2) {
-          if (node_nickname_matches(node, name2)) {
-            smartlist_add(sl, (void*)node2);
-            break;
-          }
-      } SMARTLIST_FOREACH_END(name2);
-    } SMARTLIST_FOREACH_END(name);
+    SMARTLIST_FOREACH_BEGIN(declared_family, const node_t *, node2) {
+      if (node_family_contains(node2, node)) {
+        smartlist_add(sl, (void*)node2);
+      }
+    } SMARTLIST_FOREACH_END(node2);
+    smartlist_free(declared_family);
   }
 
   /* If the user declared any families locally, honor those too. */

src/feature/nodelist/nodelist.h

@@ -68,7 +68,6 @@ const char *node_get_platform(const node_t *node);
 uint32_t node_get_prim_addr_ipv4h(const node_t *node);
 void node_get_address_string(const node_t *node, char *cp, size_t len);
 long node_get_declared_uptime(const node_t *node);
-const smartlist_t *node_get_declared_family(const node_t *node);
 const struct ed25519_public_key_t *node_get_ed25519_id(const node_t *node);
 int node_ed25519_id_matches(const node_t *node,
                             const struct ed25519_public_key_t *id);

src/test/test_microdesc.c

@@ -11,6 +11,7 @@
 #include "feature/dirparse/routerparse.h"
 #include "feature/nodelist/microdesc.h"
 #include "feature/nodelist/networkstatus.h"
+#include "feature/nodelist/nodefamily.h"
 #include "feature/nodelist/routerlist.h"
 #include "feature/nodelist/torcert.h"
 
@@ -70,6 +71,7 @@ test_md_cache(void *data)
   const char *test_md3_noannotation = strchr(test_md3, '\n')+1;
   time_t time1, time2, time3;
   char *fn = NULL, *s = NULL;
+  char *encoded_family = NULL;
   (void)data;
 
   options = get_options_mutable();
@@ -172,8 +174,9 @@ test_md_cache(void *data)
 
   tt_ptr_op(md1->family, OP_EQ, NULL);
   tt_ptr_op(md3->family, OP_NE, NULL);
-  tt_int_op(smartlist_len(md3->family), OP_EQ, 3);
-  tt_str_op(smartlist_get(md3->family, 0), OP_EQ, "nodeX");
+
+  encoded_family = nodefamily_format(md3->family);
+  tt_str_op(encoded_family, OP_EQ, "nodeX nodeY nodeZ");
 
   /* Now rebuild the cache! */
   tt_int_op(microdesc_cache_rebuild(mc, 1), OP_EQ, 0);
@@ -254,6 +257,7 @@ test_md_cache(void *data)
   smartlist_free(wanted);
   tor_free(s);
   tor_free(fn);
+  tor_free(encoded_family);
 }
 
 static const char truncated_md[] =