Merge commit 'karsten/fix-bridge-stats-master-4'

ChangeLog

@@ -25,6 +25,9 @@ Changes in version 0.2.2.7-alpha - 2009-??-??
     - Do not segfault when writing buffer stats when we haven't observed
       a single circuit to report about. Found by Fabian Lanze. Bugfix on
       0.2.2.1-alpha.
+    - Fix statistics on client numbers by country as seen by bridges that
+      were broken in 0.2.2.1-alpha. Also switch to reporting full 24-hour
+      intervals instead of variable 12-to-48-hour intervals.
 
   o Removed features:
     - Remove the HSAuthorityRecordStats option that version 0 hidden

doc/spec/control-spec.txt

@@ -1637,11 +1637,11 @@
   TimeStarted is a quoted string indicating when the reported summary
   counts from (in GMT).
 
-  The CountrySummary keyword has as its argument a comma-separated
-  set of "countrycode=count" pairs. For example,
-  650-CLIENTS_SEEN TimeStarted="Thu Dec 25 23:50:43 EST 2008"
-  650 CountrySummary=us=16,de=8,uk=8
-[XXX Matt Edman informs me that the time format above is wrong. -RD]
+  The CountrySummary keyword has as its argument a comma-separated,
+  possibly empty set of "countrycode=count" pairs. For example (without
+  linebreak),
+  650-CLIENTS_SEEN TimeStarted="2008-12-25 23:50:43"
+  CountrySummary=us=16,de=8,uk=8
 
 4.1.15. New consensus networkstatus has arrived.
 

doc/spec/dir-spec.txt

@@ -627,8 +627,8 @@
 
         As documented in 2.1 above.  See migration notes in section 2.2.1.
 
-    "geoip-start" YYYY-MM-DD HH:MM:SS NL
-    "geoip-client-origins" CC=N,CC=N,... NL
+    ("geoip-start" YYYY-MM-DD HH:MM:SS NL)
+    ("geoip-client-origins" CC=N,CC=N,... NL)
 
         Only generated by bridge routers (see blocking.pdf), and only
         when they have been configured with a geoip database.
@@ -641,6 +641,33 @@
         "geoip-start" is the time at which we began collecting geoip
         statistics.
 
+        "geoip-start" and "geoip-client-origins" have been replaced by
+        "bridge-stats-end" and "bridge-stats-ips" in 0.2.2.4-alpha. The
+        reason is that the measurement interval with "geoip-stats" as
+        determined by subtracting "geoip-start" from "published" could
+        have had a variable length, whereas the measurement interval in
+        0.2.2.4-alpha and later is set to be exactly 24 hours long. In
+        order to clearly distinguish the new measurement intervals from
+        the old ones, the new keywords have been introduced.
+
+    "bridge-stats-end" YYYY-MM-DD HH:MM:SS (NSEC s) NL
+        [At most once.]
+
+        YYYY-MM-DD HH:MM:SS defines the end of the included measurement
+        interval of length NSEC seconds (86400 seconds by default).
+
+        A "bridge-stats-end" line, as well as any other "bridge-*" line,
+        is only added when the relay has been running as a bridge for at
+        least 24 hours.
+
+    "bridge-ips" CC=N,CC=N,... NL
+        [At most once.]
+
+        List of mappings from two-letter country codes to the number of
+        unique IP addresses that have connected from that country to the
+        bridge and which are no known relays, rounded up to the nearest
+        multiple of 8.
+
     "dirreq-stats-end" YYYY-MM-DD HH:MM:SS (NSEC s) NL
         [At most once.]
 

src/common/compat_libevent.c

@@ -26,7 +26,7 @@
 /** A number representing a version of Libevent.
 
     This is a 4-byte number, with the first three bytes representing the
-    major, minor, and patchlevel respectively of the the library.  The fourth
+    major, minor, and patchlevel respectively of the library.  The fourth
     byte is unused.
 
     This is equivalent to the format of LIBEVENT_VERSION_NUMBER on Libevent

src/common/util.c

@@ -682,6 +682,29 @@ find_whitespace_eos(const char *s, const char *eos)
   return s;
 }
 
+/** Return the first occurrence of <b>needle</b> in <b>haystack</b> that
+ * occurs at the start of a line (that is, at the beginning of <b>haystack</b>
+ * or immediately after a newline).  Return NULL if no such string is found.
+ */
+const char *
+find_str_at_start_of_line(const char *haystack, const char *needle)
+{
+  size_t needle_len = strlen(needle);
+
+  do {
+    if (!strncmp(haystack, needle, needle_len))
+      return haystack;
+
+    haystack = strchr(haystack, '\n');
+    if (!haystack)
+      return NULL;
+    else
+      ++haystack;
+  } while (*haystack);
+
+  return NULL;
+}
+
 /** Return true iff the 'len' bytes at 'mem' are all zero. */
 int
 tor_mem_is_zero(const char *mem, size_t len)
