r9274@Kushana: nickm | 2006-10-19 16:16:58 -0400

Add unit tests for tor_mmap_file(); make tor_mmap_t.size always be the size of the file (not the size of the mapping); add an extra argument to read_file_to_str() so it can return the size of the result string.


svn:r8762

ChangeLog

@@ -42,6 +42,9 @@ Changes in version 0.1.2.3-alpha - 2006-10-??
       service circuits.
     - Correctly set maximum connection limit on Cygwin.
     - Try to detect windows correctly when cross-compiling.
+    - Detect the size of the routers file correctly even if it is corrupted
+      (on systems without mmap) or not page-aligned (on systems with mmap).
+      This bug was harmless.
 
 
 Changes in version 0.1.2.2-alpha - 2006-10-07

src/common/compat.c

@@ -113,14 +113,19 @@ const char compat_c_id[] =
 #endif
 
 #ifdef HAVE_SYS_MMAN_H
+typedef struct tor_mmap_impl_t {
+  tor_mmap_t base;
+  size_t mapping_size; /**< Size of the actual mapping. (This is this file
+                        * size, rounded up to the nearest page.) */
+} tor_mmap_impl_t;
 tor_mmap_t *
 tor_mmap_file(const char *filename)
 {
   int fd; /* router file */
   char *string;
   int page_size;
-  tor_mmap_t *res;
-  size_t size;
+  tor_mmap_impl_t *res;
+  size_t size, filesize;
 
   tor_assert(filename);
 
@@ -130,7 +135,7 @@ tor_mmap_file(const char *filename)
     return NULL;
   }
 
-  size = lseek(fd, 0, SEEK_END);
+  size = filesize = lseek(fd, 0, SEEK_END);
   lseek(fd, 0, SEEK_SET);
   /* ensure page alignment */
   page_size = getpagesize();
@@ -146,28 +151,31 @@ tor_mmap_file(const char *filename)
 
   close(fd);
 
-  res = tor_malloc_zero(sizeof(tor_mmap_t));
-  res->data = string;
-  res->size = size;
+  res = tor_malloc_zero(sizeof(tor_mmap_impl_t));
+  res->base.data = string;
+  res->base.size = filesize;
+  res->mapping_size = size;
 
