r11737@Kushana: nickm | 2006-12-28 18:32:13 -0500

 Remove some dead code; refactor some duplicated code.


svn:r9206

src/or/or.h

@@ -2410,6 +2410,11 @@ void clear_pending_onions(void);
 
 /********************************* policies.c ************************/
 
+/* (length of "accept 255.255.255.255/255.255.255.255:65535-65535\n" plus a
+ * nul.)
+ */
+#define POLICY_BUF_LEN 52
+
 typedef enum {
   ADDR_POLICY_ACCEPTED=0,
   ADDR_POLICY_REJECTED=-1,
@@ -2439,6 +2444,7 @@ int exit_policy_is_general_exit(addr_policy_t *policy);
 int policy_is_reject_star(addr_policy_t *policy);
 int getinfo_helper_policies(control_connection_t *conn,
                             const char *question, char **answer);
+int policy_write_item(char *buf, size_t buflen, addr_policy_t *policy);
 
 void addr_policy_free(addr_policy_t *p);
 void policies_free_all(void);

src/or/policies.c

@@ -658,6 +658,69 @@ policy_is_reject_star(addr_policy_t *p)
   return 1;
 }
 
+/** Write a single address policy to the buf_len byte buffer at buf.  Return
+ * the number of characters written, or -1 on failure. */
+int
+policy_write_item(char *buf, size_t buflen, addr_policy_t *policy)
+{
+  struct in_addr in;
+  size_t written = 0;
+  char addrbuf[INET_NTOA_BUF_LEN];
+  int result;
+
+  in.s_addr = htonl(policy->addr);
+  tor_inet_ntoa(&in, addrbuf, sizeof(addrbuf));
+  /* write accept/reject 1.2.3.4 */
+  result = tor_snprintf(buf, buflen, "%s %s",
+            policy->policy_type == ADDR_POLICY_ACCEPT ? "accept" : "reject",
+            policy->msk == 0 ? "*" : addrbuf);
+  if (result < 0)
+    return -1;
+  written += strlen(buf);
+  /* If the mask is 0xffffffff, we don't need to give it.  If the mask is 0,
+   * we already wrote "*". */
+  if (policy->msk != 0xFFFFFFFFu && policy->msk != 0) {
+    int n_bits = addr_mask_get_bits(policy->msk);
+    if (n_bits >= 0) {
+      if (tor_snprintf(buf+written, buflen-written, "/%d", n_bits)<0)
+        return -1;
+    } else {
+      /* Write "/255.255.0.0" */
+      in.s_addr = htonl(policy->msk);
+      tor_inet_ntoa(&in, addrbuf, sizeof(addrbuf));
+      if (tor_snprintf(buf+written, buflen-written, "/%s", addrbuf)<0)
+        return -1;
+    }
+    written += strlen(buf+written);
+  }
+  if (policy->prt_min <= 1 && policy->prt_max == 65535) {
+    /* There is no port set; write ":*" */
+    if (written+4 > buflen)
+      return -1;
+    strlcat(buf+written, ":*", buflen-written);
+    written += 3;
+  } else if (policy->prt_min == policy->prt_max) {
+    /* There is only one port; write ":80". */
+    result = tor_snprintf(buf+written, buflen-written, ":%d", policy->prt_min);
+    if (result<0)
+      return -1;
+    written += result;
+  } else {
+    /* There is a range of ports; write ":79-80". */
+    result = tor_snprintf(buf+written, buflen-written, ":%d-%d",
+                          policy->prt_min, policy->prt_max);
+    if (result<0)
+      return -1;
+    written += result;
+  }
+  if (written < buflen)
+    buf[written] = '\0';
+  else
+    return -1;
+
+  return (int)written;
+}
+
 int
 getinfo_helper_policies(control_connection_t *conn,
                         const char *question, char **answer)

src/or/router.c

@@ -1086,8 +1086,6 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
   char digest[DIGEST_LEN];
   char published[ISO_TIME_LEN+1];
   char fingerprint[FINGERPRINT_LEN+1];
-  struct in_addr in;
-  char addrbuf[INET_NTOA_BUF_LEN];
   size_t onion_pkeylen, identity_pkeylen;
   size_t written;
   int result=0;
@@ -1203,51 +1201,14 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     tmpe = NULL;
   }
   for ( ; tmpe; tmpe=tmpe->next) {
-    /* XXXX012 Can this move into policies.c ? */
-    /* Write: "accept 1.2.3.4" */
-    in.s_addr = htonl(tmpe->addr);
-    tor_inet_ntoa(&in, addrbuf, sizeof(addrbuf));
-    result = tor_snprintf(s+written, maxlen-written, "%s %s",
-        tmpe->policy_type == ADDR_POLICY_ACCEPT ? "accept" : "reject",
-        tmpe->msk == 0 ? "*" : addrbuf);
+    result = policy_write_item(s+written, maxlen-written, tmpe);
     if (result < 0)
       return -1;
     written += result;
-    if (tmpe->msk != 0xFFFFFFFFu && tmpe->msk != 0) {
-      int n_bits = addr_mask_get_bits(tmpe->msk);
-      if (n_bits >= 0) {
-        if (tor_snprintf(s+written, maxlen-written, "/%d", n_bits)<0)
-          return -1;
-      } else {
-        /* Write "/255.255.0.0" */
-        in.s_addr = htonl(tmpe->msk);
-        tor_inet_ntoa(&in, addrbuf, sizeof(addrbuf));
-        if (tor_snprintf(s+written, maxlen-written, "/%s", addrbuf)<0)
-          return -1;
-      }
-      written += strlen(s+written);
-    }
-    if (tmpe->prt_min <= 1 && tmpe->prt_max == 65535) {
-      /* There is no port set; write ":*" */
-      if (written+4 > maxlen)
-        return -1;
-      strlcat(s+written, ":*\n", maxlen-written);
-      written += 3;
-    } else if (tmpe->prt_min == tmpe->prt_max) {
-      /* There is only one port; write ":80". */
-      result = tor_snprintf(s+written, maxlen-written, ":%d\n", tmpe->prt_min);
-      if (result<0)
-        return -1;
-      written += result;
-    } else {
-      /* There is a range of ports; write ":79-80". */
-      result = tor_snprintf(s+written, maxlen-written, ":%d-%d\n",
-                            tmpe->prt_min, tmpe->prt_max);
-      if (result<0)
-        return -1;
-      written += result;
-    }
-  } /* end for */
+    if (written < maxlen+2)
+      return -1;
+    s[written++] = '\n';
+  }
 
   if (written+256 > maxlen) /* Not enough room for signature. */
     return -1;

