[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Implement the various get_foodir_*() functions.



commit a57bcffcc7972419418b18d9a720a32471408ca5
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Tue Nov 14 18:14:08 2017 -0500

    Implement the various get_foodir_*() functions.
---
 src/or/config.c                | 77 ++++++++++++++++++++++--------------------
 src/or/config.h                | 73 ++++++++++++++++++++++++++-------------
 src/test/test_conscache.c      | 12 +++----
 src/test/test_consdiffmgr.c    |  8 ++---
 src/test/test_dir.c            |  6 ++--
 src/test/test_dir_handle_get.c |  1 +
 src/test/test_microdesc.c      | 36 ++++++++++----------
 src/test/test_routerkeys.c     |  9 +++--
 src/test/testing_common.c      |  3 ++
 9 files changed, 132 insertions(+), 93 deletions(-)

diff --git a/src/or/config.c b/src/or/config.c
index a5007d03f..16e644561 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -7984,53 +7984,56 @@ init_libevent(const or_options_t *options)
   suppress_libevent_log_msg(NULL);
 }
 
-/** Return a newly allocated string holding a filename relative to the data
- * directory.  If <b>sub1</b> is present, it is the first path component after
+/** Return a newly allocated string holding a filename relative to the
+ * directory in <b>options</b> specified by <b>roottype</b>.
+ * If <b>sub1</b> is present, it is the first path component after
  * the data directory.  If <b>sub2</b> is also present, it is the second path
  * component after the data directory.  If <b>suffix</b> is present, it
  * is appended to the filename.
  *
- * Examples:
- *    get_datadir_fname2_suffix("a", NULL, NULL) -> $DATADIR/a
- *    get_datadir_fname2_suffix("a", NULL, ".tmp") -> $DATADIR/a.tmp
- *    get_datadir_fname2_suffix("a", "b", ".tmp") -> $DATADIR/a/b/.tmp
- *    get_datadir_fname2_suffix("a", "b", NULL) -> $DATADIR/a/b
- *
- * Note: Consider using the get_datadir_fname* macros in or.h.
+ * Note: Consider using macros in config.h that wrap this function;
+ * you should probably never need to call it as-is.
  */
 MOCK_IMPL(char *,
-options_get_datadir_fname2_suffix,(const or_options_t *options,
-                                   const char *sub1, const char *sub2,
-                                   const char *suffix))
+options_get_dir_fname2_suffix,(const or_options_t *options,
+                               directory_root_t roottype,
+                               const char *sub1, const char *sub2,
+                               const char *suffix))
 {
-  char *fname = NULL;
-  size_t len;
   tor_assert(options);
-  tor_assert(options->DataDirectory);
-  tor_assert(sub1 || !sub2); /* If sub2 is present, sub1 must be present. */
-  len = strlen(options->DataDirectory);
-  if (sub1) {
-    len += strlen(sub1)+1;
-    if (sub2)
-      len += strlen(sub2)+1;
-  }
-  if (suffix)
-    len += strlen(suffix);
-  len++;
-  fname = tor_malloc(len);
-  if (sub1) {
-    if (sub2) {
-      tor_snprintf(fname, len, "%s"PATH_SEPARATOR"%s"PATH_SEPARATOR"%s",
-                   options->DataDirectory, sub1, sub2);
-    } else {
-      tor_snprintf(fname, len, "%s"PATH_SEPARATOR"%s",
-                   options->DataDirectory, sub1);
-    }
+
+  const char *rootdir = NULL;
+  switch (roottype) {
+    case DIRROOT_DATADIR:
+      rootdir = options->DataDirectory;
+      break;
+    case DIRROOT_CACHEDIR:
+      rootdir = options->CacheDirectory;
+      break;
+    case DIRROOT_KEYDIR:
+      rootdir = options->KeyDirectory;
+      break;
+    default:
+      tor_assert_unreached();
+      break;
+  }
+  tor_assert(rootdir);
+
+  if (!suffix)
+    suffix = "";
+
+  char *fname = NULL;
+
+  if (sub1 == NULL) {
+    tor_asprintf(&fname, "%s%s", rootdir, suffix);
+    tor_assert(!sub2); /* If sub2 is present, sub1 must be present. */
+  } else if (sub2 == NULL) {
+    tor_asprintf(&fname, "%s"PATH_SEPARATOR"%s%s", rootdir, sub1, suffix);
   } else {
-    strlcpy(fname, options->DataDirectory, len);
+    tor_asprintf(&fname, "%s"PATH_SEPARATOR"%s"PATH_SEPARATOR"%s%s",
+                 rootdir, sub1, sub2, suffix);
   }
-  if (suffix)
-    strlcat(fname, suffix, len);
+
   return fname;
 }
 
diff --git a/src/or/config.h b/src/or/config.h
index 4e9bb0f6f..1e4239b6d 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -58,45 +58,72 @@ config_line_t *option_get_assignment(const or_options_t *options,
                                      const char *key);
 int options_save_current(void);
 const char *get_torrc_fname(int defaults_fname);
+typedef enum {
+  DIRROOT_DATADIR,
+  DIRROOT_CACHEDIR,
+  DIRROOT_KEYDIR
+} directory_root_t;
+
 MOCK_DECL(char *,
-          options_get_datadir_fname2_suffix,
+          options_get_dir_fname2_suffix,
           (const or_options_t *options,
+           directory_root_t roottype,
            const char *sub1, const char *sub2,
            const char *suffix));
+
+/* These macros wrap options_get_dir_fname2_suffix to provide a more
+ * convenient API for finding filenames that Tor uses inside its storage
+ * They are named according to a pattern:
+ *    (options_)?get_(cache|key|data)dir_fname(2)?(_suffix)?
+ *
+ * Macros that begin with options_ take an options argument; the others
+ * work with respect to the global options.
+ *
+ * Each macro works relative to the data directory, the key directory,
+ * or the cache directory, as determined by which one is mentioned.
+ *
+ * Macro variants with "2" in their name take two path components; others
+ * take one.
+ *
+ * Macro variants with "_suffix" at the end take an additional suffix
+ * that gets appended to the end of the file
+ */
+#define options_get_datadir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_DATADIR, \
+                                (sub1), (sub2), (suffix))
+#define options_get_cachedir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_CACHEDIR, \
+                                (sub1), (sub2), (suffix))
+#define options_get_keydir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_KEYDIR, \
+                                (sub1), (sub2), (suffix))
+
+#define options_get_datadir_fname(opts,sub1)                    \
+  options_get_datadir_fname2_suffix((opts),(sub1), NULL, NULL)
+#define options_get_datadir_fname2(opts,sub1,sub2)                      \
+  options_get_datadir_fname2_suffix((opts),(sub1), (sub2), NULL)
+
 #define get_datadir_fname2_suffix(sub1, sub2, suffix) \
   options_get_datadir_fname2_suffix(get_options(), (sub1), (sub2), (suffix))
