r11774@catbus: nickm | 2007-02-12 16:31:47 -0500

 Handle errors on opening cached-routers* more uniformly and sanely: log not-found errors at level INFO, and all other errors at level WARN.  Needs testing on win32.


svn:r9569

6 files changed, 82 insertions, 12 deletions

diff --git a/ChangeLog b/ChangeLog
index bbcdc99d4..6deef56ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -25,6 +25,12 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
       bandwidth from those directories earlier than it on the list.
     - If we start a server with ClientOnly 1, then set ClientOnly to 0
       and hup, stop triggering an assert based on an empty onion_key.
+    - On platforms with no working mmap() equivalent, don't warn the
+      user when cached-routers doesn't exist.
+    - Warn the user when mmap() [or its equivalent] fails for some reason
+      other than file-not-found.
+    - Don't warn the user when cached-routers.new doesn't exist: that's
+      perfectly fine when starting up for the first time.
 
   o Minor features:
     - Warn the user when an application uses the obsolete binary v0
diff --git a/src/common/compat.c b/src/common/compat.c
index 4d37363a9..a95355c25 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -134,7 +134,9 @@ tor_mmap_file(const char *filename)
 
   fd = open(filename, O_RDONLY, 0);
   if (fd<0) {
-    log_info(LD_FS,"Could not open \"%s\" for mmap().",filename);
+    int severity = (errno == ENOENT) ? LOG_INFO : LOG_WARN;
+    log_fn(severity, LD_FS,"Could not open \"%s\" for mmap(): %s",filename,
+           strerror(errno));
     return NULL;
   }
 
@@ -199,10 +201,15 @@ tor_mmap_file(const char *filename)
                                 0);
 
   if (res->file_handle == INVALID_HANDLE_VALUE)
-    goto err;
+    goto win_err;
 
   res->base.size = GetFileSize(res->file_handle, NULL);
 
+  if (res->base.size == 0) {
+    log_info(LD_FS,"File \"%s\" is empty. Ignoring.",filename);
+    goto err;
+  }
+
   res->mmap_handle = CreateFileMapping(res->file_handle,
                                        NULL,
                                        PAGE_READONLY,
@@ -214,14 +221,22 @@ tor_mmap_file(const char *filename)
                                        (res->base.size & 0xfffffffful),
                                        NULL);
   if (res->mmap_handle == NULL)
-    goto err;
+    goto win_err;
   res->base.data = (char*) MapViewOfFile(res->mmap_handle,
                                          FILE_MAP_READ,
                                          0, 0, 0);
   if (!res->base.data)
-    goto err;
+    goto win_err;
 
   return &(res->base);
+ win_err: {
+    DWORD e = GetLastError();
+    int severity = (e == ERROR_FILE_NOT_FOUND || e == PATH_NOT_FOUND) ?
+      LOG_INFO : LOG_WARN;
+    char *msg = format_win32_error(e);
+    log_fn(LOG_INFO, "Couldn't mmap file \"%s\": %s", filename, msg);
+    tor_free(msg);
+  }
  err:
   tor_munmap_file(&res->base);
   return NULL;
@@ -247,7 +262,7 @@ tor_mmap_t *
 tor_mmap_file(const char *filename)
 {
   struct stat st;
-  char *res = read_file_to_str(filename, 1, &st);
+  char *res = read_file_to_str(filename, RFTS_BIN|RFTS_IGNORE_MISSING, &st);
   tor_mmap_t *handle;
   if (! res)
     return NULL;
@@ -1388,3 +1403,32 @@ network_init(void)
   return 0;
 }
 
+#ifdef MS_WINDOWS
+/** Return a newly allocated string describing the windows system error code
+ * <b>err</b>.  Note that error codes are different from errno.  Error codes
+ * come from GetLastError() when a winapi call fails.  errno is set only when
+ * ansi functions fail.  Whee. */
+char *
+format_win32_error(DWORD err)
+{
+  LPVOID str = NULL;
+  char *result;
+
+  /* Somebody once decided that this interface was better than strerror(). */
+  FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+                 FORMAT_MESSAGE_FROM_SYSTEM |
+                 FORMAT_MESSAGE_IGNORE_INSERTS,
+                 NULL, err,
+                 MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+                 (LPTSTR) &str,
+                 0, NULL);
+
+  if (str) {
+    result = tor_strdup((char*)str);
+    LocalFree(str); /* LocalFree != free() */
+  } else {
+    result = tor_strdup("<unformattable error>");
+  }
+  return result;
+}
+#endif
diff --git a/src/common/compat.h b/src/common/compat.h
index 358909141..2f1bb9e03 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -320,6 +320,11 @@ unsigned long tor_get_thread_id(void);
 #define tor_get_thread_id() (1UL)
 #endif
 