src/or/routerlist.c

@@ -1859,61 +1859,6 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
     }
   }
 
-#if 0
-  /* XXXX This block is slow, and could be smarter.  All it does is ensure
-   * that if we have a named server called "Foo", we will never have another
-   * server called "Foo."  router_get_by_nickname() already knows to prefer
-   * named routers, so the problem only arises when there is a named router
-   * called 'foo', but we don't have it.  If, instead, we kept a
-   * name-to-identity-key mapping for each named router in the networkstatus
-   * list, we could eliminate this block.
-   *
-   * Hm. perhaps we should; I don't see how this code is non-broken wrt named
-   * routers. -NM
-   */
-  /* XXXX012 The above is indeed implemented; remove this block. */
-
-  /* If the identity key has changed, and one of the
-   * routers is named, drop the unnamed ones. (If more than one are named,
-   * drop the old ones.)
-   */
-  for (i = 0; i < smartlist_len(routerlist->routers); ++i) {
-    routerinfo_t *old_router = smartlist_get(routerlist->routers, i);
-    if (!memcmp(router->cache_info.identity_digest,
-                old_router->cache_info.identity_digest, DIGEST_LEN)) {
-
-    } else if (!strcasecmp(router->nickname, old_router->nickname)) {
-      /* nicknames match, keys don't. */
-      if (router->is_named) {
-        /* The new named router replaces the old one; remove the
-         * old one.  And carry on to the end of the list, in case
-         * there are more old unnamed routers with this nickname.
-         */
-        /* mark-for-close connections using the old key, so we can
-         * make new ones with the new key.
-         */
-        or_connection_t *conn;
-        while ((conn = connection_or_get_by_identity_digest(
-                      old_router->cache_info.identity_digest))) {
-          log_info(LD_DIR,"Closing conn to router '%s'; there is now a named "
-                   "router with that name.",
-                   old_router->nickname);
-          connection_mark_for_close(TO_CONN(conn));
-        }
-        routerlist_remove(routerlist, old_router, i--, 0);
-      } else if (old_router->is_named) {
-        /* Can't replace a named router with an unnamed one. */
-        log_debug(LD_DIR, "Skipping unnamed entry for named router '%s'",
-                  router->nickname);
-        routerinfo_free(router);
-        *msg =
-          "Already have named router with same nickname and different key.";
-        return -2;
-      }
-    }
-  }
-#endif
-
   /* We haven't seen a router with this name before.  Add it to the end of
    * the list. */
   routerlist_insert(routerlist, router);

src/or/routerparse.c

@@ -1370,10 +1370,8 @@ static addr_policy_t *
 router_parse_addr_policy(directory_token_t *tok)
 {
   addr_policy_t *newe;
-//  struct in_addr in;
   char *arg;
-//  char *address;
-//  char buf[INET_NTOA_BUF_LEN];
+  char buf[POLICY_BUF_LEN];
 
   tor_assert(tok->tp == K_REJECT || tok->tp == K_ACCEPT);
 
@@ -1386,10 +1384,6 @@ router_parse_addr_policy(directory_token_t *tok)
 
   newe = tor_malloc_zero(sizeof(addr_policy_t));
 
-  newe->string = tor_malloc(8+strlen(arg));
-  /* XXXX012 eventually, use the code from router.c:727 to generate this */
-  tor_snprintf(newe->string, 8+strlen(arg), "%s %s",
-               (tok->tp == K_REJECT) ? "reject" : "accept", arg);
   newe->policy_type = (tok->tp == K_REJECT) ? ADDR_POLICY_REJECT
     : ADDR_POLICY_ACCEPT;
 
@@ -1397,21 +1391,14 @@ router_parse_addr_policy(directory_token_t *tok)
                                 &newe->prt_min, &newe->prt_max))
     goto policy_read_failed;
 
-//  in.s_addr = htonl(newe->addr);
-//  tor_inet_ntoa(&in, buf, sizeof(buf));
-//  address = tor_strdup(buf);
-//  in.s_addr = htonl(newe->msk);
-//  log_fn(LOG_DEBUG,"%s %s/%s:%d-%d",
-//         newe->policy_type == ADDR_POLICY_REJECT ? "reject" : "accept",
-//         address, inet_ntoa(in), newe->prt_min, newe->prt_max);
-//  tor_free(address);
+  if (policy_write_item(buf, sizeof(buf), newe) < 0)
+    goto policy_read_failed;
 
+  newe->string = tor_strdup(buf);
   return newe;
 
 policy_read_failed:
-  tor_assert(newe->string);
-  log_warn(LD_DIR,"Couldn't parse line %s. Dropping", escaped(newe->string));
-  tor_free(newe->string);
+  log_warn(LD_DIR,"Couldn't parse line %s. Dropping", escaped(arg));
   tor_free(newe);
   return NULL;
 }