-/** Return a newly allocated string containing datadir/sub1.  See
- * get_datadir_fname2_suffix.  */
-#define get_datadir_fname(sub1) get_datadir_fname2_suffix((sub1), NULL, NULL)
-/** Return a newly allocated string containing datadir/sub1/sub2.  See
- * get_datadir_fname2_suffix.  */
+#define get_datadir_fname(sub1)                 \
+  get_datadir_fname2_suffix((sub1), NULL, NULL)
 #define get_datadir_fname2(sub1,sub2) \
   get_datadir_fname2_suffix((sub1), (sub2), NULL)
-/** Return a newly allocated string containing datadir/sub1suffix.  See
- * get_datadir_fname2_suffix. */
 #define get_datadir_fname_suffix(sub1, suffix) \
   get_datadir_fname2_suffix((sub1), NULL, (suffix))
 
-/** Return a newly allocated string containing datadir/sub1 relative to
- * opts.  See get_datadir_fname2_suffix.  */
-#define options_get_datadir_fname(opts,sub1)                    \
-  options_get_datadir_fname2_suffix((opts),(sub1), NULL, NULL)
-/** Return a newly allocated string containing datadir/sub1/sub2 relative to
- * opts.  See get_datadir_fname2_suffix.  */
-#define options_get_datadir_fname2(opts,sub1,sub2)                      \
-  options_get_datadir_fname2_suffix((opts),(sub1), (sub2), NULL)
-
 /** DOCDOC */
-#define get_keydir_fname(sub1)                  \
-  get_datadir_fname2("keys", (sub1))
 #define options_get_keydir_fname(options, sub1)  \
-  options_get_datadir_fname2((options), "keys", (sub1))
+  options_get_keydir_fname2_suffix((options), (sub1), NULL, NULL)
 #define get_keydir_fname_suffix(sub1, suffix)   \
-  get_datadir_fname2_suffix("keys", (sub1), (suffix))
+  options_get_keydir_fname2_suffix(get_options(), (sub1), NULL, suffix)
+#define get_keydir_fname(sub1)                  \
+  options_get_keydir_fname2_suffix(get_options(), (sub1), NULL, NULL)
 
-#define get_cachedir_fname(sub1) get_datadir_fname((sub1))
+#define get_cachedir_fname(sub1) \
+  options_get_cachedir_fname2_suffix(get_options(), (sub1), NULL, NULL)
 #define get_cachedir_fname_suffix(sub1, suffix) \