@@ -1214,7 +1237,7 @@ format_rfc1123_time(char *buf, time_t t)
   memcpy(buf+8, MONTH_NAMES[tm.tm_mon], 3);
 }
 
-/** Parse the the RFC1123 encoding of some time (in GMT) from <b>buf</b>,
+/** Parse the RFC1123 encoding of some time (in GMT) from <b>buf</b>,
  * and store the result in *<b>t</b>.
  *
  * Return 0 on success, -1 on failure.
@@ -1755,7 +1778,7 @@ write_str_to_file(const char *fname, const char *str, int bin)
 }
 
 /** Represents a file that we're writing to, with support for atomic commit:
- * we can write into a a temporary file, and either remove the file on
+ * we can write into a temporary file, and either remove the file on
  * failure, or replace the original file on success. */
 struct open_file_t {
   char *tempname; /**< Name of the temporary file. */

src/common/util.h

@@ -195,6 +195,8 @@ const char *eat_whitespace_no_nl(const char *s) ATTR_PURE;
 const char *eat_whitespace_eos_no_nl(const char *s, const char *eos) ATTR_PURE;
 const char *find_whitespace(const char *s) ATTR_PURE;
 const char *find_whitespace_eos(const char *s, const char *eos) ATTR_PURE;
+const char *find_str_at_start_of_line(const char *haystack, const char *needle)
+  ATTR_PURE;
 int tor_mem_is_zero(const char *mem, size_t len) ATTR_PURE;
 int tor_digest_is_zero(const char *digest) ATTR_PURE;
 int tor_digest256_is_zero(const char *digest) ATTR_PURE;

src/or/connection.c

@@ -1375,7 +1375,7 @@ connection_proxy_connect(connection_t *conn, int type)
       /* Send a SOCKS4 connect request with empty user id */
 
       if (tor_addr_family(&conn->addr) != AF_INET) {
-        log_warn(LD_NET, "SOCKS4 client is incompatible with with IPv6");
+        log_warn(LD_NET, "SOCKS4 client is incompatible with IPv6");
         return -1;
       }
 

src/or/connection_or.c

@@ -80,7 +80,6 @@ connection_or_clear_identity_map(void)
     }
   });
 
-
   digestmap_free(orconn_identity_map, NULL);
   orconn_identity_map = NULL;
 }
@@ -887,7 +886,7 @@ connection_or_nonopen_was_started_here(or_connection_t *conn)
  * return -1 if he is lying, broken, or otherwise something is wrong.
  *
  * If we initiated this connection (<b>started_here</b> is true), make sure
- * the other side sent sent a correctly formed certificate. If I initiated the
+ * the other side sent a correctly formed certificate. If I initiated the
  * connection, make sure it's the right guy.
  *
  * Otherwise (if we _didn't_ initiate this connection), it's okay for

src/or/control.c

@@ -279,7 +279,7 @@ connection_write_str_to_buf(const char *s, control_connection_t *conn)
 /** Given a <b>len</b>-character string in <b>data</b>, made of lines
  * terminated by CRLF, allocate a new string in *<b>out</b>, and copy the
  * contents of <b>data</b> into *<b>out</b>, adding a period before any period
- * that that appears at the start of a line, and adding a period-CRLF line at
+ * that appears at the start of a line, and adding a period-CRLF line at
  * the end. Replace all LF characters sequences with CRLF.  Return the number
  * of bytes in *<b>out</b>.
  */
@@ -1755,21 +1755,10 @@ getinfo_helper_events(control_connection_t *control_conn,
                  "information", question);
       }
     } else if (!strcmp(question, "status/clients-seen")) {
-      char geoip_start[ISO_TIME_LEN+1];
-      size_t answer_len;
-      char *geoip_summary = extrainfo_get_client_geoip_summary(time(NULL));
-
-      if (!geoip_summary)
+      char *bridge_stats = geoip_get_bridge_stats_controller(time(NULL));
+      if (!bridge_stats)
         return -1;
-
-      answer_len = strlen("TimeStarted=\"\" CountrySummary=") +
-                   ISO_TIME_LEN + strlen(geoip_summary) + 1;
-      *answer = tor_malloc(answer_len);
-      format_iso_time(geoip_start, geoip_get_history_start());
-      tor_snprintf(*answer, answer_len,
-                   "TimeStarted=\"%s\" CountrySummary=%s",
-                   geoip_start, geoip_summary);
-      tor_free(geoip_summary);
+      *answer = bridge_stats;
     } else {
       return 0;
     }
