r15877@catbus: nickm | 2007-10-17 12:54:56 -0400

Make unverified-consensus get removed when it is accepted or rejected.  Make a new get_datadir_fname*() set of functions to eliminate the common code of "get the options, get the datadir, append some stuff".


svn:r12000

ChangeLog

@@ -33,6 +33,7 @@ Changes in version 0.2.0.9-alpha - 2007-10-??
     - When we're configured to be a v3 authority, but we're only listed
       as a non-v3 authority in our DirServer line for ourself, correct the
       listing.
+    - Delete unverified-consensus when the real consensus is set.
 
   o Minor bugfixes (memory leaks):
     - Stop leaking memory on failing case of base32_decode.  Bugfix on
@@ -48,7 +49,7 @@ Changes in version 0.2.0.9-alpha - 2007-10-??
       This may result in bandwidth accounting errors if you try to upgrade
       from 0.1.1.x or earlier, or if you try to downgrade to 0.1.1.x or
       earlier.
-
+    - New convenience code to locate a file within the DataDirectory.
 
 Changes in version 0.2.0.8-alpha - 2007-10-12
   o Major features (router descriptor cache):

src/or/config.c

@@ -4312,18 +4312,57 @@ get_or_state(void)
   return global_state;
 }
 
-/** Return the filename used to write and read the persistent state. */
-static char *
-get_or_state_fname(void)
+/** 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
+ * 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.
+ */
+char *
+get_datadir_fname2_suffix(const char *sub1, const char *sub2,
+                          const char *suffix)
 {
-  char *fname = NULL;
   or_options_t *options = get_options();
-  size_t len = strlen(options->DataDirectory) + 16;
+  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);
-  tor_snprintf(fname, len, "%s"PATH_SEPARATOR"state", options->DataDirectory);
+  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);
+    }
+  } else {
+    strlcpy(fname, options->DataDirectory, len);
+  }
+  if (suffix)
+    strlcat(fname, suffix, len);
   return fname;
 }
 
