commit fa32574bdb7381f02abc68ad917ad79f354938a4 Author: Nick Mathewson nickm@torproject.org Date: Thu Sep 27 10:15:39 2018 -0400

Remove excess dependencies from geoip.c --- src/app/config/config.c | 24 ++++++++++++++++++++---- src/feature/stats/geoip.c | 41 +++++++++++++++++++++++++---------------- src/feature/stats/geoip.h | 12 +++++++++--- src/test/test_geoip.c | 14 ++++++++------ 4 files changed, 62 insertions(+), 29 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c index 0909ced50..53cc9c0ac 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -96,6 +96,7 @@ #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nickname.h" #include "feature/nodelist/nodelist.h" +#include "feature/nodelist/routerlist.h" #include "feature/nodelist/routerset.h" #include "feature/relay/dns.h" #include "feature/relay/ext_orport.h" @@ -8348,6 +8349,11 @@ config_load_geoip_file_(sa_family_t family, const char *fname, const char *default_fname) { + const or_options_t *options = get_options(); + const char *msg = ""; + int severity = options_need_geoip_info(options, &msg) ? LOG_WARN : LOG_INFO; + int r; + #ifdef _WIN32 char *free_fname = NULL; /* Used to hold any temporary-allocated value */ /* XXXX Don't use this "<default>" junk; make our filename options @@ -8357,12 +8363,16 @@ config_load_geoip_file_(sa_family_t family, tor_asprintf(&free_fname, "%s\%s", conf_root, default_fname); fname = free_fname; } - geoip_load_file(family, fname); + r = geoip_load_file(family, fname, severity); tor_free(free_fname); #else /* !(defined(_WIN32)) */ (void)default_fname; - geoip_load_file(family, fname); + r = geoip_load_file(family, fname, severity); #endif /* defined(_WIN32) */ + + if (r < 0 && severity == LOG_WARN) { + log_warn(LD_GENERAL, "%s", msg); + } }

/** Load geoip files for IPv4 and IPv6 if <a>options</a> and @@ -8376,13 +8386,19 @@ config_maybe_load_geoip_files_(const or_options_t *options, if (options->GeoIPFile && ((!old_options || !opt_streq(old_options->GeoIPFile, options->GeoIPFile)) - || !geoip_is_loaded(AF_INET))) + || !geoip_is_loaded(AF_INET))) { config_load_geoip_file_(AF_INET, options->GeoIPFile, "geoip"); + /* Okay, now we need to maybe change our mind about what is in + * which country. We do this for IPv4 only since that's what we + * store in node->country. */ + refresh_all_country_info(); + } if (options->GeoIPv6File && ((!old_options || !opt_streq(old_options->GeoIPv6File, options->GeoIPv6File)) - || !geoip_is_loaded(AF_INET6))) + || !geoip_is_loaded(AF_INET6))) { config_load_geoip_file_(AF_INET6, options->GeoIPv6File, "geoip6"); + } }

/** Initialize cookie authentication (used so far by the ControlPort diff --git a/src/feature/stats/geoip.c b/src/feature/stats/geoip.c index 885f26bc3..496615080 100644 --- a/src/feature/stats/geoip.c +++ b/src/feature/stats/geoip.c @@ -28,16 +28,32 @@ */

#define GEOIP_PRIVATE -#include "core/or/or.h" -#include "ht.h" -#include "lib/container/buffers.h" -#include "app/config/config.h" // XXXX -#include "feature/stats/geoip.h" -#include "feature/nodelist/routerlist.h" // XXXX

+#include "lib/cc/torint.h" +/** A signed integer representing a country code. */ +typedef int16_t country_t; // XXXX duplicate in or.h + +#include "feature/stats/geoip.h" +#include "lib/container/map.h" #include "lib/container/order.h" +#include "lib/container/smartlist.h" +#include "lib/crypt_ops/crypto_digest.h" +#include "lib/ctime/di_ops.h" +#include "lib/encoding/binascii.h" +#include "lib/fs/files.h" +#include "lib/log/escape.h" +#include "lib/malloc/malloc.h" +#include "lib/net/address.h" //???? +#include "lib/net/inaddr.h" +#include "lib/string/compat_ctype.h" +#include "lib/string/compat_string.h" +#include "lib/string/scanf.h" +#include "lib/string/util_string.h" #include "lib/time/tvdiff.h"

+#include <stdio.h> +#include <string.h> + static void init_geoip_countries(void);