-  return res;
+  return &(res->base);
 }
 void
 tor_munmap_file(tor_mmap_t *handle)
 {
-  munmap((char*)handle->data, handle->size);
-  tor_free(handle);
+  tor_mmap_impl_t *h = (tor_mmap_impl_t*)
+    (((char*)handle) - STRUCT_OFFSET(tor_mmap_impl_t, base));
+  munmap((char*)h->base.data, h->mapping_size);
+  tor_free(h);
 }
 #elif defined(MS_WINDOWS)
 typedef struct win_mmap_t {
   tor_mmap_t base;
   HANDLE file_handle;
   HANDLE mmap_handle;
-} tor_mmap_impl_t;
+} win_mmap_t;
 tor_mmap_t *
 tor_mmap_file(const char *filename)
 {
-  struct win_mmap_t *res = tor_malloc_zero(sizeof(struct win_mmap_t));
+  win_mmap_t *res = tor_malloc_zero(sizeof(win_mmap_t));
   res->mmap_handle = res->file_handle = INVALID_HANDLE_VALUE;
 
   res->file_handle = CreateFile(filename,
@@ -208,8 +216,8 @@ tor_mmap_file(const char *filename)
 void
 tor_munmap_file(tor_mmap_t *handle)
 {
-  struct win_mmap_t *h = (struct win_mmap_t*)
-    (((char*)handle) - STRUCT_OFFSET(struct win_mmap_t, base));
+  win_mmap_t *h = (win_mmap_t*)
+    (((char*)handle) - STRUCT_OFFSET(win_mmap_t, base));
   if (handle->data)
 
   /*this is an ugly cast, but without it, "data" in struct tor_mmap_t would
@@ -226,13 +234,14 @@ tor_munmap_file(tor_mmap_t *handle)
 tor_mmap_t *
 tor_mmap_file(const char *filename)
 {
-  char *res = read_file_to_str(filename, 1);
+  size_t size;
+  char *res = read_file_to_str(filename, 1, &size);
   tor_mmap_t *handle;
   if (! res)
     return NULL;
   handle = tor_malloc_zero(sizeof(tor_mmap_t));
   handle->data = res;
-  handle->size = strlen(res) + 1;
+  handle->size = size;
   return handle;
 }
 void

src/common/compat.h

@@ -129,10 +129,11 @@ size_t strlcpy(char *dst, const char *src, size_t siz) ATTR_NONNULL((1,2));
 #define U64_LITERAL(n) (n ## llu)
 #endif
 
-/** Opaque bookkeeping type used for mmap accounting. */
+/** Represents an mmaped file. Allocated via tor_mmap_file; freed with
+ * tor_munmap_file. */
 typedef struct tor_mmap_t {
-  const char *data;
-  size_t size;
+  const char *data; /**< Mapping of the file's contents. */
+  size_t size; /**< Size of the file. */
 } tor_mmap_t;
 
 tor_mmap_t *tor_mmap_file(const char *filename) ATTR_NONNULL((1));

src/common/crypto.c

@@ -460,7 +460,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_env_t *env,
   int r;
 
   /* Read the file into a string. */
-  contents = read_file_to_str(keyfile, 0);
+  contents = read_file_to_str(keyfile, 0, NULL);
   if (!contents) {
     log_warn(LD_CRYPTO, "Error reading private key from \"%s\"", keyfile);
     return -1;

src/common/util.c

@@ -1288,6 +1288,9 @@ append_bytes_to_file(const char *fname, const char *str, size_t len,
 
 /** Read the contents of <b>filename</b> into a newly allocated
  * string; return the string on success or NULL on failure.
+ *
+ * If <b>size_out</b> is provided, store the length of the result in
+ * <b>size_out</b>
  */
 /*
  * This function <em>may</em> return an erroneous result if the file
@@ -1297,7 +1300,7 @@ append_bytes_to_file(const char *fname, const char *str, size_t len,
  * be truncated.
  */
 char *
-read_file_to_str(const char *filename, int bin)
+read_file_to_str(const char *filename, int bin, size_t *size_out)
 {
   int fd; /* router file */
   struct stat statbuf;
@@ -1351,6 +1354,8 @@ read_file_to_str(const char *filename, int bin)
   }
 #endif
   close(fd);
+  if (size_out)
+    *size_out = (size_t) r;
 
   return string;
 }

src/common/util.h

@@ -180,7 +180,8 @@ int write_chunks_to_file(const char *fname, const struct smartlist_t *chunks,
 int append_bytes_to_file(const char *fname, const char *str, size_t len,
                          int bin);
 
-char *read_file_to_str(const char *filename, int bin) ATTR_MALLOC;
+char *read_file_to_str(const char *filename, int bin, size_t *size_out)
+  ATTR_MALLOC;
 char *parse_line_from_str(char *line, char **key_out, char **value_out);
 char *expand_filename(const char *filename);
 struct smartlist_t *tor_listdir(const char *dirname);

src/or/config.c

@@ -2801,7 +2801,7 @@ options_init_from_torrc(int argc, char **argv)
 
   /* get config lines, assign them */
   if (file_status(fname) != FN_FILE ||
-      !(cf = read_file_to_str(fname,0))) {
+      !(cf = read_file_to_str(fname,0,NULL))) {
     if (using_default_torrc == 1) {
       log(LOG_NOTICE, LD_CONFIG, "Configuration file \"%s\" not present, "
           "using reasonable defaults.", fname);
@@ -3421,7 +3421,7 @@ write_configuration_file(const char *fname, or_options_t *options)
   if (fname) {
     switch (file_status(fname)) {
       case FN_FILE:
-        old_val = read_file_to_str(fname, 0);
+        old_val = read_file_to_str(fname, 0, NULL);
         if (strcmpstart(old_val, GENERATED_FILE_PREFIX)) {
           rename_old = 1;
         }
@@ -3835,7 +3835,7 @@ or_state_load(void)
   fname = get_or_state_fname();
   switch (file_status(fname)) {
     case FN_FILE:
-      if (!(contents = read_file_to_str(fname, 0))) {
+      if (!(contents = read_file_to_str(fname, 0, NULL))) {
         log_warn(LD_FS, "Unable to read state file \"%s\"", fname);
         goto done;
       }

src/or/dirserv.c

@@ -148,7 +148,7 @@ dirserv_load_fingerprint_file(void)
   log_info(LD_GENERAL,
            "Reloading approved fingerprints from \"%s\"...", fname);
 
-  cf = read_file_to_str(fname, 0);
+  cf = read_file_to_str(fname, 0, NULL);
   if (!cf) {
     if (options->NamingAuthoritativeDir) {
       log_warn(LD_FS, "Cannot open fingerprint file '%s'. Failing.", fname);

src/or/hibernate.c

@@ -587,7 +587,7 @@ read_bandwidth_usage(void)
 
   tor_snprintf(fname, sizeof(fname), "%s/bw_accounting",
                get_options()->DataDirectory);
-  if (!(s = read_file_to_str(fname, 0))) {
+  if (!(s = read_file_to_str(fname, 0, NULL))) {
     return 0;
   }
   elts = smartlist_create();

src/or/routerlist.c

@@ -136,7 +136,7 @@ router_reload_networkstatus(void)
       }
       tor_snprintf(filename,sizeof(filename),"%s/cached-status/%s",
                    get_options()->DataDirectory, fn);
-      s = read_file_to_str(filename, 0);
+      s = read_file_to_str(filename, 0, NULL);
       if (s) {
         stat(filename, &st);
         if (router_set_networkstatus(s, st.st_mtime, NS_FROM_CACHE, NULL)<0) {
@@ -369,7 +369,7 @@ router_reload_router_list(void)
 
   tor_snprintf(fname, fname_len, "%s/cached-routers.new",
                options->DataDirectory);
-  contents = read_file_to_str(fname, 1);
+  contents = read_file_to_str(fname, 1, NULL);
   if (contents) {
     stat(fname, &st);
     router_load_routers_from_string(contents,

src/or/test.c

@@ -1163,6 +1163,58 @@ test_strmap(void)
   strmap_free(map,NULL);
 }
 
+static void
+test_mmap(void)
+{
+  char *fname1 = tor_strdup(get_fname("mapped_1"));
+  char *fname2 = tor_strdup(get_fname("mapped_2"));
+  char *fname3 = tor_strdup(get_fname("mapped_3"));
+  const size_t buflen = 17000;
+  char *buf = tor_malloc(17000);
+  tor_mmap_t *mapping;
+
+  crypto_rand(buf, buflen);
+
+  write_str_to_file(fname1, "Short file.", 1);
+  write_bytes_to_file(fname2, buf, buflen, 1);
+  write_bytes_to_file(fname3, buf, 16384, 1);
+
+  mapping = tor_mmap_file(fname1);
+  test_assert(mapping);
+  test_eq(mapping->size, strlen("Short file."));
+  test_streq(mapping->data, "Short file.");
+  /* make sure we can unlink. */
+  test_assert(unlink(fname1) == 0);
+  test_streq(mapping->data, "Short file.");
+  tor_munmap_file(mapping);
+
+  /* Make sure that we fail to map a no-longer-existant file. */
+  mapping = tor_mmap_file(fname1);
+  test_assert(mapping == NULL);
+
+  /* Now try a big file that stretches across a few pages and isn't aligned */
+  mapping = tor_mmap_file(fname2);
+  test_assert(mapping);
+  test_eq(mapping->size, buflen);
+  test_memeq(mapping->data, buf, buflen);
+  tor_munmap_file(mapping);
+
+  /* Now try a big aligned file. */
+  mapping = tor_mmap_file(fname3);
+  test_assert(mapping);
+  test_eq(mapping->size, 16384);
+  test_memeq(mapping->data, buf, 16384);
+
+  /* fname1 got unlinked above */
+  unlink(fname2);
+  unlink(fname3);
+
+  tor_free(fname1);
+  tor_free(fname2);
+  tor_free(fname3);
+  tor_free(buf);
+}
+
 static void
 test_control_formats(void)
 {
@@ -1771,6 +1823,7 @@ main(int c, char**v)
   test_strmap();
   test_control_formats();
   test_pqueue();
+  test_mmap();
   puts("\n========================= Onion Skins =====================");
   test_onion();
   test_onion_handshake();