+
 /** Return 0 if every setting in <b>state</b> is reasonable, and a
  * permissible transition from <b>old_state</b>.  Else warn and return -1.
  * Should have no side effects, except for normalizing the contents of
@@ -4375,7 +4414,7 @@ or_state_load(void)
   char *errmsg = NULL;
   int r = -1, badstate = 0;
 
-  fname = get_or_state_fname();
+  fname = get_datadir_fname("state");
   switch (file_status(fname)) {
     case FN_FILE:
       if (!(contents = read_file_to_str(fname, 0, NULL))) {
@@ -4510,7 +4549,7 @@ or_state_save(time_t now)
                "# You *do not* need to edit this file.\n\n%s",
                tbuf, state);
   tor_free(state);
-  fname = get_or_state_fname();
+  fname = get_datadir_fname("state");
   if (write_str_to_file(fname, contents, 0)<0) {
     log_warn(LD_FS, "Unable to write state to file \"%s\"", fname);
     tor_free(fname);

src/or/control.c

@@ -1397,19 +1397,15 @@ getinfo_helper_dir(control_connection_t *control_conn,
     } else {
       smartlist_t *fp_list = smartlist_create();
       smartlist_t *status_list = smartlist_create();
-      size_t fn_len = strlen(get_options()->DataDirectory)+HEX_DIGEST_LEN+32;
-      char *fn = tor_malloc(fn_len+1);
-      char hex_id[HEX_DIGEST_LEN+1];
       dirserv_get_networkstatus_v2_fingerprints(
                              fp_list, question+strlen("dir/status/"));
       SMARTLIST_FOREACH(fp_list, const char *, fp, {
           char *s;
-          base16_encode(hex_id, sizeof(hex_id), fp, DIGEST_LEN);
-          tor_snprintf(fn, fn_len, "%s/cached-status/%s",
-                       get_options()->DataDirectory, hex_id);
-          s = read_file_to_str(fn, 0, NULL);
+          char *fname = networkstatus_get_cache_filename(fp);
+          s = read_file_to_str(fname, 0, NULL);
           if (s)
             smartlist_add(status_list, s);
+          tor_free(fname);
         });
       SMARTLIST_FOREACH(fp_list, char *, fp, tor_free(fp));
       smartlist_free(fp_list);
@@ -3565,11 +3561,7 @@ get_cookie_file(void)
   if (options->CookieAuthFile && strlen(options->CookieAuthFile)) {
     return tor_strdup(options->CookieAuthFile);
   } else {
-    const char *datadir = get_options()->DataDirectory;
-    size_t len = strlen(datadir)+64;
-    char *fname = tor_malloc(len);
-    tor_snprintf(fname, len, "%s"PATH_SEPARATOR"control_auth_cookie", datadir);
-    return fname;
+    return get_datadir_fname("control_auth_cookie");
   }
 }
 

src/or/dirserv.c

@@ -178,7 +178,7 @@ dirserv_add_own_fingerprint(const char *nickname, crypto_pk_env_t *pk)
 int
 dirserv_load_fingerprint_file(void)
 {
-  char fname[512];
+  char *fname;
   char *cf;
   char *nickname, *fingerprint;
   authdir_config_t *fingerprint_list_new;
@@ -186,8 +186,7 @@ dirserv_load_fingerprint_file(void)
   config_line_t *front=NULL, *list;
   or_options_t *options = get_options();
 
-  tor_snprintf(fname, sizeof(fname),
-               "%s/approved-routers", options->DataDirectory);
+  fname = get_datadir_fname("approved-routers");
   log_info(LD_GENERAL,
            "Reloading approved fingerprints from \"%s\"...", fname);
 
@@ -195,12 +194,16 @@ dirserv_load_fingerprint_file(void)
   if (!cf) {
     if (options->NamingAuthoritativeDir) {
       log_warn(LD_FS, "Cannot open fingerprint file '%s'. Failing.", fname);
+      tor_free(fname);
       return -1;
     } else {
       log_info(LD_FS, "Cannot open fingerprint file '%s'. Returning.", fname);
+      tor_free(fname);
       return 0;
     }
   }
+  tor_free(fname);
+
   result = config_get_lines(cf, &front);
   tor_free(cf);
   if (result < 0) {

src/or/hibernate.c

@@ -575,10 +575,9 @@ read_bandwidth_usage(void)
   or_state_t *state = get_or_state();
 
   {
-    char fname[512];
-    tor_snprintf(fname, sizeof(fname), "%s/bw_accounting",
-                 get_options()->DataDirectory);
+    char *fname = get_datadir_fname("bw_accounting");
     unlink(fname);
+    tor_free(fname);
   }
 
   if (!state)

src/or/networkstatus.c

@@ -118,17 +118,15 @@ networkstatus_reset_download_failures(void)
 int
 router_reload_v2_networkstatus(void)
 {
-  char filename[512];
   smartlist_t *entries;
   struct stat st;
   char *s;
-  tor_assert(get_options()->DataDirectory);
+  char *filename = get_datadir_fname("cached-status");
   if (!networkstatus_v2_list)
     networkstatus_v2_list = smartlist_create();
 
-  tor_snprintf(filename,sizeof(filename),"%s"PATH_SEPARATOR"cached-status",
-               get_options()->DataDirectory);
   entries = tor_listdir(filename);
+  tor_free(filename);
   SMARTLIST_FOREACH(entries, const char *, fn, {
       char buf[DIGEST_LEN];
       if (strlen(fn) != HEX_DIGEST_LEN ||
@@ -137,9 +135,7 @@ router_reload_v2_networkstatus(void)
                  "Skipping cached-status file with unexpected name \"%s\"",fn);
         continue;
       }
-      tor_snprintf(filename,sizeof(filename),
-                   "%s"PATH_SEPARATOR"cached-status"PATH_SEPARATOR"%s",
-                   get_options()->DataDirectory, fn);
+      filename = get_datadir_fname2("cached-status", fn);
       s = read_file_to_str(filename, 0, &st);
       if (s) {
         if (router_set_networkstatus_v2(s, st.st_mtime, NS_FROM_CACHE,
@@ -148,6 +144,7 @@ router_reload_v2_networkstatus(void)
         }
         tor_free(s);
       }
+      tor_free(filename);
     });
   SMARTLIST_FOREACH(entries, char *, fn, tor_free(fn));
   smartlist_free(entries);
@@ -160,13 +157,12 @@ router_reload_v2_networkstatus(void)
 int
 router_reload_consensus_networkstatus(void)
 {
-  char filename[512];
+  char *filename;
   char *s;
 
   /* XXXX020 Suppress warnings if cached consensus is bad. */
 
