Tinker with log behavior: never send error messages about logs into the bitbucket

svn:r1912

5 files changed, 68 insertions, 24 deletions

diff --git a/src/common/log.c b/src/common/log.c
index 0581bd792..1b77cc49a 100644
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -25,11 +25,12 @@
 /** Information for a single logfile; only used in log.c */
 typedef struct logfile_t {
   struct logfile_t *next; /**< Next logfile_t in the linked list. */
-  const char *filename; /**< Filename to open. */
+  char *filename; /**< Filename to open. */
   FILE *file; /**< Stream to receive log messages. */
   int needs_close; /**< Boolean: true if the stream gets closed on shutdown. */
   int loglevel; /**< Lowest severity level to send to this stream. */
   int max_loglevel; /**< Highest severity level to send to this stream. */
+  int is_temporary; /**< Boolean: close after initializing logging subsystem.*/
 } logfile_t;
 
 /** Helper: map a log severity to descriptive string. */
@@ -145,7 +146,8 @@ void close_logs()
     logfiles = logfiles->next;
     if (victim->needs_close)
       fclose(victim->file);
-    free(victim);
+    tor_free(victim->filename);
+    tor_free(victim);
   }
 }
 
@@ -167,15 +169,42 @@ void add_stream_log(int loglevelMin, int loglevelMax, const char *name, FILE *st
 {
   logfile_t *lf;
   lf = tor_malloc(sizeof(logfile_t));
-  lf->filename = name;
+  lf->filename = tor_strdup(name);
   lf->needs_close = 0;
   lf->loglevel = loglevelMin;
   lf->max_loglevel = loglevelMax;
   lf->file = stream;
   lf->next = logfiles;
+  lf->is_temporary = 0;
   logfiles = lf;
 }
 
+/** Add a log handler to receive messages during startup (before the real
+ * logs are initialized).
+ */
+void add_temp_log(void)
+{
+  add_stream_log(LOG_INFO, LOG_ERR, "<temp>", stdout);
+  logfiles->is_temporary = 1;
+}
+
+void close_temp_logs(void)
+{
+  logfile_t *lf, **p;
+  for (p = &logfiles; *p; ) {
+    if ((*p)->is_temporary) {
+      lf = *p;
+      *p = (*p)->next;
+      if (lf->needs_close)
+        fclose(lf->file);
+      tor_free(lf->filename);
+      tor_free(lf);
+    } else {
+      p = &((*p)->next);
+    }
+  }
+}
+
 /**
  * Add a log handler to send messages to <b>filename</b>. If opening
  * the logfile fails, -1 is returned and errno is set appropriately
diff --git a/src/common/log.h b/src/common/log.h
index 786e7fcc1..c8b631b58 100644
--- a/src/common/log.h
+++ b/src/common/log.h
@@ -52,6 +52,8 @@ int add_file_log(int severityMin, int severityMax, const char *filename);
 int get_min_log_level(void);
 void close_logs();
 void reset_logs();
+void add_temp_log(void);
+void close_temp_logs(void);
 
 /* Outputs a message to stdout */
 void _log(int severity, const char *format, ...) CHECK_PRINTF(2,3);
diff --git a/src/or/config.c b/src/or/config.c
index f089aa92c..c3dc7c58a 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -716,7 +716,7 @@ int getconfig(int argc, char **argv, or_options_t *options) {
   return result;
 }
 
-static void add_single_log(struct config_line_t *level_opt,
+static int add_single_log(struct config_line_t *level_opt,
                            struct config_line_t *file_opt,
                            int isDaemon)
 {
@@ -729,17 +729,21 @@ static void add_single_log(struct config_line_t *level_opt,
       tmp_sev = tor_strndup(level_opt->value, cp - level_opt->value);
       levelMin = parse_log_level(tmp_sev);
       if (levelMin<0) {
-        log_fn(LOG_WARN, "Unrecognized log severity %s: must be one of err|warn|notice|info|debug", tmp_sev);
+        log_fn(LOG_WARN, "Unrecognized log severity '%s': must be one of err|warn|notice|info|debug", tmp_sev);
+        return -1;
       }
       tor_free(tmp_sev);
       levelMax = parse_log_level(cp+1);
       if (levelMax<0) {
-        log_fn(LOG_WARN, "Unrecognized log severity %s: must be one of err|warn|notice|info|debug", cp+1);
+        log_fn(LOG_WARN, "Unrecognized log severity '%s': must be one of err|warn|notice|info|debug", cp+1);
+        return -1;
       }
     } else {
       levelMin = parse_log_level(level_opt->value);
       if (levelMin<0) {
-        log_fn(LOG_WARN, "Unrecognized log severity %s: must be one of err|warn|notice|info|debug", level_opt->value);
+        log_fn(LOG_WARN, "Unrecognized log severity '%s': must be one of err|warn|notice|info|debug", level_opt->value);
+        return -1;
+
       }
     }
   }
@@ -753,44 +757,47 @@ static void add_single_log(struct config_line_t *level_opt,
   }
   if (file_opt) {
     if (add_file_log(levelMin, levelMax, file_opt->value) < 0) {
-      /* opening the log file failed!  Use stderr and log a warning */
-      add_stream_log(levelMin, levelMax, "<stderr>", stderr);
       log_fn(LOG_WARN, "Cannot write to LogFile '%s': %s.", file_opt->value,
              strerror(errno));
-      return;
+      return -1;
     }
     log_fn(LOG_NOTICE, "Successfully opened LogFile '%s', redirecting output.",
            file_opt->value);
   } else if (!isDaemon) {
     add_stream_log(levelMin, levelMax, "<stdout>", stdout);
+    close_temp_logs();
   }
+  return 0;
 }
 
 /**
  * Initialize the logs based on the configuration file.
  */
-void config_init_logs(or_options_t *options)
+int config_init_logs(or_options_t *options)
 {
   /* The order of options is:  Level? (File Level?)+
    */
   struct config_line_t *opt = options->LogOptions;
 
-  /* Special case if nothing is specified. */
-  if(!opt) {
-    add_single_log(NULL, NULL, options->RunAsDaemon);
+  /* Special case if no options are given. */
+  if (!opt) {
+    add_stream_log(LOG_NOTICE, LOG_ERR, "<stdout>", stdout);
+    close_temp_logs();
     /* don't return yet, in case we want to do a debuglogfile below */
   }
 
   /* Special case for if first option is LogLevel. */
   if (opt && !strcasecmp(opt->key, "LogLevel")) {
     if (opt->next && !strcasecmp(opt->next->key, "LogFile")) {
-      add_single_log(opt, opt->next, options->RunAsDaemon);
+      if (add_single_log(opt, opt->next, options->RunAsDaemon)<0)
+        return -1;
       opt = opt->next->next;
     } else if (!opt->next) {
-      add_single_log(opt, NULL, options->RunAsDaemon);
-      return;
-    } else {
+      if (add_single_log(opt, NULL, options->RunAsDaemon)<0)
+        return -1;
       opt = opt->next;
+    } else {
+      ; /* give warning below */
     }
   }
 
@@ -802,11 +809,13 @@ void config_init_logs(or_options_t *options)
       tor_assert(!strcasecmp(opt->key, "LogFile"));
       if (opt->next && !strcasecmp(opt->next->key, "LogLevel")) {
         /* LogFile followed by LogLevel */
-        add_single_log(opt->next, opt, options->RunAsDaemon);
+        if (add_single_log(opt->next, opt, options->RunAsDaemon)<0)
+          return -1;
         opt = opt->next->next;
       } else {
         /* LogFile followed by LogFile or end of list. */
-        add_single_log(NULL, opt, options->RunAsDaemon);
+        if (add_single_log(NULL, opt, options->RunAsDaemon)<0)
+          return -1;
         opt = opt->next;
       }
     }
@@ -814,8 +823,10 @@ void config_init_logs(or_options_t *options)
 
   if (options->DebugLogFile) {
     log_fn(LOG_WARN, "DebugLogFile is deprecated; use LogFile and LogLevel instead");
-    add_file_log(LOG_DEBUG, LOG_ERR, options->DebugLogFile);
+    if (add_file_log(LOG_DEBUG, LOG_ERR, options->DebugLogFile)<0)
+      return -1;
   }
+  return 0;
 }
 
 void
diff --git a/src/or/main.c b/src/or/main.c
index 50280f074..eb7a1a05f 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -567,7 +567,9 @@ static int init_from_config(int argc, char **argv) {
   close_logs(); /* we'll close, then open with correct loglevel if necessary */
 
   /* Configure the log(s) */
-  config_init_logs(&options);
+  if (config_init_logs(&options)<0)
+    return -1;
+  close_temp_logs();
 
   /* Set up our buckets */
   connection_bucket_init();
@@ -853,7 +855,7 @@ void exit_function(void)
 int tor_main(int argc, char *argv[]) {
 
   /* give it somewhere to log to initially */
-  add_stream_log(LOG_NOTICE, LOG_ERR, "<stdout>", stdout);
+  add_temp_log();
   log_fn(LOG_NOTICE,"Tor v%s. This is experimental software. Do not use it if you need anonymity.",VERSION);
 
   if (network_init()<0) {
diff --git a/src/or/or.h b/src/or/or.h
index d0f0c59e9..46238f2cf 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -958,7 +958,7 @@ struct config_line_t {
 
 int config_assign_default_dirservers(void);
 int getconfig(int argc, char **argv, or_options_t *options);
-void config_init_logs(or_options_t *options);
+int config_init_logs(or_options_t *options);
 void config_parse_exit_policy(struct config_line_t *cfg,
                               struct exit_policy_t **dest);
 void exit_policy_free(struct exit_policy_t *p);