@@ -3846,10 +3835,9 @@ control_event_bootstrap_problem(const char *warn, int reason)
  * from recently. Send a copy to the controller in case it wants to
  * display it for the user. */
 void
-control_event_clients_seen(const char *timestarted, const char *countries)
+control_event_clients_seen(const char *controller_str)
 {
   send_control_event(EVENT_CLIENTS_SEEN, 0,
-    "650 CLIENTS_SEEN TimeStarted=\"%s\" CountrySummary=%s\r\n",
-    timestarted, countries);
+    "650 CLIENTS_SEEN %s\r\n", controller_str);
 }
 

src/or/geoip.c

@@ -988,7 +988,7 @@ geoip_dirreq_stats_write(time_t now)
   statsdir = get_datadir_fname("stats");
   if (check_private_dir(statsdir, CPD_CREATE) < 0)
     goto done;
-  filename = get_datadir_fname("stats"PATH_SEPARATOR"dirreq-stats");
+  filename = get_datadir_fname2("stats", "dirreq-stats");
   data_v2 = geoip_get_client_history_dirreq(now,
                 GEOIP_CLIENT_NETWORKSTATUS_V2);
   data_v3 = geoip_get_client_history_dirreq(now,
@@ -1081,6 +1081,190 @@ geoip_dirreq_stats_write(time_t now)
   tor_free(data_v3);
 }
 
+/** Start time of bridge stats. */
+static time_t start_of_bridge_stats_interval;
+
+/** Initialize bridge stats. */
+void
+geoip_bridge_stats_init(time_t now)
+{
+  start_of_bridge_stats_interval = now;
+}
+
+/** Parse the bridge statistics as they are written to extra-info
+ * descriptors for being returned to controller clients. Return the
+ * controller string if successful, or NULL otherwise. */
+static char *
+parse_bridge_stats_controller(const char *stats_str, time_t now)
+{
+  char stats_end_str[ISO_TIME_LEN+1], stats_start_str[ISO_TIME_LEN+1],
+       *controller_str, *eos, *eol, *summary;
+
+  const char *BRIDGE_STATS_END = "bridge-stats-end ";
+  const char *BRIDGE_IPS = "bridge-ips ";
+  const char *BRIDGE_IPS_EMPTY_LINE = "bridge-ips\n";
+  const char *tmp;
+  time_t stats_end_time;
+  size_t controller_len;
+  int seconds;
+  tor_assert(stats_str);
+
+  /* Parse timestamp and number of seconds from
+    "bridge-stats-end YYYY-MM-DD HH:MM:SS (N s)" */
+  tmp = find_str_at_start_of_line(stats_str, BRIDGE_STATS_END);
+  if (!tmp)
+    return NULL;
+  tmp += strlen(BRIDGE_STATS_END);
+
+  if (strlen(tmp) < ISO_TIME_LEN + 6)
+    return NULL;
+  strlcpy(stats_end_str, tmp, sizeof(stats_end_str));
+  if (parse_iso_time(stats_end_str, &stats_end_time) < 0)
+    return NULL;
+  if (stats_end_time < now - (25*60*60) ||
+      stats_end_time > now + (1*60*60))
+    return NULL;
+  seconds = (int)strtol(tmp + ISO_TIME_LEN + 2, &eos, 10);
+  if (!eos || seconds < 23*60*60)
+    return NULL;
+  format_iso_time(stats_start_str, stats_end_time - seconds);
+
+  /* Parse: "bridge-ips CC=N,CC=N,..." */
+  tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS);
+  if (tmp) {
+    tmp += strlen(BRIDGE_IPS);
+    tmp = eat_whitespace_no_nl(tmp);
+    eol = strchr(tmp, '\n');
+    if (eol)
+      summary = tor_strndup(tmp, eol-tmp);
+    else
+      summary = tor_strdup(tmp);
+  } else {
+    /* Look if there is an empty "bridge-ips" line */
+    tmp = find_str_at_start_of_line(stats_str, BRIDGE_IPS_EMPTY_LINE);
+    if (!tmp)
+      return NULL;
+    summary = tor_strdup("");
+  }
+
+  controller_len = strlen("TimeStarted=\"\" CountrySummary=") +
+                          strlen(summary) + 42;
+  controller_str = tor_malloc(controller_len);
+  if (tor_snprintf(controller_str, controller_len,
+                   "TimeStarted=\"%s\" CountrySummary=%s",
+                   stats_start_str, summary) < 0) {
+    tor_free(controller_str);
+    tor_free(summary);
+    return NULL;
+  }
+  tor_free(summary);
+  return controller_str;
+}
+
+/** Most recent bridge statistics formatted to be written to extra-info
+ * descriptors. */
+static char *bridge_stats_extrainfo = NULL;
+
+/** Most recent bridge statistics formatted to be returned to controller
+ * clients. */
+static char *bridge_stats_controller = NULL;
+
+/** Write bridge statistics to $DATADIR/stats/bridge-stats and return
+ * when we should next try to write statistics. */
+time_t
+geoip_bridge_stats_write(time_t now)
+{
+  char *statsdir = NULL, *filename = NULL, *data = NULL,
+       written[ISO_TIME_LEN+1], *out = NULL, *controller_str;
+  size_t len;
+
+  /* If we changed from relay to bridge recently, adapt starting time
+   * of current measurements. */
+  if (start_of_bridge_stats_interval < client_history_starts)
+    start_of_bridge_stats_interval = client_history_starts;
+
+  /* Check if 24 hours have passed since starting measurements. */
+  if (now < start_of_bridge_stats_interval +
+            DIR_ENTRY_RECORD_USAGE_RETAIN_IPS)
+    return start_of_bridge_stats_interval +
+           DIR_ENTRY_RECORD_USAGE_RETAIN_IPS;
+
+  /* Discard all items in the client history that are too old. */
+  geoip_remove_old_clients(start_of_bridge_stats_interval);
+
+  statsdir = get_datadir_fname("stats");
+  if (check_private_dir(statsdir, CPD_CREATE) < 0)
+    goto done;
+  filename = get_datadir_fname2("stats", "bridge-stats");
+  data = geoip_get_client_history_bridge(now, GEOIP_CLIENT_CONNECT);
+  format_iso_time(written, now);
+  len = strlen("bridge-stats-end  (999999 s)\nbridge-ips \n") +
+        ISO_TIME_LEN + (data ? strlen(data) : 0) + 42;
+  out = tor_malloc(len);
+  if (tor_snprintf(out, len, "bridge-stats-end %s (%u s)\nbridge-ips %s\n",
+              written, (unsigned) (now - start_of_bridge_stats_interval),
+              data ? data : "") < 0)
+    goto done;
+  write_str_to_file(filename, out, 0);
+  controller_str = parse_bridge_stats_controller(out, now);
+  if (!controller_str)
+    goto done;
+  start_of_bridge_stats_interval = now;
+  tor_free(bridge_stats_extrainfo);
+  tor_free(bridge_stats_controller);
+  bridge_stats_extrainfo = out;
+  out = NULL;
+  bridge_stats_controller = controller_str;
+  control_event_clients_seen(controller_str);
+ done:
+  tor_free(filename);
+  tor_free(statsdir);
+  tor_free(data);
+  tor_free(out);
+  return start_of_bridge_stats_interval +
+         DIR_ENTRY_RECORD_USAGE_RETAIN_IPS;
+}
+
+/** Try to load the most recent bridge statistics from disk, unless we
+ * have finished a measurement interval lately. */
+static void
+load_bridge_stats(time_t now)
+{
+  char *fname, *contents, *controller_str;
+  if (bridge_stats_extrainfo)
+    return;
+  fname = get_datadir_fname2("stats", "bridge-stats");
+  contents = read_file_to_str(fname, 0, NULL);
+  if (contents) {
+    controller_str = parse_bridge_stats_controller(contents, now);
+    if (controller_str) {
+      bridge_stats_extrainfo = contents;
+      bridge_stats_controller = controller_str;
+    } else {
+      tor_free(contents);
+    }
+  }
+  tor_free(fname);
+}
+
+/** Return most recent bridge statistics for inclusion in extra-info
+ * descriptors, or NULL if we don't have recent bridge statistics. */
+char *
+geoip_get_bridge_stats_extrainfo(time_t now)
+{
+  load_bridge_stats(now);
+  return bridge_stats_extrainfo;
+}
+
+/** Return most recent bridge statistics to be returned to controller
+ * clients, or NULL if we don't have recent bridge statistics. */
+char *
+geoip_get_bridge_stats_controller(time_t now)
+{
+  load_bridge_stats(now);
+  return bridge_stats_controller;
+}
+
 /** Start time of entry stats. */
 static time_t start_of_entry_stats_interval;
 
@@ -1110,7 +1294,7 @@ geoip_entry_stats_write(time_t now)
   statsdir = get_datadir_fname("stats");
   if (check_private_dir(statsdir, CPD_CREATE) < 0)
     goto done;
-  filename = get_datadir_fname("stats"PATH_SEPARATOR"entry-stats");
+  filename = get_datadir_fname2("stats", "entry-stats");
   data = geoip_get_client_history_dirreq(now, GEOIP_CLIENT_CONNECT);
   format_iso_time(written, now);
   out = start_writing_to_stdio_file(filename, OPEN_FLAGS_APPEND,

src/or/main.c

@@ -831,6 +831,8 @@ run_scheduled_events(time_t now)
   static time_t time_to_recheck_bandwidth = 0;
   static time_t time_to_check_for_expired_networkstatus = 0;
   static time_t time_to_write_stats_files = 0;
+  static time_t time_to_write_bridge_stats = 0;
+  static int should_init_bridge_stats = 1;
   static time_t time_to_retry_dns_init = 0;
   or_options_t *options = get_options();
   int i;
@@ -965,7 +967,8 @@ run_scheduled_events(time_t now)
     if (options->CellStatistics || options->DirReqStatistics ||
         options->EntryStatistics || options->ExitPortStatistics) {
       if (!time_to_write_stats_files) {
-        /* Initialize stats. */
+        /* Initialize stats. We're doing this here and not in options_act,
+         * so that we know exactly when the 24 hours interval ends. */
         if (options->CellStatistics)
           rep_hist_buffer_stats_init(now);
         if (options->DirReqStatistics)
@@ -997,6 +1000,28 @@ run_scheduled_events(time_t now)
     }
   }
 
+  /* 1h. Check whether we should write bridge statistics to disk.
+   */
+  if (should_record_bridge_info(options)) {
+    if (time_to_write_bridge_stats < now) {
+      if (should_init_bridge_stats) {
+        /* (Re-)initialize bridge statistics. */
+        geoip_bridge_stats_init(now);
+        time_to_write_bridge_stats = now + WRITE_STATS_INTERVAL;
+        should_init_bridge_stats = 0;
+      } else {
+        /* Possibly write bridge statistics to disk and ask when to write
+         * them next time. */
+        time_to_write_bridge_stats = geoip_bridge_stats_write(
+                                           time_to_write_bridge_stats);
+      }
+    }
+  } else if (!should_init_bridge_stats) {
+    /* Bridge mode was turned off. Ensure that stats are re-initialized
+     * next time bridge mode is turned on. */
+    should_init_bridge_stats = 1;
+  }
+
   /* Remove old information from rephist and the rend cache. */
   if (time_to_clean_caches < now) {
     rep_history_clean(now - options->RephistTrackTime);

src/or/or.h

@@ -3628,8 +3628,7 @@ typedef enum {
 void control_event_bootstrap(bootstrap_status_t status, int progress);
 void control_event_bootstrap_problem(const char *warn, int reason);
 
-void control_event_clients_seen(const char *timestarted,
-                                const char *countries);
+void control_event_clients_seen(const char *controller_str);
 
 #ifdef CONTROL_PRIVATE
 /* Used only by control.c and test.c */
@@ -4087,6 +4086,10 @@ void geoip_dirreq_stats_init(time_t now);
 void geoip_dirreq_stats_write(time_t now);
 void geoip_entry_stats_init(time_t now);
 void geoip_entry_stats_write(time_t now);
+void geoip_bridge_stats_init(time_t now);
+time_t geoip_bridge_stats_write(time_t now);
+char *geoip_get_bridge_stats_extrainfo(time_t);
+char *geoip_get_bridge_stats_controller(time_t);
 
 /********************************* hibernate.c **********************/
 
@@ -4747,7 +4750,6 @@ int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
                                  crypto_pk_env_t *ident_key);
 int extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
                              crypto_pk_env_t *ident_key);
-char *extrainfo_get_client_geoip_summary(time_t);
 int is_legal_nickname(const char *s);
 int is_legal_nickname_or_hexdigest(const char *s);
 int is_legal_hexdigest(const char *s);

src/or/rephist.c

@@ -1375,7 +1375,7 @@ rep_hist_exit_stats_write(time_t now)
   statsdir = get_datadir_fname("stats");
   if (check_private_dir(statsdir, CPD_CREATE) < 0)
     goto done;
-  filename = get_datadir_fname("stats"PATH_SEPARATOR"exit-stats");
+  filename = get_datadir_fname2("stats", "exit-stats");
   format_iso_time(t, now);
   log_info(LD_HIST, "Writing exit port statistics to disk for period "
            "ending at %s.", t);
@@ -2180,7 +2180,7 @@ rep_hist_buffer_stats_write(time_t now)
   statsdir = get_datadir_fname("stats");
   if (check_private_dir(statsdir, CPD_CREATE) < 0)
     goto done;
-  filename = get_datadir_fname("stats"PATH_SEPARATOR"buffer-stats");
+  filename = get_datadir_fname2("stats", "buffer-stats");
   out = start_writing_to_stdio_file(filename, OPEN_FLAGS_APPEND,
                                     0600, &open_file);
   if (!out)

src/or/router.c

@@ -1660,7 +1660,7 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     return -1;
   }
 
-  /* PEM-encode the identity key key */
+  /* PEM-encode the identity key */
   if (crypto_pk_write_public_key_to_string(router->identity_pkey,
                                         &identity_pkey,&identity_pkeylen)<0) {
     log_warn(LD_BUG,"write identity_pkey to string failed!");
@@ -1882,6 +1882,7 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
   int result;
   size_t len;
   static int write_stats_to_extrainfo = 1;
+  time_t now = time(NULL);
 
   base16_encode(identity, sizeof(identity),
                 extrainfo->cache_info.identity_digest, DIGEST_LEN);
@@ -1896,7 +1897,6 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
 
   if (options->ExtraInfoStatistics && write_stats_to_extrainfo) {
     char *contents = NULL;
-    time_t now = time(NULL);
     log_info(LD_GENERAL, "Adding stats to extra-info descriptor.");
     if (options->DirReqStatistics &&
         load_stats_file("stats"PATH_SEPARATOR"dirreq-stats",
@@ -1953,18 +1953,16 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
     return -1;
 
   if (should_record_bridge_info(options)) {
-    char *geoip_summary = extrainfo_get_client_geoip_summary(time(NULL));
-    if (geoip_summary) {
-      char geoip_start[ISO_TIME_LEN+1];
-      format_iso_time(geoip_start, geoip_get_history_start());
-      result = tor_snprintf(s+strlen(s), maxlen-strlen(s),
-                            "geoip-start-time %s\n"
-                            "geoip-client-origins %s\n",
-                            geoip_start, geoip_summary);
-      control_event_clients_seen(geoip_start, geoip_summary);
-      tor_free(geoip_summary);
-      if (result<0)
-        return -1;
+    char *bridge_stats = geoip_get_bridge_stats_extrainfo(now);
+    if (bridge_stats) {
+      size_t pos = strlen(s);
+      if (strlcpy(s + pos, bridge_stats, maxlen - strlen(s)) !=
+          strlen(bridge_stats)) {
+        log_warn(LD_DIR, "Could not write bridge-stats to extra-info "
+                 "descriptor.");
+        s[pos] = '\0';
+      }
+      tor_free(bridge_stats);
     }
   }
 
@@ -2013,30 +2011,6 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
   return (int)strlen(s)+1;
 }
 
-/** Wrapper function for geoip_get_client_history(). It first discards
- * any items in the client history that are too old -- it dumps anything
- * more than 48 hours old, but it only considers whether to dump at most
- * once per 48 hours, so we aren't too precise to an observer (see also
- * r14780).
- */
-char *
-extrainfo_get_client_geoip_summary(time_t now)
-{
-  static time_t last_purged_at = 0;
-  int geoip_purge_interval =
-      (get_options()->DirReqStatistics || get_options()->EntryStatistics) ?
-      DIR_ENTRY_RECORD_USAGE_RETAIN_IPS : 48*60*60;
-  if (now > last_purged_at+geoip_purge_interval) {
-    /* (Note that this also discards items in the client history with
-     * action GEOIP_CLIENT_NETWORKSTATUS{_V2}, which doesn't matter
-     * because bridge and directory stats are independent. Keep in mind
-     * for future extensions, though.) */
-    geoip_remove_old_clients(now-geoip_purge_interval);
-    last_purged_at = now;
-  }
-  return geoip_get_client_history_bridge(now, GEOIP_CLIENT_CONNECT);
-}
-
 /** Return true iff <b>s</b> is a legally valid server nickname. */
 int
 is_legal_nickname(const char *s)

src/or/routerlist.c

@@ -3937,7 +3937,7 @@ client_would_use_router(routerstatus_t *rs, time_t now, or_options_t *options)
  * this number per server. */
 #define MIN_DL_PER_REQUEST 4
 /** To prevent a single screwy cache from confusing us by selective reply,
- * try to split our requests into at least this this many requests. */
+ * try to split our requests into at least this many requests. */
 #define MIN_REQUESTS 3
 /** If we want fewer than this many descriptors, wait until we
  * want more, or until MAX_CLIENT_INTERVAL_WITHOUT_REQUEST has

src/or/routerparse.c

@@ -2928,7 +2928,7 @@ networkstatus_parse_detached_signatures(const char *s, const char *eos)
     }
     if (base16_decode(digests->d[alg], DIGEST256_LEN,
                       hexdigest, strlen(hexdigest)) < 0) {
-      log_warn(LD_DIR, "Bad encoding on on consensus-digest in detached "
+      log_warn(LD_DIR, "Bad encoding on consensus-digest in detached "
                "networkstatus signatures");
       goto err;
     }

src/test/test_util.c

@@ -1023,9 +1023,38 @@ test_util_strtok(void)
   ;
 }
 
+static void
+test_util_find_str_at_start_of_line(void *ptr)
+{
+  const char *long_string =
+    "hello world. hello world. hello hello. howdy.\n"
+    "hello hello world\n";
+
+  (void)ptr;
+
+  /* not-found case. */
+  tt_assert(! find_str_at_start_of_line(long_string, "fred"));
+
+  /* not-found case where haystack doesn't end with \n */
+  tt_assert(! find_str_at_start_of_line("foobar\nbaz", "fred"));
+
+  /* start-of-string case */
+  tt_assert(long_string ==
+            find_str_at_start_of_line(long_string, "hello world."));
+
+  /* start-of-line case */
+  tt_assert(strchr(long_string,'\n')+1 ==
+            find_str_at_start_of_line(long_string, "hello hello"));
+ done:
+  ;
+}
+
 #define UTIL_LEGACY(name)                                               \
   { #name, legacy_test_helper, 0, &legacy_setup, test_util_ ## name }
 
+#define UTIL_TEST(name, flags)                          \
+  { #name, test_util_ ## name, flags, NULL, NULL }
+
 struct testcase_t util_tests[] = {
   UTIL_LEGACY(time),
   UTIL_LEGACY(config_line),
@@ -1040,6 +1069,7 @@ struct testcase_t util_tests[] = {
   UTIL_LEGACY(threads),
   UTIL_LEGACY(sscanf),
   UTIL_LEGACY(strtok),
+  UTIL_TEST(find_str_at_start_of_line, 0),
   END_OF_TESTCASES
 };