-  tor_snprintf(filename,sizeof(filename),"%s"PATH_SEPARATOR"cached-consensus",
-               get_options()->DataDirectory);
+  filename = get_datadir_fname("cached-consensus");
   s = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
   if (s) {
     if (networkstatus_set_current_consensus(s, 1, 0)) {
@@ -175,10 +171,9 @@ router_reload_consensus_networkstatus(void)
     }
     tor_free(s);
   }
+  tor_free(filename);
 
-  tor_snprintf(filename,sizeof(filename),
-               "%s"PATH_SEPARATOR"unverified-consensus",
-               get_options()->DataDirectory);
+  filename = get_datadir_fname("unverified-consensus");
   s = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
   if (s) {
     if (networkstatus_set_current_consensus(s, 1, 1)) {
@@ -187,6 +182,7 @@ router_reload_consensus_networkstatus(void)
     }
     tor_free(s);
   }
+  tor_free(filename);
 
   return 0;
 }
@@ -221,14 +217,9 @@ networkstatus_v2_free(networkstatus_v2_t *ns)
 char *
 networkstatus_get_cache_filename(const char *identity_digest)
 {
-  const char *datadir = get_options()->DataDirectory;
-  size_t len = strlen(datadir)+64;
   char fp[HEX_DIGEST_LEN+1];
-  char *fn = tor_malloc(len+1);
   base16_encode(fp, HEX_DIGEST_LEN+1, identity_digest, DIGEST_LEN);
-  tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status"PATH_SEPARATOR"%s",
-               datadir,fp);
-  return fn;
+  return get_datadir_fname2("cached-status", fp);
 }
 
 /** Helper for smartlist_sort: Compare two networkstatus objects by
@@ -901,16 +892,20 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
                                     int was_waiting_for_certs)
 {
   networkstatus_vote_t *c;
-  int r;
+  int r, result=-1;
   time_t now = time(NULL);
+  char *unverified_fname = NULL, *consensus_fname = NULL;
 
   /* Make sure it's parseable. */
   c = networkstatus_parse_vote_from_string(consensus, NULL, 0);
   if (!c) {
     log_warn(LD_DIR, "Unable to parse networkstatus consensus");
-    return -1;
+    goto done;
   }
 
+  consensus_fname = get_datadir_fname("cached-consensus");
+  unverified_fname = get_datadir_fname("unverified-consensus");
+
   /* Make sure it's signed enough. */
   if ((r=networkstatus_check_consensus_signature(c, 1))<0) {
     if (r == -1 && !was_waiting_for_certs) {
@@ -927,23 +922,27 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
         consensus_waiting_for_certs_body = tor_strdup(consensus);
         /*XXXX020 delay next update. NMNM */
         if (!from_cache) {
-          or_options_t *options = get_options();
-          char filename[512];
-          tor_snprintf(filename, sizeof(filename),
-                       "%s"PATH_SEPARATOR"unverified-consensus",
-                       options->DataDirectory);
-          write_str_to_file(filename, consensus, 0);
+          write_str_to_file(unverified_fname, consensus, 0);
         }
         authority_certs_fetch_missing(c, now);
+      } else {
+        /* Even if we had enough signatures, we'd never use this as the
+         * latest consensus. */
+        if (was_waiting_for_certs && from_cache)
+          unlink(unverified_fname);
       }
       download_status_reset(&consensus_dl_status); /*XXXX020 not quite right.*/
-      return 0;
+      result = 0;
+      goto done;
     } else {
+      /* This can never be signed enough Kill it. */
       if (!was_waiting_for_certs)
         log_warn(LD_DIR, "Not enough good signatures on networkstatus "
                  "consensus");
+      if (was_waiting_for_certs && from_cache)
+        unlink(unverified_fname);
       networkstatus_vote_free(c);
-      return -1;
+      goto done;
     }
   }
 