-  get_datadir_fname_suffix((sub1), (suffix))
+  options_get_cachedir_fname2_suffix(get_options(), (sub1), NULL, (suffix))
 
 int using_default_dir_authorities(const or_options_t *options);
 
diff --git a/src/test/test_conscache.c b/src/test/test_conscache.c
index ddb1bc53c..ffec3149b 100644
--- a/src/test/test_conscache.c
+++ b/src/test/test_conscache.c
@@ -31,8 +31,8 @@ test_conscache_simple_usage(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
@@ -124,8 +124,8 @@ test_conscache_cleanup(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
@@ -267,8 +267,8 @@ test_conscache_filter(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c
index 80d3f943a..a9a4b6a98 100644
--- a/src/test/test_consdiffmgr.c
+++ b/src/test/test_consdiffmgr.c
@@ -24,8 +24,8 @@ consdiffmgr_test_setup(const struct testcase_t *arg)
 {
   (void)arg;
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer.
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = ddir_fname; // now owns the pointer.
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
 
   consdiff_cfg_t consdiff_cfg = { 300 };
@@ -215,8 +215,8 @@ test_consdiffmgr_init_failure(void *arg)
   /* As in ...test_setup, but do not create the datadir. The missing directory
    * will cause a failure. */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer.
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = ddir_fname; // now owns the pointer.
 
   consdiff_cfg_t consdiff_cfg = { 7200, 300 };
 
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index ee4a9780b..83734ccfd 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -4874,9 +4874,11 @@ mock_check_private_dir(const char *dirname, cpd_check_t check,
 
 static char *
 mock_get_datadir_fname(const or_options_t *options,
+                       directory_root_t roottype,
                        const char *sub1, const char *sub2,
                        const char *suffix)
 {
+  (void) roottype;
   char *rv = NULL;
 
   /*
@@ -5033,7 +5035,7 @@ test_dir_dump_unparseable_descriptors(void *data)
   mock_options->MaxUnparseableDescSizeToLog = 1536;
   MOCK(get_options, mock_get_options);
   MOCK(check_private_dir, mock_check_private_dir);
-  MOCK(options_get_datadir_fname2_suffix,
+  MOCK(options_get_dir_fname2_suffix,
        mock_get_datadir_fname);
 
   /*
@@ -5551,7 +5553,7 @@ test_dir_dump_unparseable_descriptors(void *data)
   mock_unlink_reset();
   UNMOCK(write_str_to_file);
   mock_write_str_to_file_reset();
-  UNMOCK(options_get_datadir_fname2_suffix);
+  UNMOCK(options_get_dir_fname2_suffix);
   UNMOCK(check_private_dir);
   UNMOCK(get_options);
   tor_free(mock_options);
diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c
index fe26657ad..28ecc14c4 100644
--- a/src/test/test_dir_handle_get.c
+++ b/src/test/test_dir_handle_get.c
@@ -469,6 +469,7 @@ init_mock_options(void)
   memset(mock_options, 0, sizeof(or_options_t));
   mock_options->TestingTorNetwork = 1;
   mock_options->DataDirectory = tor_strdup(get_fname_rnd("datadir_tmp"));
+  mock_options->CacheDirectory = tor_strdup(mock_options->DataDirectory);
   check_private_dir(mock_options->DataDirectory, CPD_CREATE, NULL);
 }
 
diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c
index 4f0ecd778..59b28f758 100644
--- a/src/test/test_microdesc.c
+++ b/src/test/test_microdesc.c
@@ -69,12 +69,12 @@ test_md_cache(void *data)
   time3 = time(NULL) - 15*24*60*60;
 
   /* Possibly, turn this into a test setup/cleanup pair */
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test"));
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   tt_assert(!strcmpstart(test_md3_noannotation, "onion-key"));
@@ -152,7 +152,7 @@ test_md_cache(void *data)
               strlen(test_md3_noannotation));
 
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs.new",
-               options->DataDirectory);
+               options->CacheDirectory);
   s = read_file_to_str(fn, RFTS_BIN, NULL);
   tt_assert(s);
   tt_mem_op(md1->body, OP_EQ, s + md1->off, md1->bodylen);
@@ -180,7 +180,7 @@ test_md_cache(void *data)
 
   /* read the cache. */
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs",
-               options->DataDirectory);
+               options->CacheDirectory);
   s = read_file_to_str(fn, RFTS_BIN, NULL);
   tt_mem_op(md1->body, OP_EQ, s + md1->off, strlen(test_md1));
   tt_mem_op(md2->body, OP_EQ, s + md2->off, strlen(test_md2));
@@ -234,7 +234,7 @@ test_md_cache(void *data)
 
  done:
   if (options)
-    tor_free(options->DataDirectory);
+    tor_free(options->CacheDirectory);
   microdesc_free_all();
 
   smartlist_free(added);
@@ -266,17 +266,17 @@ test_md_cache_broken(void *data)
 
   options = get_options_mutable();
   tt_assert(options);
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test2"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test2"));
 
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs",
-               options->DataDirectory);
+               options->CacheDirectory);
 
   write_str_to_file(fn, truncated_md, 1);
 