/** An entry from the GeoIP IPv4 file: maps an IPv4 range to a country. */ @@ -305,19 +321,16 @@ init_geoip_countries(void) * with '#' (comments). */ int -geoip_load_file(sa_family_t family, const char *filename) +geoip_load_file(sa_family_t family, const char *filename, int severity) { FILE *f; - const char *msg = ""; - const or_options_t *options = get_options(); - int severity = options_need_geoip_info(options, &msg) ? LOG_WARN : LOG_INFO; crypto_digest_t *geoip_digest_env = NULL;

tor_assert(family == AF_INET || family == AF_INET6);

if (!(f = tor_fopen_cloexec(filename, "r"))) { - log_fn(severity, LD_GENERAL, "Failed to open GEOIP file %s. %s", - filename, msg); + log_fn(severity, LD_GENERAL, "Failed to open GEOIP file %s.", + filename); return -1; } if (!geoip_countries) @@ -357,10 +370,6 @@ geoip_load_file(sa_family_t family, const char *filename) * our extra-info descriptors. */ if (family == AF_INET) { smartlist_sort(geoip_ipv4_entries, geoip_ipv4_compare_entries_); - /* Okay, now we need to maybe change our mind about what is in - * which country. We do this for IPv4 only since that's what we - * store in node->country. */ - refresh_all_country_info(); crypto_digest_get_digest(geoip_digest_env, geoip_digest, DIGEST_LEN); } else { /* AF_INET6 */ diff --git a/src/feature/stats/geoip.h b/src/feature/stats/geoip.h index 4adfc10fb..1ca04cdff 100644 --- a/src/feature/stats/geoip.h +++ b/src/feature/stats/geoip.h @@ -12,13 +12,18 @@ #ifndef TOR_GEOIP_H #define TOR_GEOIP_H

+#include "lib/net/nettypes.h" #include "lib/testsupport/testsupport.h" +#include "lib/cc/torint.h"

#ifdef GEOIP_PRIVATE STATIC int geoip_parse_entry(const char *line, sa_family_t family); STATIC void clear_geoip_db(void); #endif /* defined(GEOIP_PRIVATE) */

+struct in6_addr; +struct tor_addr_t; + int geoip_get_country_by_ipv4(uint32_t ipaddr); int geoip_get_country_by_ipv6(const struct in6_addr *addr);

@@ -27,10 +32,11 @@ typedef struct geoip_country_t { char countrycode[3]; } geoip_country_t;

-const smartlist_t *geoip_get_countries(void); +struct smartlist_t; +const struct smartlist_t *geoip_get_countries(void);

-int geoip_load_file(sa_family_t family, const char *filename); -MOCK_DECL(int, geoip_get_country_by_addr, (const tor_addr_t *addr)); +int geoip_load_file(sa_family_t family, const char *filename, int severity); +MOCK_DECL(int, geoip_get_country_by_addr, (const struct tor_addr_t *addr)); MOCK_DECL(int, geoip_get_n_countries, (void)); const char *geoip_get_country_name(country_t num); MOCK_DECL(int, geoip_is_loaded, (sa_family_t family)); diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index 8f13939db..5e2849147 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -404,7 +404,8 @@ test_geoip_load_file(void *arg)

/* A nonexistant filename should fail. */ tt_int_op(-1, OP_EQ, - geoip_load_file(AF_INET, "/you/did/not/put/a/file/here/I/hope")); + geoip_load_file(AF_INET, "/you/did/not/put/a/file/here/I/hope", + LOG_INFO));

/* We start out with only "Ningunpartia" in the database. */ tt_int_op(1, OP_EQ, geoip_get_n_countries()); @@ -418,7 +419,7 @@ test_geoip_load_file(void *arg) const char *fname = get_fname("geoip"); tt_int_op(0, OP_EQ, write_str_to_file(fname, GEOIP_CONTENT, 1));

- int rv = geoip_load_file(AF_INET, fname); + int rv = geoip_load_file(AF_INET, fname, LOG_WARN); if (rv != 0) { TT_GRIPE(("Unable to load geoip from %s", escaped(fname))); } @@ -468,7 +469,8 @@ test_geoip6_load_file(void *arg)

/* A nonexistant filename should fail. */ tt_int_op(-1, OP_EQ, - geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope")); + geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope", + LOG_INFO));

/* Any lookup attempt should say "-1" because we have no info */ tor_inet_pton(AF_INET6, "2001:4860:4860::8888", &iaddr6); @@ -494,7 +496,7 @@ test_geoip6_load_file(void *arg) "2001:4878:205::,2001:4878:214:ffff:ffff:ffff:ffff:ffff,US\n"; tt_int_op(0, OP_EQ, write_str_to_file(fname6, CONTENT, 1));

- tt_int_op(0, OP_EQ, geoip_load_file(AF_INET6, fname6)); + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET6, fname6, LOG_WARN));

/* Check that we loaded some countries; this will fail if there are ever * fewer than 5 countries in our test data above. */ @@ -546,11 +548,11 @@ test_geoip_load_2nd_file(void *arg) tt_int_op(0, OP_EQ, write_str_to_file(fname_empty, "\n", 1));

/* Load 1st geoip file */ - tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, fname_geoip)); + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, fname_geoip, LOG_WARN));

/* Load 2nd geoip (empty) file */ /* It has to be the same IP address family */ - tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, fname_empty)); + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, fname_empty, LOG_WARN));

/* Check that there is no geoip information for 8.8.8.8, */ /* since loading the empty 2nd file should have delete it. */