@@ -964,6 +963,7 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
     consensus_waiting_for_certs = NULL;
     if (consensus != consensus_waiting_for_certs_body)
       tor_free(consensus_waiting_for_certs_body);
+    unlink(unverified_fname);
   }
 
   current_consensus = c;
@@ -973,12 +973,7 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
   routerstatus_list_update_named_server_map();
 
   if (!from_cache) {
-    or_options_t *options = get_options();
-    char filename[512];
-    tor_snprintf(filename, sizeof(filename),
-                 "%s"PATH_SEPARATOR"cached-consensus",
-                 options->DataDirectory);
-    write_str_to_file(filename, consensus, 0);
+    write_str_to_file(consensus_fname, consensus, 0);
   }
 
   if (dirserver_mode(get_options()))
@@ -986,7 +981,11 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
 
   router_dir_info_changed();
 
-  return 0;
+  result = 0;
+ done:
+  tor_free(consensus_fname);
+  tor_free(unverified_fname);
+  return result;
 }
 
 /** DOCDOC */

src/or/or.h

@@ -2443,6 +2443,14 @@ config_line_t *option_get_assignment(or_options_t *options,
                                      const char *key);
 int options_save_current(void);
 const char *get_torrc_fname(void);
+char *get_datadir_fname2_suffix(const char *sub1, const char *sub2,
+                                const char *suffix);
+/**DOCDOC*/
+#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)
+#define get_datadir_fname_suffix(sub1, suffix) \
+  get_datadir_fname2_suffix((sub1), NULL, (suffix))
 
 or_state_t *get_or_state(void);
 int or_state_save(time_t now);

src/or/rephist.c

@@ -595,18 +595,6 @@ rep_history_clean(time_t before)
   }
 }
 
-/** Return a newly allocated string holding the filename in which we store
- * MTBF information. */
-static char *
-get_mtbf_filename(void)
-{
-  const char *datadir = get_options()->DataDirectory;
-  size_t len = strlen(datadir)+32;
-  char *fn = tor_malloc(len);
-  tor_snprintf(fn, len, "%s"PATH_SEPARATOR"router-stability", datadir);
-  return fn;
-}
-
 /** Write MTBF data to disk.  Returns 0 on success, negative on failure. */
 int
 rep_hist_record_mtbf_data(void)