@@ -285,7 +285,7 @@ test_md_cache_broken(void *data)
 
  done:
   if (options)
-    tor_free(options->DataDirectory);
+    tor_free(options->CacheDirectory);
   tor_free(fn);
   microdesc_free_all();
 }
@@ -754,8 +754,8 @@ test_md_reject_cache(void *arg)
   or_options_t *options = get_options_mutable();
   char buf[DIGEST256_LEN];
 
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test_rej"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test_rej"));
   mock_rgsbd_val_a = tor_malloc_zero(sizeof(routerstatus_t));
   mock_rgsbd_val_b = tor_malloc_zero(sizeof(routerstatus_t));
   mock_ns_val = tor_malloc_zero(sizeof(networkstatus_t));
@@ -765,9 +765,9 @@ test_md_reject_cache(void *arg)
   mock_ns_val->flavor = FLAV_MICRODESC;
 
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   MOCK(router_get_mutable_consensus_status_by_descriptor_digest,
@@ -802,7 +802,7 @@ test_md_reject_cache(void *arg)
  done:
   UNMOCK(networkstatus_get_latest_consensus_by_flavor);
   UNMOCK(router_get_mutable_consensus_status_by_descriptor_digest);
-  tor_free(options->DataDirectory);
+  tor_free(options->CacheDirectory);
   microdesc_free_all();
   smartlist_free(added);
   SMARTLIST_FOREACH(wanted, char *, cp, tor_free(cp));
diff --git a/src/test/test_routerkeys.c b/src/test/test_routerkeys.c
index d8b72651a..e4abcdb92 100644
--- a/src/test/test_routerkeys.c
+++ b/src/test/test_routerkeys.c
@@ -421,6 +421,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 {
   (void)arg;
   char *dir = tor_strdup(get_fname("test_ed_keys_init_all"));
+  char *keydir = tor_strdup(get_fname("test_ed_keys_init_all/KEYS"));
   or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
   time_t now = time(NULL);
   ed25519_public_key_t id;
@@ -445,13 +446,14 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
 #ifdef _WIN32
   mkdir(dir);
-  mkdir(get_fname("test_ed_keys_init_all/keys"));
+  mkdir(keydir);
 #else
   mkdir(dir, 0700);
-  mkdir(get_fname("test_ed_keys_init_all/keys"), 0700);
+  mkdir(keydir, 0700);
 #endif /* defined(_WIN32) */
 
   options->DataDirectory = dir;
+  options->KeyDirectory = keydir;
 
   tt_int_op(1, OP_EQ, load_ed_keys(options, now));
   tt_int_op(0, OP_EQ, generate_ed_link_cert(options, now, 0));
@@ -521,7 +523,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
   /* Demonstrate that we can start up with no secret identity key */
   routerkeys_free_all();
-  unlink(get_fname("test_ed_keys_init_all/keys/"
+  unlink(get_fname("test_ed_keys_init_all/KEYS/"
                    "ed25519_master_id_secret_key"));
   tt_int_op(1, OP_EQ, load_ed_keys(options, now));
   tt_int_op(0, OP_EQ, generate_ed_link_cert(options, now, 0));
@@ -542,6 +544,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
  done:
   tor_free(dir);
+  tor_free(keydir);
   tor_free(options);
   tor_cert_free(link_cert);
   routerkeys_free_all();
diff --git a/src/test/testing_common.c b/src/test/testing_common.c
index e43fa46c8..142c68107 100644
--- a/src/test/testing_common.c
+++ b/src/test/testing_common.c
@@ -293,6 +293,9 @@ main(int c, const char **v)
   setup_directory();
   options_init(options);
   options->DataDirectory = tor_strdup(temp_dir);
+  tor_asprintf(&options->KeyDirectory, "%s"PATH_SEPARATOR"keys",
+               options->DataDirectory);
+  options->CacheDirectory = tor_strdup(temp_dir);
   options->EntryStatistics = 1;
   if (set_options(options, &errmsg) < 0) {
     printf("Failed to set initial options: %s\n", errmsg);



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits