Remove excess dependencies from geoip.c

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

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",
+    log_fn(severity, LD_GENERAL, "Failed to open GEOIP file %s.",
-           filename, msg);
+           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 */

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);
+int geoip_load_file(sa_family_t family, const char *filename, int severity);
-MOCK_DECL(int, geoip_get_country_by_addr, (const tor_addr_t *addr));
+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));

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. */