r16263@catbus: nickm | 2007-10-29 15:08:17 -0400

 Tidy last patch a bit.


svn:r12273

ChangeLog

@@ -8,7 +8,9 @@ Changes in version 0.2.0.10-alpha - 2007-1?-??
       we can make anonymized begin_dir connections for (e.g.) more secure
       hidden service posting and fetching.
     - Code to parse and generate new hidden service descriptor format
-      (From Karsten Loesing.)
+      (From Karsten Loesing).
+    - Code to cache and download new hidden service descriptor format
+      (From Karsten Loesing).
 
   o Major bugfixes:
     - Stop servers from crashing if they set a Family option (or

src/or/config.c

@@ -118,8 +118,6 @@ typedef struct config_var_t {
 /** An entry for config_vars: "The option <b>name</b> is obsolete." */
 #define OBSOLETE(name) { name, CONFIG_TYPE_OBSOLETE, 0, NULL }
 
-
-
 /** Array of configuration options.  Until we disallow nonstandard
  * abbreviations, order is significant, since the first matching option will
  * be chosen first.
@@ -287,13 +285,12 @@ static config_var_t _option_vars[] = {
   VAR("VersioningAuthoritativeDirectory",BOOL,VersioningAuthoritativeDir, "0"),
   V(VirtualAddrNetwork,          STRING,   "127.192.0.0/10"),
   VAR("__AllDirActionsPrivate",  BOOL,  AllDirActionsPrivate,     "0"),
-  VAR("__ConsiderAllRoutersAsHidServDirectories", BOOL,
-      __ConsiderAllRoutersAsHidServDirectories, "0"),
+  /*XXXX020 for testing.  Maybe remove before -rc. */
+  V(__ConsiderAllRoutersAsHidServDirectories, BOOL, "0"),
   VAR("__DisablePredictedCircuits",BOOL,DisablePredictedCircuits, "0"),
   VAR("__LeaveStreamsUnattached",BOOL,  LeaveStreamsUnattached,   "0"),
-  VAR("__MinUptimeHidServDirectoryV2", INTERVAL,
-      __MinUptimeHidServDirectoryV2, "24 hours"),
-
+  /*XXXX020 for testing.  Maybe remove before -rc. */
+  V(__MinUptimeHidServDirectoryV2, INTERVAL, "24 hours"),
   { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }
 };
 #undef VAR

src/or/directory.c

@@ -1716,6 +1716,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
           log_warn(LD_REND,"Failed to fetch rendezvous descriptor.");
           /* alice's ap_stream will notice when connection_mark_for_close
            * cleans it up */
+          /*XXXX020 maybe retry quickly; timeout takes a while. */
         } else {
           /* success. notify pending connections about this. */
           conn->_base.purpose = DIR_PURPOSE_HAS_FETCHED_RENDDESC;
@@ -1725,6 +1726,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
       case 404:
         /* not there. pending connections will be notified when
          * connection_mark_for_close cleans it up. */
+        /*XXXX020 maybe retry quickly; timeout takes a while. */
         break;
       case 400:
         log_warn(LD_REND,
@@ -1746,7 +1748,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
              (int)body_len, status_code, escaped(reason));
     switch (status_code) {
       case 200:
-        if (rend_cache_store_v2_client(body, NULL) < 0) {
+        if (rend_cache_store_v2_desc_as_client(body, NULL) < 0) {
           log_warn(LD_REND,"Fetching v2 rendezvous descriptor failed.");
           /* alice's ap_stream will notice when connection_mark_for_close
            * cleans it up */
@@ -2488,12 +2490,12 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers,
   if (options->HidServDirectoryV2 &&
        !strcmpstart(url,"/tor/rendezvous2/")) {
     /* Handle v2 rendezvous descriptor fetch request. */
-    char *descp;
+    const char *descp;
     const char *query = url + strlen("/tor/rendezvous2/");
     if (strlen(query) == REND_DESC_ID_V2_BASE32) {
       log_info(LD_REND, "Got a v2 rendezvous descriptor request for ID '%s'",
                query);
-      switch (rend_cache_lookup_v2_dir(query, &descp)) {
+      switch (rend_cache_lookup_v2_desc(query, &descp)) {
         case 1: /* valid */
           write_http_response_header(conn, strlen(descp), 0, 0);
           connection_write_to_buf(descp, strlen(descp), TO_CONN(conn));
@@ -2638,7 +2640,7 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers,
   /* Handle v2 rendezvous service publish request. */
   if (options->HidServDirectoryV2 &&
       !strcmpstart(url,"/tor/rendezvous2/publish")) {
-    if (rend_cache_store_v2_dir(body) < 0) {
+    if (rend_cache_store_v2_desc_as_dir(body) < 0) {
       log_warn(LD_REND, "Rejected rend descriptor (length %d) from %s.",
              (int)body_len, conn->_base.address);
       write_http_status_line(conn, 400, "Invalid service descriptor rejected");
@@ -3052,17 +3054,18 @@ dir_split_resource_into_fingerprints(const char *resource,
  * REND_NUMBER_OF_NON_CONSECUTIVE_REPLICAS entries; <b>service_id</b> and
  * <b>seconds_valid</b> are only passed for logging purposes.*/
 /* XXXX020 enable tunneling when available!! */
+/* XXXX020 desc_ids and desc_strs could be merged.  Should they? */
 void
 directory_post_to_hs_dir(smartlist_t *desc_ids, smartlist_t *desc_strs,
                          const char *service_id, int seconds_valid,
-                         smartlist_t *hs_dirs_)
+                         smartlist_t *hs_dirs)
 {
   int i, j;
   smartlist_t *responsible_dirs;
   routerinfo_t *hs_dir;
   if (smartlist_len(desc_ids) != REND_NUMBER_OF_NON_CONSECUTIVE_REPLICAS ||
       smartlist_len(desc_strs) != REND_NUMBER_OF_NON_CONSECUTIVE_REPLICAS) {
-    log_warn(LD_REND, "Could not post descriptors to hidden service "
+    log_warn(LD_BUG, "Could not post descriptors to hidden service "
                       "directories: Illegal number of descriptor "
                       "IDs/strings");
     return;
@@ -3073,12 +3076,14 @@ directory_post_to_hs_dir(smartlist_t *desc_ids, smartlist_t *desc_strs,
     const char *desc_str = smartlist_get(desc_strs, i);
     /* Determine responsible dirs. */
     if (hid_serv_get_responsible_directories(responsible_dirs, desc_id,
-                                             hs_dirs_) < 0) {
+                                             hs_dirs) < 0) {
       log_warn(LD_REND, "Could not determine the responsible hidden service "
                         "directories to post descriptors to.");
       smartlist_free(responsible_dirs);
       return;
     }
+    tor_assert(smartlist_len(responsible_dirs) ==
+               REND_NUMBER_OF_CONSECUTIVE_REPLICAS);
     for (j = 0; j < REND_NUMBER_OF_CONSECUTIVE_REPLICAS; j++) {
       char desc_id_base32[REND_DESC_ID_V2_BASE32 + 1];
       hs_dir = smartlist_get(responsible_dirs, j);
@@ -3089,7 +3094,7 @@ directory_post_to_hs_dir(smartlist_t *desc_ids, smartlist_t *desc_strs,
                                  DIR_PURPOSE_UPLOAD_RENDDESC_V2,
                                  ROUTER_PURPOSE_GENERAL,
                                  1, NULL, desc_str, strlen(desc_str), 0);
-      base32_encode(desc_id_base32, REND_DESC_ID_V2_BASE32 + 1,
+      base32_encode(desc_id_base32, sizeof(desc_id_base32),
                     desc_id, DIGEST_LEN);
       log_info(LD_REND, "Sending publish request for v2 descriptor for "
                         "service '%s' with descriptor ID '%s' with validity "
@@ -3112,25 +3117,28 @@ directory_post_to_hs_dir(smartlist_t *desc_ids, smartlist_t *desc_strs,
  * XXXX020 enable tunneling when available!! */
 void
 directory_get_from_hs_dir(const char *desc_id, const char *query,
-                          smartlist_t *hs_dirs_)
+                          smartlist_t *hs_dirs)
 {
   smartlist_t *responsible_dirs = smartlist_create();
   routerinfo_t *hs_dir;
   char desc_id_base32[REND_DESC_ID_V2_BASE32 + 1];
-  int replica;
   tor_assert(desc_id);
   tor_assert(query);
   tor_assert(strlen(query) == REND_SERVICE_ID_LEN);
   /* Determine responsible dirs. */
   if (hid_serv_get_responsible_directories(responsible_dirs, desc_id,
-                                           hs_dirs_) < 0) {
+                                           hs_dirs) < 0) {
     log_warn(LD_REND, "Could not determine the responsible hidden service "
                       "directories to fetch descriptors.");
     smartlist_free(responsible_dirs);
     return;
   }
-  replica = crypto_rand_int(REND_NUMBER_OF_CONSECUTIVE_REPLICAS);
-  hs_dir = smartlist_get(responsible_dirs, replica);
+  hs_dir = smartlist_choose(responsible_dirs);
+  smartlist_free(responsible_dirs);
+  if (!hs_dir) {
+    /*XXXX020 log. */
+    return;
+  }
   /* XXXX020 if hsdir fails, use another one... */
   base32_encode(desc_id_base32, REND_DESC_ID_V2_BASE32 + 1,
                 desc_id, DIGEST_LEN);

src/or/or.h

@@ -1211,11 +1211,10 @@ typedef struct {
   unsigned int is_exit:1; /**< Do we think this is an OK exit? */
   unsigned int is_bad_exit:1; /**< Do we think this exit is censored, borked,
                                * or otherwise nasty? */
-  unsigned int wants_to_be_hs_dir:1; /**< True iff this router has set a flag
-                                         to possibly act as hidden service
-                                         directory. */
+  unsigned int wants_to_be_hs_dir:1; /**< True iff this router claims to be
+                                      * a hidden service directory. */
   unsigned int is_hs_dir:1; /**< True iff this router is a hidden service
-                             * directory. */
+                             * directory according to the authorities. */
 
 /** Tor can use this router for general positions in circuits. */
 #define ROUTER_PURPOSE_GENERAL 0
@@ -2020,6 +2019,10 @@ typedef struct {
   int FetchServerDescriptors; /**< Do we fetch server descriptors as normal? */
   int FetchHidServDescriptors; /** and hidden service descriptors? */
   int HidServDirectoryV2; /**< Do we act as hs dir? */
+
+  /*XXXX020 maybe remove these next two testing options.  DEFINITELY rename
+   * them at some point, since I think C says that identifiers beginning with
+   * __ are implementation-reserved or something. */
   int __MinUptimeHidServDirectoryV2; /**< Accept hs dirs after what time? */
   int __ConsiderAllRoutersAsHidServDirectories; /**< Consider all routers as
                                                  * hidden service dirs? */
@@ -3452,6 +3455,9 @@ typedef struct rend_cache_entry_t {
 } rend_cache_entry_t;
 
 void rend_cache_init(void);
+/*XXXX020 clean *and* clean_up *and* clean_v2_dir? Rename some. */
+/*XXXX020 Call clean_up and clean_v2_dir from somewhere; nothing calls them
+ * now. */
 void rend_cache_clean(void);
 void rend_cache_clean_up(void);
 void rend_cache_clean_v2_dir(void);
@@ -3461,11 +3467,11 @@ int rend_cache_lookup_desc(const char *query, int version, const char **desc,
                            size_t *desc_len);
 int rend_cache_lookup_entry(const char *query, int version,
                             rend_cache_entry_t **entry_out);
-int rend_cache_lookup_v2_dir(const char *query, char **desc);
+int rend_cache_lookup_v2_desc(const char *query, const char **desc);
 int rend_cache_store(const char *desc, size_t desc_len, int published);
-int rend_cache_store_v2_client(const char *desc,
+int rend_cache_store_v2_desc_as_client(const char *desc,
                                const char *descriptor_cookie);
-int rend_cache_store_v2_dir(const char *desc);
+int rend_cache_store_v2_desc_as_dir(const char *desc);
 int rend_cache_size(void);
 int rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
                                smartlist_t *desc_ids_out,
@@ -3740,15 +3746,15 @@ const char *esc_router_info(routerinfo_t *router);
 void routers_sort_by_identity(smartlist_t *routers);
 
 smartlist_t *hid_serv_create_routing_table(void);
-int hid_serv_have_enough_directories(smartlist_t *hs_dirs);
+int hid_serv_have_enough_directories(const smartlist_t *hs_dirs);
 int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
                                          const char *id,
-                                         smartlist_t *hs_dirs);
+                                         const smartlist_t *hs_dirs);
 routerinfo_t *hid_serv_next_directory(const char *id,
-                                      smartlist_t *hs_dirs);
+                                      const smartlist_t *hs_dirs);
 routerinfo_t *hid_serv_previous_directory(const char *id,
-                                          smartlist_t *hs_dirs);
-int hid_serv_acting_as_directory(smartlist_t *hs_dirs);
+                                          const smartlist_t *hs_dirs);
+int hid_serv_acting_as_directory(const smartlist_t *hs_dirs);
 int hid_serv_responsible_for_desc_id(const char *id, smartlist_t *hs_dirs);
 
 /********************************* routerparse.c ************************/

src/or/rendcommon.c

@@ -375,7 +375,7 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
     /* Calculate secret-id-part = h(time-period + cookie + replica). */
     get_secret_id_part_bytes(secret_id_part, time_period, descriptor_cookie,
                              k);
-    base32_encode(secret_id_part_base32, REND_SECRET_ID_PART_LEN_BASE32 + 1,
+    base32_encode(secret_id_part_base32, sizeof(secret_id_part_base32),
                   secret_id_part, DIGEST_LEN);
     /* Calculate descriptor ID. */
     desc_id = tor_malloc_zero(DIGEST_LEN);
@@ -685,10 +685,10 @@ rend_cache_clean_v2_dir(void)
   for (iter = digestmap_iter_init(rend_cache_v2_dir);
        !digestmap_iter_done(iter); ) {
     digestmap_iter_get(iter, &key, &val);
-    ent = (rend_cache_entry_t*)val;
+    ent = val;
     if (ent->parsed->timestamp < cutoff) {
       char key_base32[REND_DESC_ID_V2_BASE32 + 1];
-      base32_encode(key_base32, REND_DESC_ID_V2_BASE32 + 1, key, DIGEST_LEN);
+      base32_encode(key_base32, sizeof(key_base32), key, DIGEST_LEN);
       log_info(LD_REND, "Removing descriptor with ID '%s' from cache, "
                         "because it is too old!",
                key_base32);
@@ -707,23 +707,28 @@ rend_cache_clean_v2_dir(void)
 int
 rend_id_is_in_interval(const char *a, const char *b, const char *c)
 {
+  int a_b, b_c, c_a;
   tor_assert(a);
   tor_assert(b);
   tor_assert(c);
+
   /* There are five cases in which a is outside the interval ]b,c]: */
-  if ((memcmp(a, b, DIGEST_LEN) == 0) || /* 1. a == b (b is excluded) */
-      /* 2. b == c (interval is empty) */
-      (memcmp(b, c, DIGEST_LEN) == 0) ||
-      /* 3. a b c */
-      (memcmp(a, b, DIGEST_LEN) <= 0 && memcmp(b, c, DIGEST_LEN) < 0) ||
-      /* 4. c a b */
-      (memcmp(c, a, DIGEST_LEN) < 0 && memcmp(a, b, DIGEST_LEN) <= 0) ||
-      /* 5. b c a */
-      (memcmp(b, c, DIGEST_LEN) < 0 && memcmp(c, a, DIGEST_LEN) < 0))
-    return 0;
-  /* In the other cases, a is inside the interval. */
-  else
-    return 1;
+  a_b = memcmp(a,b,DIGEST_LEN);
+  if (a_b == 0)
+    return 0; /* 1. a == b (b is excluded) */
+  b_c = memcmp(b,c,DIGEST_LEN);
+  if (b_c == 0)
+    return 0; /* 2. b == c (interval is empty) */
+  else if (a_b <= 0 && b_c < 0)
+    return 0; /* 3. a b c */
+  c_a = memcmp(c,a,DIGEST_LEN);
+  if (c_a < 0 && a_b <= 0)
+    return 0; /* 4. c a b */
+  else if (b_c < 0 && c_a < 0)
+    return 0; /* 5. b c a */
+
+  /* In the other cases (a c b; b a c; c b a), a is inside the interval. */
+  return 1;
 }
 
 /** Clean up all values for which this node as hidden service directory is
@@ -732,14 +737,14 @@ void
 rend_cache_clean_up(void)
 {
   digestmap_iter_t *iter;
-  const char *key;
-  void *val;
-  rend_cache_entry_t *ent;
   smartlist_t *hs_dirs = hid_serv_create_routing_table();
   for (iter = digestmap_iter_init(rend_cache_v2_dir);
        !digestmap_iter_done(iter); ) {
+    const char *key;
+    void *val;
+    rend_cache_entry_t *ent;
     digestmap_iter_get(iter, &key, &val);
-    ent = (rend_cache_entry_t*)val;
+    ent = val;
     if (!hid_serv_responsible_for_desc_id(key, hs_dirs)) {
       char key_base32[REND_DESC_ID_V2_BASE32 + 1];
       base32_encode(key_base32, REND_DESC_ID_V2_BASE32 + 1, key, DIGEST_LEN);
@@ -809,10 +814,11 @@ rend_cache_lookup_desc(const char *query, int version, const char **desc,
 }
 
 /** Lookup the v2 service descriptor with base32-encoded <b>desc_id</b> and
- * copy the pointer to it to <b>desc</b>.
+ * copy the pointer to it to *<b>desc</b>.  Return 1 on success, 0 on
+ * well-formed-but-not-found, and -1 on failure.
  */
 int
-rend_cache_lookup_v2_dir(const char *desc_id, char **desc)
+rend_cache_lookup_v2_desc(const char *desc_id, const char **desc)
 {
   rend_cache_entry_t *e;
   char desc_id_digest[DIGEST_LEN];
@@ -835,7 +841,7 @@ rend_cache_lookup_v2_dir(const char *desc_id, char **desc)
   }
   smartlist_free(hs_dirs);
   /* Lookup descriptor and return. */
-  e = (rend_cache_entry_t*) digestmap_get(rend_cache_v2_dir, desc_id_digest);
+  e = digestmap_get(rend_cache_v2_dir, desc_id_digest);
   if (e) {
     *desc = e->desc;
     return 1;
@@ -936,7 +942,7 @@ rend_cache_store(const char *desc, size_t desc_len, int published)
  * it's novel.
  */
 int
-rend_cache_store_v2_dir(const char *desc)
+rend_cache_store_v2_desc_as_dir(const char *desc)
 {
   rend_service_descriptor_t *parsed;
   char desc_id[DIGEST_LEN];
@@ -965,7 +971,7 @@ rend_cache_store_v2_dir(const char *desc)
     /* We don't care about the introduction points. */
     tor_free(intro_content);
     /* For pretty log statements. */
-    base32_encode(desc_id_base32, REND_DESC_ID_V2_BASE32 + 1,
+    base32_encode(desc_id_base32, sizeof(desc_id_base32),
                   desc_id, DIGEST_LEN);
     /* Is desc ID in the range that we are (directly or indirectly) responsible
      * for? */
@@ -973,14 +979,12 @@ rend_cache_store_v2_dir(const char *desc)
       log_info(LD_REND, "Service descriptor with desc ID %s is not in "
                         "interval that we are responsible for.",
                desc_id_base32);
-      rend_service_descriptor_free(parsed);
       goto skip;
     }
     /* Is descriptor too old? */
     if (parsed->timestamp < now - REND_CACHE_MAX_AGE-REND_CACHE_MAX_SKEW) {
       log_info(LD_REND, "Service descriptor with desc ID %s is too old.",
                desc_id_base32);
-      rend_service_descriptor_free(parsed);
       goto skip;
     }
     /* Is descriptor too far in the future? */
@@ -988,15 +992,13 @@ rend_cache_store_v2_dir(const char *desc)
       log_info(LD_REND, "Service descriptor with desc ID %s is too far in the "
                         "future.",
                desc_id_base32);
-      rend_service_descriptor_free(parsed);
       goto skip;
     }
     /* Do we already have a newer descriptor? */
-    e = (rend_cache_entry_t *)digestmap_get(rend_cache_v2_dir, desc_id);
+    e = digestmap_get(rend_cache_v2_dir, desc_id);
     if (e && e->parsed->timestamp > parsed->timestamp) {
       log_info(LD_REND, "We already have a newer service descriptor with the "
                         "same desc ID %s and version.", desc_id_base32);
-      rend_service_descriptor_free(parsed);
       goto skip;
     }
     /* Do we already have this descriptor? */
@@ -1004,7 +1006,6 @@ rend_cache_store_v2_dir(const char *desc)
       log_info(LD_REND, "We already have this service descriptor with desc "
                         "ID %s.", desc_id_base32);
       e->received = time(NULL);
-      rend_service_descriptor_free(parsed);
       goto skip;
     }
     /* Store received descriptor. */
@@ -1017,18 +1018,20 @@ rend_cache_store_v2_dir(const char *desc)
     }
     e->received = time(NULL);
     e->parsed = parsed;
-    e->desc = tor_malloc(encoded_size + 1);
-    strlcpy(e->desc, current_desc, encoded_size + 1);
+    e->desc = tor_strndup(current_desc, encoded_size);
     e->len = encoded_size;
     log_info(LD_REND, "Successfully stored service descriptor with desc ID "
                       "'%s' and len %d.", desc_id_base32, encoded_size);
     number_stored++;
- skip:
+    goto advance;
+  skip:
+    rend_service_descriptor_free(parsed);
+  advance:
     /* advance to next descriptor, if available. */
     current_desc = next_desc;
     /* check if there is a next descriptor. */
-    if (strncmp(current_desc, "rendezvous-service-descriptor ",
-                strlen("rendezvous-service-descriptor ")))
+    if (!current_desc ||
+        strcmpstart(current_desc, "rendezvous-service-descriptor "))
       break;
   }
   log_info(LD_REND, "Parsed and added %d descriptor%s.",
@@ -1048,8 +1051,11 @@ rend_cache_store_v2_dir(const char *desc)
  * it's novel.
  */
 int
-rend_cache_store_v2_client(const char *desc, const char *descriptor_cookie)
+rend_cache_store_v2_desc_as_client(const char *desc,
+                                   const char *descriptor_cookie)
 {
+  /*XXXX this seems to have a bit of duplicate code with
+   * rend_cache_store_v2_desc_as_dir().  Fix that. */
   rend_service_descriptor_t *parsed = NULL;
   char desc_id[DIGEST_LEN];
   char *intro_content = NULL;
@@ -1067,7 +1073,7 @@ rend_cache_store_v2_client(const char *desc, const char *descriptor_cookie)
                                        &intro_size, &encoded_size,
                                        &next_desc, desc) < 0) {
     if (parsed) rend_service_descriptor_free(parsed);
-    if (intro_content) tor_free(intro_content);
+    tor_free(intro_content);
     log_warn(LD_REND, "Could not parse descriptor.");
     return -1;
   }

src/or/routerlist.c

@@ -4249,18 +4249,18 @@ routers_sort_by_identity(smartlist_t *routers)
  * NULL, or has no elements, return NULL.
  */
 routerinfo_t *
-hid_serv_next_directory(const char *id, smartlist_t *hs_dirs)
+hid_serv_next_directory(const char *id, const smartlist_t *hs_dirs)
 {
   int i;
   if (!hs_dirs) return NULL;
   if (smartlist_len(hs_dirs) == 0) return NULL;
   for (i = 0; i < smartlist_len(hs_dirs); i++) {
-    routerinfo_t *router = (routerinfo_t *) smartlist_get(hs_dirs, i);
+    routerinfo_t *router = smartlist_get(hs_dirs, i);
     if (memcmp(router->cache_info.identity_digest, id, DIGEST_LEN) > 0) {
       return router;
     }
   }
-  return (routerinfo_t *) smartlist_get(hs_dirs, 0);
+  return smartlist_get(hs_dirs, 0);
 }
 
 /** Return the first router that is acting as hidden service directory and that
@@ -4269,45 +4269,45 @@ hid_serv_next_directory(const char *id, smartlist_t *hs_dirs)
  * NULL, or has no elements, return NULL.
  */
 routerinfo_t *
-hid_serv_previous_directory(const char *id, smartlist_t *hs_dirs)
+hid_serv_previous_directory(const char *id, const smartlist_t *hs_dirs)
 {
   int i;
   if (!hs_dirs) return NULL;
   if (smartlist_len(hs_dirs) == 0) return NULL;
   for (i = smartlist_len(hs_dirs) - 1; i >= 0; i--) {
-    routerinfo_t *router = (routerinfo_t *) smartlist_get(hs_dirs, i);
+    routerinfo_t *router = smartlist_get(hs_dirs, i);
     if (memcmp(router->cache_info.identity_digest, id, DIGEST_LEN) < 0) {
       return router;
     }
   }
-  return (routerinfo_t *)
-    smartlist_get(hs_dirs, smartlist_len(hs_dirs) - 1);
+  return smartlist_get(hs_dirs, smartlist_len(hs_dirs) - 1);
 }
 
 /** Returns true, if we are aware of enough hidden service directory to
  * usefully perform v2 rend operations on them (publish, fetch, replicate),
  * or false otherwise. */
 int
-hid_serv_have_enough_directories(smartlist_t *hs_dirs)
+hid_serv_have_enough_directories(const smartlist_t *hs_dirs)
 {
   return (smartlist_len(hs_dirs) > REND_NUMBER_OF_CONSECUTIVE_REPLICAS);
 }
 
 /** Determine the REND_NUMBER_OF_CONSECUTIVE_REPLICAS routers that are
  * responsible for <b>id</b> (binary) and add pointers to those routers'
- * routerstatus_t to <b>responsible_dirs</b>. If we don't have enough
+ * routerinfo_t to <b>responsible_dirs</b>. If we don't have enough
  * hidden service directories, return -1, else 0. */
+/*XXXX020 yield routerstatus_t, not routerinfo_t! */
 int
 hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
                                      const char *id,
-                                     smartlist_t *hs_dirs)
+                                     const smartlist_t *hs_dirs)
 {
   const char *digest;
   int i;
   routerinfo_t *router;
-  char id_base32[32+1];
-  base32_encode(id_base32, REND_DESC_ID_V2_BASE32 + 1, id, DIGEST_LEN);
+  char id_base32[REND_DESC_ID_V2_BASE32+1];
   tor_assert(id);
+  base32_encode(id_base32, sizeof(id_base32), id, DIGEST_LEN);
   if (!hid_serv_have_enough_directories(hs_dirs)) {
     log_warn(LD_REND, "We don't have enough hidden service directories to "
                       "perform v2 rendezvous operations!");
@@ -4331,6 +4331,8 @@ hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
  * containing all routers that have been assigned as hidden service
  * directories by the directory authorities; this list can be used as
  * hidden service routing table. */
+/*XXXX020 using routerinfo_t here instead of routerstatus_t is error-prone.
+ * Best change that. */
 smartlist_t *
 hid_serv_create_routing_table(void)
 {
@@ -4350,7 +4352,7 @@ hid_serv_create_routing_table(void)
 /** Return true if this node is currently acting as hidden service
  * directory, false otherwise. */
 int
-hid_serv_acting_as_directory(smartlist_t *hs_dirs)
+hid_serv_acting_as_directory(const smartlist_t *hs_dirs)
 {
   routerinfo_t *me = routerlist_find_my_routerinfo();
   int found_me = 0;
@@ -4370,13 +4372,15 @@ hid_serv_acting_as_directory(smartlist_t *hs_dirs)
   }
   SMARTLIST_FOREACH(hs_dirs, routerinfo_t *, router,
   {
-    if (router_is_me(router))
+    if (router_is_me(router)) {
       found_me = 1;
+      break;
+    }
   });
   if (!found_me) {
     /* not acting as HS Dir */
     char me_base32[REND_DESC_ID_V2_BASE32 + 1];
-    base32_encode(me_base32, REND_DESC_ID_V2_BASE32 + 1,
+    base32_encode(me_base32, sizeof(me_base32),
                   me->cache_info.identity_digest, DIGEST_LEN);
     log_info(LD_REND, "We are not acting as hidden service directory, "
                       "because we are not listed as such in our own "