+/* Platform-specific helpers. */
+#ifdef MS_WINDOWS
+char *format_win32_error(DWORD err);
+#endif
+
 /*for some reason my compiler doesn't have these version flags defined
   a nice homework assignment for someone one day is to define the rest*/
 //these are the values as given on MSDN
diff --git a/src/common/util.c b/src/common/util.c
index a5b87c431..c55aa0329 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1351,7 +1351,11 @@ append_bytes_to_file(const char *fname, const char *str, size_t len,
  * 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>
+ * <b>size_out</b>.
+ *
+ * If <b>flags</b> &amp; RFTS_BIN, open the file in binary mode.
+ * If <b>flags</b> &amp; RFTS_IGNORE_MISSING, don't warn if the file
+ * doesn't exist.
  */
 /*
  * This function <em>may</em> return an erroneous result if the file
@@ -1361,24 +1365,29 @@ 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, struct stat *stat_out)
+read_file_to_str(const char *filename, int flags, struct stat *stat_out)
 {
   int fd; /* router file */
   struct stat statbuf;
   char *string;
   int r;
+  int bin = flags & RFTS_BIN;
 
   tor_assert(filename);
 
   fd = open(filename,O_RDONLY|(bin?O_BINARY:O_TEXT),0);
   if (fd<0) {
-    log_warn(LD_FS,"Could not open \"%s\".",filename);
+    int severity = LOG_WARN;
+    if (errno == ENOENT && (flags & RFTS_IGNORE_MISSING))
+      severity = LOG_INFO;
+    log_fn(severity, LD_FS,"Could not open \"%s\": %s ",filename,
+           strerror(errno));
     return NULL;
   }
 
   if (fstat(fd, &statbuf)<0) {
     close(fd);
-    log_info(LD_FS,"Could not fstat \"%s\".",filename);
+    log_warn(LD_FS,"Could not fstat \"%s\".",filename);
     return NULL;
   }
 
diff --git a/src/common/util.h b/src/common/util.h
index 5b6dafdaa..9d29a420e 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -189,8 +189,13 @@ 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);
 
+/** Flag for read_file_to_str: open the file in binary mode. */
+#define RFTS_BIN            1
+/** Flag for read_file_to_str: it's okay if the file doesn't exist */
+#define RFTS_IGNORE_MISSING 2
+
 struct stat;
-char *read_file_to_str(const char *filename, int bin, struct stat *stat_out)
+char *read_file_to_str(const char *filename, int flags, struct stat *stat_out)
   ATTR_MALLOC;
 char *parse_line_from_str(char *line, char **key_out, char **value_out);
 char *expand_filename(const char *filename);
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index dc98a1abb..97b739ef8 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -353,7 +353,7 @@ router_reload_router_list(void)
 {
   or_options_t *options = get_options();
   size_t fname_len = strlen(options->DataDirectory)+32;
-  char *fname = tor_malloc(fname_len), *contents;
+  char *fname = tor_malloc(fname_len), *contents = NULL;
 
   if (!routerlist)
     router_get_routerlist(); /* mallocs and inits it in place */
@@ -374,7 +374,8 @@ router_reload_router_list(void)
 
   tor_snprintf(fname, fname_len, "%s/cached-routers.new",
                options->DataDirectory);
-  contents = read_file_to_str(fname, 1, NULL);
+  if (file_status(fname) == FN_FILE)
+    contents = read_file_to_str(fname, RFTS_BIN|RFTS_IGNORE_MISSING, NULL);
   if (contents) {
     router_load_routers_from_string(contents,
                                     SAVED_IN_JOURNAL, NULL);