@@ -621,7 +609,7 @@ rep_hist_record_mtbf_data(void)
   FILE *f;
 
   {
-    char *filename = get_mtbf_filename();
+    char *filename = get_datadir_fname("router-stability");
     f = start_writing_to_stdio_file(filename, OPEN_FLAGS_REPLACE|O_TEXT, 0600,
                                     &open_file);
     tor_free(filename);
@@ -725,7 +713,7 @@ rep_hist_load_mtbf_data(time_t now)
   long format = -1;
 
   {
-    char *filename = get_mtbf_filename();
+    char *filename = get_datadir_fname("router-stability");
     char *d = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
     tor_free(filename);
     if (!d)

src/or/router.c

@@ -153,17 +153,12 @@ get_my_v3_authority_signing_key(void)
 void
 rotate_onion_key(void)
 {
-  char fname[512];
-  char fname_prev[512];
+  char *fname, *fname_prev;
   crypto_pk_env_t *prkey;
   or_state_t *state = get_or_state();
   time_t now;
-  tor_snprintf(fname,sizeof(fname),
-               "%s"PATH_SEPARATOR"keys"PATH_SEPARATOR"secret_onion_key",
-               get_options()->DataDirectory);
-  tor_snprintf(fname_prev,sizeof(fname_prev),
-               "%s"PATH_SEPARATOR"keys"PATH_SEPARATOR"secret_onion_key.old",
-               get_options()->DataDirectory);
+  fname = get_datadir_fname2("keys", "secret_onion_key");
+  fname_prev = get_datadir_fname2("keys", "secret_onion_key.old");
   if (!(prkey = crypto_new_pk_env())) {
     log_err(LD_GENERAL,"Error constructing rotated onion key");
     goto error;
@@ -191,9 +186,12 @@ rotate_onion_key(void)
   tor_mutex_release(key_lock);
   mark_my_descriptor_dirty();
   or_state_mark_dirty(state, get_options()->AvoidDiskWrites ? now+3600 : 0);
-  return;
+  goto done;
  error:
   log_warn(LD_GENERAL, "Couldn't rotate onion key.");
+ done:
+  tor_free(fname);
+  tor_free(fname_prev);
 }
 
 /** Try to read an RSA key from <b>fname</b>.  If <b>fname</b> doesn't exist
@@ -372,11 +370,11 @@ v3_authority_check_key_expiry(void)
 int
 init_keys(void)
 {
-  char keydir[512];
+  char *keydir;
   char fingerprint[FINGERPRINT_LEN+1];
   /*nickname<space>fp\n\0 */
   char fingerprint_line[MAX_NICKNAME_LEN+FINGERPRINT_LEN+3];
-  const char *mydesc, *datadir;
+  const char *mydesc;
   crypto_pk_env_t *prkey;
   char digest[20];
   char v3_digest[20];
@@ -408,15 +406,16 @@ init_keys(void)
     return 0;
   }
   /* Make sure DataDirectory exists, and is private. */
-  datadir = options->DataDirectory;
-  if (check_private_dir(datadir, CPD_CREATE)) {
+  if (check_private_dir(options->DataDirectory, CPD_CREATE)) {
     return -1;
   }
   /* Check the key directory. */
-  tor_snprintf(keydir,sizeof(keydir),"%s"PATH_SEPARATOR"keys", datadir);
+  keydir = get_datadir_fname("keys");
   if (check_private_dir(keydir, CPD_CREATE)) {
+    tor_free(keydir);
     return -1;
   }
+  tor_free(keydir);
 
   /* 1a. Read v3 directory authority key/cert information. */
   memset(v3_digest, 0, sizeof(v3_digest));
@@ -430,18 +429,18 @@ init_keys(void)
   }
 
   /* 1. Read identity key. Make it if none is found. */
-  tor_snprintf(keydir,sizeof(keydir),
-               "%s"PATH_SEPARATOR"keys"PATH_SEPARATOR"secret_id_key",datadir);
+  keydir = get_datadir_fname2("keys", "secret_id_key");
   log_info(LD_GENERAL,"Reading/making identity key \"%s\"...",keydir);
   prkey = init_key_from_file(keydir, 1, LOG_ERR);
+  tor_free(keydir);
   if (!prkey) return -1;
   set_identity_key(prkey);
 
   /* 2. Read onion key.  Make it if none is found. */
-  tor_snprintf(keydir,sizeof(keydir),
-             "%s"PATH_SEPARATOR"keys"PATH_SEPARATOR"secret_onion_key",datadir);
+  keydir = get_datadir_fname2("keys", "secret_onion_key");
   log_info(LD_GENERAL,"Reading/making onion key \"%s\"...",keydir);
   prkey = init_key_from_file(keydir, 1, LOG_ERR);
+  tor_free(keydir);
   if (!prkey) return -1;
   set_onion_key(prkey);
   if (options->command == CMD_RUN_TOR) {
@@ -463,13 +462,13 @@ init_keys(void)
     }
   }
 
-  tor_snprintf(keydir,sizeof(keydir),
-         "%s"PATH_SEPARATOR"keys"PATH_SEPARATOR"secret_onion_key.old",datadir);
+  keydir = get_datadir_fname2("keys", "secret_onion_key.old");
   if (file_status(keydir) == FN_FILE) {
     prkey = init_key_from_file(keydir, 1, LOG_ERR);
     if (prkey)
       lastonionkey = prkey;
   }
+  tor_free(keydir);
 
   /* 3. Initialize link key and TLS context. */
   if (tor_tls_context_new(get_identity_key(), options->Nickname,
@@ -503,8 +502,9 @@ init_keys(void)
   }
 
   /* 5. Dump fingerprint to 'fingerprint' */
-  tor_snprintf(keydir,sizeof(keydir),"%s"PATH_SEPARATOR"fingerprint", datadir);
+  keydir = get_datadir_fname("fingerprint");
   log_info(LD_GENERAL,"Dumping fingerprint to \"%s\"...",keydir);
+  tor_free(keydir);
   if (crypto_pk_get_fingerprint(get_identity_key(), fingerprint, 1)<0) {
     log_err(LD_GENERAL,"Error computing fingerprint");
     return -1;

src/or/routerlist.c

@@ -87,13 +87,13 @@ get_n_authorities(authority_type_t type)
 int
 trusted_dirs_reload_certs(void)
 {
-  char filename[512];
+  char *filename;
   char *contents;
   int r;
 
-  tor_snprintf(filename,sizeof(filename),"%s"PATH_SEPARATOR"cached-certs",
-               get_options()->DataDirectory);
+  filename = get_datadir_fname("cached-certs");
   contents = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
+  tor_free(filename);
   if (!contents)
     return 0;
   r = trusted_dirs_load_certs_from_string(contents, 1);
@@ -161,7 +161,7 @@ trusted_dirs_load_certs_from_string(const char *contents, int from_store)
 void
 trusted_dirs_flush_certs_to_disk(void)
 {
-  char filename[512];
+  char *filename;
   smartlist_t *chunks;
 
   if (!trusted_dir_servers_certs_changed)
@@ -169,8 +169,6 @@ trusted_dirs_flush_certs_to_disk(void)
 
   chunks = smartlist_create();
 
-  tor_snprintf(filename,sizeof(filename),"%s"PATH_SEPARATOR"cached-certs",
-               get_options()->DataDirectory);
   SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, ds,
   {
       if (ds->v3_certs) {
@@ -183,9 +181,11 @@ trusted_dirs_flush_certs_to_disk(void)
           });
       }
   });
+  filename = get_datadir_fname("cached-certs");
   if (write_chunks_to_file(filename, chunks, 0)) {
     log_warn(LD_FS, "Error writing certificates to disk.");
   }
+  tor_free(filename);
   SMARTLIST_FOREACH(chunks, sized_chunk_t *, c, tor_free(c));
   smartlist_free(chunks);
 
@@ -415,16 +415,10 @@ static int
 signed_desc_append_to_journal(signed_descriptor_t *desc,
                               desc_store_t *store)
 {
-  or_options_t *options = get_options();
-  size_t fname_len = strlen(options->DataDirectory)+32;
-  char *fname;
+  char *fname = get_datadir_fname_suffix(store->fname_base, ".new");
   const char *body = signed_descriptor_get_body_impl(desc,1);
   size_t len = desc->signed_descriptor_len + desc->annotations_len;
 
-  fname = tor_malloc(fname_len);
-  tor_snprintf(fname, fname_len, "%s"PATH_SEPARATOR"%s.new",
-               options->DataDirectory, store->fname_base);
-
   tor_assert(len == strlen(body));
 
   if (append_bytes_to_file(fname, body, len, 1)) {
@@ -462,7 +456,6 @@ static int
 router_rebuild_store(int force, desc_store_t *store)
 {
   or_options_t *options;
-  size_t fname_len;
   smartlist_t *chunk_list = NULL;
   char *fname = NULL, *fname_tmp = NULL;
   int r = -1;
@@ -483,13 +476,9 @@ router_rebuild_store(int force, desc_store_t *store)
   log_info(LD_DIR, "Rebuilding %s cache", store->description);
 
   options = get_options();
-  fname_len = strlen(options->DataDirectory)+32;
-  fname = tor_malloc(fname_len);
-  fname_tmp = tor_malloc(fname_len);
-  tor_snprintf(fname, fname_len, "%s"PATH_SEPARATOR"%s",
-               options->DataDirectory, store->fname_base);
-  tor_snprintf(fname_tmp, fname_len, "%s"PATH_SEPARATOR"%s.tmp",
-               options->DataDirectory, store->fname_base);
+
+  fname = get_datadir_fname(store->fname_base);
+  fname_tmp = get_datadir_fname_suffix(store->fname_base, ".tmp");
 
   chunk_list = smartlist_create();
 
@@ -571,9 +560,8 @@ router_rebuild_store(int force, desc_store_t *store)
       signed_descriptor_get_body(sd); /* reconstruct and assert */
     });
 
-  tor_snprintf(fname, fname_len, "%s"PATH_SEPARATOR"%s.new",
-               options->DataDirectory, store->fname_base);
-
+  tor_free(fname);
+  fname = get_datadir_fname_suffix(store->fname_base, ".new");
   write_str_to_file(fname, "", 1);
 
   r = 0;
@@ -597,23 +585,16 @@ router_rebuild_store(int force, desc_store_t *store)
 static int
 router_reload_router_list_impl(desc_store_t *store)
 {
-  or_options_t *options = get_options();
-  size_t fname_len = strlen(options->DataDirectory)+32;
-  char *fname = tor_malloc(fname_len), *altname = NULL,
-    *contents = NULL;
+  char *fname = NULL, *altname = NULL, *contents = NULL;
   struct stat st;
   int read_from_old_location = 0;
   int extrainfo = (store->type == EXTRAINFO_STORE);
   time_t now = time(NULL);
   store->journal_len = store->store_len = 0;
 
-  tor_snprintf(fname, fname_len, "%s"PATH_SEPARATOR"%s",
-               options->DataDirectory, store->fname_base);
-  if (store->fname_alt_base) {
-    altname = tor_malloc(fname_len);
-    tor_snprintf(altname, fname_len, "%s"PATH_SEPARATOR"%s",
-                 options->DataDirectory, store->fname_alt_base);
-  }
+  fname = get_datadir_fname(store->fname_base);
+  if (store->fname_alt_base)
+    altname = get_datadir_fname(store->fname_alt_base);
 
   if (store->mmap) /* get rid of it first */
     tor_munmap_file(store->mmap);
@@ -642,13 +623,13 @@ router_reload_router_list_impl(desc_store_t *store)
                                       SAVED_IN_CACHE, NULL, 0, NULL);
   }
 
-  tor_snprintf(fname, fname_len, "%s"PATH_SEPARATOR"%s.new",
-               options->DataDirectory, store->fname_base);
+  tor_free(fname);
+  fname = get_datadir_fname_suffix(store->fname_base, ".new");
   if (file_status(fname) == FN_FILE)
     contents = read_file_to_str(fname, RFTS_BIN|RFTS_IGNORE_MISSING, &st);
   if (read_from_old_location) {
-    tor_snprintf(altname, fname_len, "%s"PATH_SEPARATOR"%s.new",
-                 options->DataDirectory, store->fname_alt_base);
+    tor_free(altname);
+    altname = get_datadir_fname_suffix(store->fname_alt_base, ".new");
     if (!contents)
       contents = read_file_to_str(altname, RFTS_BIN|RFTS_IGNORE_MISSING, &st);
     else

src/or/test.c

@@ -3033,6 +3033,36 @@ test_util_mempool(void)
   smartlist_free(allocated);
 }
 
+static void
+test_util_datadir(void)
+{
+  char buf[1024];
+  char *f;
+
+  f = get_datadir_fname(NULL);
+  test_streq(f, temp_dir);
+  tor_free(f);
+  f = get_datadir_fname("state");
+  tor_snprintf(buf, sizeof(buf), "%s"PATH_SEPARATOR"state", temp_dir);
+  test_streq(f, buf);
+  tor_free(f);
+  f = get_datadir_fname2("cache", "thingy");
+  tor_snprintf(buf, sizeof(buf),
+               "%s"PATH_SEPARATOR"cache"PATH_SEPARATOR"thingy", temp_dir);
+  test_streq(f, buf);
+  tor_free(f);
+  f = get_datadir_fname2_suffix("cache", "thingy", ".foo");
+  tor_snprintf(buf, sizeof(buf),
+               "%s"PATH_SEPARATOR"cache"PATH_SEPARATOR"thingy.foo", temp_dir);
+  test_streq(f, buf);
+  tor_free(f);
+  f = get_datadir_fname_suffix("cache", ".foo");
+  tor_snprintf(buf, sizeof(buf), "%s"PATH_SEPARATOR"cache.foo",
+               temp_dir);
+  test_streq(f, buf);
+  tor_free(f);
+}
+
 /* Test AES-CTR encryption and decryption with IV. */
 static void
 test_crypto_aes_iv(void)
@@ -3194,6 +3224,7 @@ static struct {
   ENT(util),
   SUBENT(util, ip6_helpers),
   SUBENT(util, gzip),
+  SUBENT(util, datadir),
   SUBENT(util, smartlist),
   SUBENT(util, bitarray),
   SUBENT(util, mempool),