[or-cvs] [tor/master] Add some fixes after discussion with Nick.
Nick Mathewson
nickm at seul.org
Wed Aug 26 15:36:46 UTC 2009
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Fri, 21 Aug 2009 23:02:36 +0200
Subject: Add some fixes after discussion with Nick.
Commit: 8c29b7920ae18a46ce0527806507275783d1ae42
- Refactor geoip.c by moving duplicate code into rotate_request_period().
- Don't leak memory when cleaning up cell queues.
- Make sure that exit_(streams|bytes_(read|written)) are initialized in all
places accessing these arrays.
- Read only the last block from *stats files and ensure that its timestamp
is not more than 25 hours in the past and not more than 1 hour in the
future.
- Stop truncating the last character when reading *stats files.
The only thing that's left now is to avoid reading whole *stats files into
memory.
---
src/or/geoip.c | 41 +++++++++++++++++--------------------
src/or/relay.c | 2 +-
src/or/rephist.c | 9 ++++++++
src/or/router.c | 58 ++++++++++++++++++++++++++++++-----------------------
4 files changed, 62 insertions(+), 48 deletions(-)
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 622b582..247e16c 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -365,6 +365,23 @@ geoip_get_mean_shares(time_t now, double *v2_share_out,
return 0;
}
+/* Rotate period of v2 and v3 network status requests. */
+static void
+rotate_request_period(void)
+{
+ SMARTLIST_FOREACH(geoip_countries, geoip_country_t *, c, {
+ memmove(&c->n_v2_ns_requests[0], &c->n_v2_ns_requests[1],
+ sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
+ memmove(&c->n_v3_ns_requests[0], &c->n_v3_ns_requests[1],
+ sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
+ c->n_v2_ns_requests[REQUEST_HIST_LEN-1] = 0;
+ c->n_v3_ns_requests[REQUEST_HIST_LEN-1] = 0;
+ });
+ current_request_period_starts += REQUEST_HIST_PERIOD;
+ if (n_old_request_periods < REQUEST_HIST_LEN-1)
+ ++n_old_request_periods;
+}
+
/** Note that we've seen a client connect from the IP <b>addr</b> (host order)
* at time <b>now</b>. Ignored by all but bridges and directories if
* configured accordingly. */
@@ -404,17 +421,7 @@ geoip_note_client_seen(geoip_client_action_t action,
* with action GEOIP_CLIENT_NETWORKSTATUS{_V2}.) */
geoip_remove_old_clients(current_request_period_starts);
/* Now rotate request period */
- SMARTLIST_FOREACH(geoip_countries, geoip_country_t *, c, {
- memmove(&c->n_v2_ns_requests[0], &c->n_v2_ns_requests[1],
- sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
- memmove(&c->n_v3_ns_requests[0], &c->n_v3_ns_requests[1],
- sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
- c->n_v2_ns_requests[REQUEST_HIST_LEN-1] = 0;
- c->n_v3_ns_requests[REQUEST_HIST_LEN-1] = 0;
- });
- current_request_period_starts += REQUEST_HIST_PERIOD;
- if (n_old_request_periods < REQUEST_HIST_LEN-1)
- ++n_old_request_periods;
+ rotate_request_period();
}
}
@@ -1046,17 +1053,7 @@ geoip_dirreq_stats_write(time_t now)
open_file = NULL;
/* Rotate request period */
- SMARTLIST_FOREACH(geoip_countries, geoip_country_t *, c, {
- memmove(&c->n_v2_ns_requests[0], &c->n_v2_ns_requests[1],
- sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
- memmove(&c->n_v3_ns_requests[0], &c->n_v3_ns_requests[1],
- sizeof(uint32_t)*(REQUEST_HIST_LEN-1));
- c->n_v2_ns_requests[REQUEST_HIST_LEN-1] = 0;
- c->n_v3_ns_requests[REQUEST_HIST_LEN-1] = 0;
- });
- current_request_period_starts += REQUEST_HIST_PERIOD;
- if (n_old_request_periods < REQUEST_HIST_LEN-1)
- ++n_old_request_periods;
+ rotate_request_period();
start_of_dirreq_stats_interval = now;
diff --git a/src/or/relay.c b/src/or/relay.c
index a79a4c1..c81b831 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -1677,7 +1677,7 @@ cell_queue_clear(cell_queue_t *queue)
queue->insertion_times->first = elem->next;
mp_pool_release(elem);
}
- queue->insertion_times = NULL;
+ tor_free(queue->insertion_times);
}
}
diff --git a/src/or/rephist.c b/src/or/rephist.c
index f7d0b2b..8d78ac2 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -1371,6 +1371,9 @@ rep_hist_exit_stats_write(time_t now)
open_file_t *open_file = NULL;
FILE *out = NULL;
+ if (!exit_streams)
+ return; /* Not initialized */
+
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0)
goto done;
@@ -1480,6 +1483,8 @@ rep_hist_note_exit_bytes_written(uint16_t port, size_t num_bytes)
{
if (!get_options()->ExitPortStatistics)
return;
+ if (!exit_bytes_written)
+ return; /* Not initialized */
exit_bytes_written[port] += num_bytes;
log_debug(LD_HIST, "Written %lu bytes to exit connection to port %d.",
(unsigned long)num_bytes, port);
@@ -1492,6 +1497,8 @@ rep_hist_note_exit_bytes_read(uint16_t port, size_t num_bytes)
{
if (!get_options()->ExitPortStatistics)
return;
+ if (!exit_bytes_read)
+ return; /* Not initialized */
exit_bytes_read[port] += num_bytes;
log_debug(LD_HIST, "Read %lu bytes from exit connection to port %d.",
(unsigned long)num_bytes, port);
@@ -1503,6 +1510,8 @@ rep_hist_note_exit_stream_opened(uint16_t port)
{
if (!get_options()->ExitPortStatistics)
return;
+ if (!exit_streams)
+ return; /* Not initialized */
exit_streams[port]++;
log_debug(LD_HIST, "Opened exit stream to port %d", port);
}
diff --git a/src/or/router.c b/src/or/router.c
index 64267f2..2d6734a 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1826,36 +1826,40 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
return (int)written+1;
}
-/** Load the contents of <b>filename</b>, find the first line starting
- * with <b>end_line</b> that has a timestamp after <b>after</b>, and write
- * the file contents starting with that line to **<b>contents</b>. Return
- * 1 for success, 0 if the file does not exist or does not contain a line
- * matching these criteria, or -1 for failure. */
+/** Load the contents of <b>filename</b>, find the last line starting with
+ * <b>end_line</b>, ensure that its timestamp is not more than 25 hours in
+ * the past or more than 1 hour in the future with respect to <b>now</b>,
+ * and write the file contents starting with that line to **<b>out</b>.
+ * Return 1 for success, 0 if the file does not exist or does not contain
+ * a line matching these criteria, or -1 for failure. */
static int
-load_stats_file(const char *filename, const char *end_line, time_t after,
+load_stats_file(const char *filename, const char *end_line, time_t now,
char **out)
{
int r = -1;
char *fname = get_datadir_fname(filename);
- char *contents, *start, timestr[ISO_TIME_LEN+1];
+ char *contents, *start = NULL, *tmp, timestr[ISO_TIME_LEN+1];
time_t written;
switch (file_status(fname)) {
case FN_FILE:
+ /* X022 Find an alternative to reading the whole file to memory. */
if ((contents = read_file_to_str(fname, 0, NULL))) {
- start = contents;
- do {
- if (start != contents)
- start++; /* Avoid infinite loops. */
- if (!(start = strstr(start, end_line)))
- goto notfound;
- if (strlen(start) < strlen(end_line) + 1 + sizeof(timestr))
- goto notfound;
- strlcpy(timestr, start + 1 + strlen(end_line), sizeof(timestr));
- if (parse_iso_time(timestr, &written) < 0)
- goto notfound;
- } while (written <= after);
- *out = tor_malloc(strlen(start));
- strlcpy(*out, start, strlen(start));
+ tmp = strstr(contents, end_line);
+ /* Find last block starting with end_line */
+ while (tmp) {
+ start = tmp;
+ tmp = strstr(tmp + 1, end_line);
+ }
+ if (!start)
+ goto notfound;
+ if (strlen(start) < strlen(end_line) + 1 + sizeof(timestr))
+ goto notfound;
+ strlcpy(timestr, start + 1 + strlen(end_line), sizeof(timestr));
+ if (parse_iso_time(timestr, &written) < 0)
+ goto notfound;
+ if (written < now - (25*60*60) || written > now + (1*60*60))
+ goto notfound;
+ *out = tor_strdup(start);
r = 1;
}
notfound:
@@ -1908,7 +1912,8 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
load_stats_file("stats"PATH_SEPARATOR"dirreq-stats",
"dirreq-stats-end", since, &contents) > 0) {
int pos = strlen(s);
- if (tor_snprintf(s + pos, maxlen - strlen(s), "%s\n", contents) < 0) {
+ if (strlcpy(s + pos, contents, maxlen - strlen(s)) !=
+ strlen(contents)) {
log_warn(LD_DIR, "Could not write dirreq-stats to extra-info "
"descriptor.");
s[pos] = '\0';
@@ -1919,7 +1924,8 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
load_stats_file("stats"PATH_SEPARATOR"entry-stats",
"entry-stats-end", since, &contents) > 0) {
int pos = strlen(s);
- if (tor_snprintf(s + pos, maxlen - strlen(s), "%s\n", contents) < 0) {
+ if (strlcpy(s + pos, contents, maxlen - strlen(s)) !=
+ strlen(contents)) {
log_warn(LD_DIR, "Could not write entry-stats to extra-info "
"descriptor.");
s[pos] = '\0';
@@ -1930,7 +1936,8 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
load_stats_file("stats"PATH_SEPARATOR"buffer-stats",
"cell-stats-end", since, &contents) > 0) {
int pos = strlen(s);
- if (tor_snprintf(s + pos, maxlen - strlen(s), "%s\n", contents) < 0) {
+ if (strlcpy(s + pos, contents, maxlen - strlen(s)) !=
+ strlen(contents)) {
log_warn(LD_DIR, "Could not write buffer-stats to extra-info "
"descriptor.");
s[pos] = '\0';
@@ -1941,7 +1948,8 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo,
load_stats_file("stats"PATH_SEPARATOR"exit-stats",
"exit-stats-end", since, &contents) > 0) {
int pos = strlen(s);
- if (tor_snprintf(s + pos, maxlen - strlen(s), "%s\n", contents) < 0) {
+ if (strlcpy(s + pos, contents, maxlen - strlen(s)) !=
+ strlen(contents)) {
log_warn(LD_DIR, "Could not write exit-stats to extra-info "
"descriptor.");
s[pos] = '\0';
--
1.5.6.5
More information about the tor-commits
mailing list