more cleanup and rearranging

still not finished integrating new dirserv stuff


svn:r507

src/common/util.c

@@ -430,3 +430,53 @@ char *read_file_to_str(const char *filename) {
   return string;
 }
 
+/* read lines from f (no more than maxlen-1 bytes each) until we
+ * get one with a well-formed "key value".
+ * point *key to the first word in line, point *value to the second.
+ * Put a \0 at the end of key, remove everything at the end of value
+ * that is whitespace or comment.
+ * Return 1 if success, 0 if no more lines, -1 if error.
+ */
+int parse_line_from_file(char *line, int maxlen, FILE *f, char **key_out, char **value_out) {
+  char *s, *key, *end, *value;
+
+try_next_line:
+  if(!fgets(line, maxlen, f)) {
+    if(feof(f))
+      return 0;
+    return -1; /* real error */
+  }
+
+  if((s = strchr(line,'#'))) /* strip comments */
+    *s = 0; /* stop the line there */
+
+  /* remove end whitespace */
+  s = strchr(line, 0); /* now we're at the null */
+  do {
+    *s = 0;
+    s--;
+  } while (isspace(*s));
+
+  key = line;
+  while(isspace(*key))
+    key++;
+  if(*key == 0)
+    goto try_next_line; /* this line has nothing on it */
+  end = key;
+  while(*end && !isspace(*end))
+    end++;
+  value = end;
+  while(*value && isspace(*value))
+    value++;
+
+  if(!*end || !*value) { /* only a key on this line. no value. */
+    log_fn(LOG_WARNING,"Line has keyword '%s' but no value. Skipping.",s);
+    goto try_next_line;
+  }
+  *end = 0; /* null it out */
+
+  log_fn(LOG_DEBUG,"got keyword '%s', value '%s'", key, value);
+  *key_out = key, *value_out = value;
+  return 1;
+}
+

src/common/util.h

@@ -5,21 +5,14 @@
 #ifndef __UTIL_H
 #define __UTIL_H
 
-#include "orconfig.h"
+#include "../or/or.h"
 
-#ifdef HAVE_SYS_TIME_H
-#include <sys/time.h>
-#endif
-#ifdef HAVE_TIME_H
-#include <time.h>
-#endif
 #if _MSC_VER > 1300
 #include <winsock2.h>
 #include <ws2tcpip.h>
 #elif defined(_MSC_VER)
 #include <winsock.h>
 #endif
-#include <errno.h>
 #ifndef HAVE_GETTIMEOFDAY
 #ifdef HAVE_FTIME
 #define USING_FAKE_TIMEVAL
@@ -68,6 +61,7 @@ file_status_t file_status(const char *filename);
 int check_private_dir(const char *dirname, int create);
 int write_str_to_file(const char *fname, const char *str);
 char *read_file_to_str(const char *filename);
+int parse_line_from_file(char *line, int maxlen, FILE *f, char **key_out, char **value_out);
 
 /* Minimalist interface to run a void function in the background.  On
    unix calls fork, on win32 calls beginthread.  Returns -1 on failure.

src/or/config.c

@@ -73,64 +73,26 @@ static struct config_line *config_get_commandlines(int argc, char **argv) {
   return front;
 }
 
-/* parse the config file and strdup into key/value strings. Return list.
+/* parse the config file and strdup into key/value strings. Return list,
+ * or NULL if parsing the file failed.
  * Warn and ignore mangled lines. */
 static struct config_line *config_get_lines(FILE *f) {
   struct config_line *new;
   struct config_line *front = NULL;
   char line[CONFIG_LINE_MAXLEN];
-  int lineno=0; /* current line number */
-  char *s;
-  char *start, *end;
+  int result;
+  char *key, *value;
 
-  assert(f);
-
-  fseek(f,0,SEEK_SET); /* make sure we start at the beginning of file */
-
-  while(fgets(line, CONFIG_LINE_MAXLEN, f)) {
-    lineno++;
-
-    /* first strip comments */
-    s = strchr(line,'#');
-    if(s) {
-      *s = 0; /* stop the line there */
-    }
-
-    /* walk to the end, remove end whitespace */
-    s = strchr(line, 0); /* now we're at the null */
-    do {
-      *s = 0;
-      s--;
-    } while (isspace(*s));
-
-    start = line;
-    while(isspace(*start))
-      start++;
-    if(*start == 0)
-      continue; /* this line has nothing on it */
-
-    end = start;
-    while(*end && !isspace(*end))
-      end++;
-    s = end;
-    while(*s && isspace(*s))
-      s++;
-    if(!*end || !*s) { /* only a keyword on this line. no value. */
-      log(LOG_WARNING,"Config line %d has keyword '%s' but no value. Skipping.",lineno,s);
-    }
-    *end = 0; /* null it out */
-
-    /* prepare to parse the string into key / value */
+  while( (result=parse_line_from_file(line,sizeof(line),f,&key,&value)) > 0) {
     new = tor_malloc(sizeof(struct config_line));
-    new->key = strdup(start);
-    new->value = strdup(s);
+    new->key = strdup(key);
+    new->value = strdup(value);
 
-    log(LOG_DEBUG,"Config line %d: parsed keyword '%s', value '%s'",
-      lineno, new->key, new->value);
     new->next = front;
     front = new;
   }
-
+  if(result < 0)
+    return NULL;
   return front;
 }
 
@@ -261,11 +223,12 @@ int getconfig(int argc, char **argv, or_options_t *options) {
 
   cf = config_open(fname);
   if(!cf) {
-    log(LOG_ERR, "Unable to open configuration file '%s'.",fname);
+    log(LOG_WARNING, "Unable to open configuration file '%s'.",fname);
     return -1;
   }
 
   cl = config_get_lines(cf);
+  if(!cl) return -1;
   config_assign(options,cl);
   config_free_lines(cl);
   config_close(cf);
@@ -287,69 +250,69 @@ int getconfig(int argc, char **argv, or_options_t *options) {
     else if(!strcmp(options->LogLevel,"debug"))
       options->loglevel = LOG_DEBUG;
     else {
-      log(LOG_ERR,"LogLevel must be one of err|warning|info|debug.");
+      log(LOG_WARNING,"LogLevel must be one of err|warning|info|debug.");
       result = -1;
     }
   }
 
   if(options->RouterFile == NULL) {
-    log(LOG_ERR,"RouterFile option required, but not found.");
+    log(LOG_WARNING,"RouterFile option required, but not found.");
     result = -1;
   }
 
   if(options->ORPort < 0) {
-    log(LOG_ERR,"ORPort option can't be negative.");
+    log(LOG_WARNING,"ORPort option can't be negative.");
     result = -1;
   }
 
   if(options->OnionRouter && options->ORPort == 0) {
-    log(LOG_ERR,"If OnionRouter is set, then ORPort must be positive.");
+    log(LOG_WARNING,"If OnionRouter is set, then ORPort must be positive.");
     result = -1;
   }
 
   if(options->OnionRouter && options->DataDirectory == NULL) {
-    log(LOG_ERR,"DataDirectory option required for OnionRouter, but not found.");
+    log(LOG_WARNING,"DataDirectory option required for OnionRouter, but not found.");
     result = -1;
   }
 
   if(options->OnionRouter && options->Nickname == NULL) {
-    log_fn(LOG_ERR,"Nickname required for OnionRouter, but not found.");
+    log_fn(LOG_WARNING,"Nickname required for OnionRouter, but not found.");
     result = -1;
   }
 
   if(options->APPort < 0) {
-    log(LOG_ERR,"APPort option can't be negative.");
+    log(LOG_WARNING,"APPort option can't be negative.");
     result = -1;
   }
 
   if(options->DirPort < 0) {
-    log(LOG_ERR,"DirPort option can't be negative.");
+    log(LOG_WARNING,"DirPort option can't be negative.");
     result = -1;
   }
 
   if(options->APPort > 1 &&
      (options->CoinWeight < 0.0 || options->CoinWeight >= 1.0)) {
-    log(LOG_ERR,"CoinWeight option must be >=0.0 and <1.0.");
+    log(LOG_WARNING,"CoinWeight option must be >=0.0 and <1.0.");
     result = -1;
   }
 
   if(options->MaxConn < 1) {
-    log(LOG_ERR,"MaxConn option must be a non-zero positive integer.");
+    log(LOG_WARNING,"MaxConn option must be a non-zero positive integer.");
     result = -1;
   }
 
   if(options->MaxConn >= MAXCONNECTIONS) {
-    log(LOG_ERR,"MaxConn option must be less than %d.", MAXCONNECTIONS);
+    log(LOG_WARNING,"MaxConn option must be less than %d.", MAXCONNECTIONS);
     result = -1;
   }
 
   if(options->DirFetchPeriod < 1) {
-    log(LOG_ERR,"DirFetchPeriod option must be positive.");
+    log(LOG_WARNING,"DirFetchPeriod option must be positive.");
     result = -1;
   }
 
   if(options->KeepalivePeriod < 1) {
-    log(LOG_ERR,"KeepalivePeriod option must be positive.");
+    log(LOG_WARNING,"KeepalivePeriod option must be positive.");
     result = -1;
   }
 

src/or/directory.c

@@ -191,7 +191,7 @@ static int directory_handle_command(connection_t *conn) {
 
     if(dlen == 0) {
       log_fn(LOG_WARNING,"My directory is empty. Closing.");
-      return -1;
+      return -1; /* XXX send some helpful http error code */
     }
 
     log_fn(LOG_DEBUG,"Dumping directory to client."); 
@@ -207,6 +207,11 @@ static int directory_handle_command(connection_t *conn) {
   if(!strncasecmp(headers,"POST",4)) {
     /* XXX should check url and http version */
     log_fn(LOG_DEBUG,"Received POST command, body '%s'", body);
+    cp = body;
+    if(dirserv_add_descriptor(&cp) < 0) {
+      log_fn(LOG_WARNING,"dirserv_add_descriptor() failed. Dropping.");
+      return -1; /* XXX should write an http failed code */
+    }
     if(connection_write_to_buf(answerstring, strlen(answerstring), conn) < 0) {
       log_fn(LOG_WARNING,"Failed to write answerstring to outbuf.");
       return -1;

src/or/dirserv.c

@@ -8,9 +8,8 @@ static int the_directory_is_dirty = 1;
 static char *the_directory = NULL;
 static int the_directory_len = -1;
 
-/*
- *    Fingerprint handling code.
- */
+/************** Fingerprint handling code ************/
+
 typedef struct fingerprint_entry_t {
   char *nickname;
   char *fingerprint;
@@ -24,83 +23,54 @@ int
 dirserv_parse_fingerprint_file(const char *fname)
 {
   FILE *file;
-#define BUF_LEN (FINGERPRINT_LEN+MAX_NICKNAME_LEN+20)
-  char buf[BUF_LEN+1];
-  char *cp, *nickname, *fingerprint;
+  char line[FINGERPRINT_LEN+MAX_NICKNAME_LEN+20+1];
+  char *nickname, *fingerprint;
   fingerprint_entry_t fingerprint_list_tmp[MAX_ROUTERS_IN_DIR];
   int n_fingerprints_tmp = 0;
-  int lineno=0;
-  int i;
-  if (!(file = fopen(fname, "r"))) {
-    log(LOG_WARNING, "Cannot open fingerprint file %s", fname);
-    goto err;
+  int i, result;
+
+  if(!(file = fopen(fname, "r"))) {
+    log_fn(LOG_WARNING, "Cannot open fingerprint file %s", fname);
+    return -1;
   }
-  while (1) {
-    cp = fgets(buf, BUF_LEN, file);
-    ++lineno;
-    if (!cp) {
-      if (feof(file))
-        break;
-      else {
-        log(LOG_WARNING, "Error reading from fingerprint file");
-        goto err;
-      }
-    }
-    buf[BUF_LEN]='\0';
-    cp = buf;
-    while (isspace(*cp))
-      ++cp;
-    if (*cp == '#' || *cp == '\0') 
-      continue;
-    nickname = cp;
-    cp = strchr(cp, ' ');
-    if (!cp) {
-      log(LOG_WARNING, "Bad line %d of fingerprint file", lineno);
-      goto err;
-    }
-    *cp++ = '\0';
-    while (isspace(*cp))
-      ++cp;
-    if (strlen(cp) < FINGERPRINT_LEN) {
-      log(LOG_WARNING, "Bad line %d of fingerprint file", lineno);
-      goto err;
-    }
-    fingerprint = cp;
-    cp[FINGERPRINT_LEN] = '\0';
+  while( (result=parse_line_from_file(line, sizeof(line),file,&nickname,&fingerprint)) > 0) {
     if (strlen(nickname) > MAX_NICKNAME_LEN) {
-      log(LOG_WARNING, "Nickname too long on line %d of fingerprint file",
-          lineno);
-      goto err;
+      log(LOG_WARNING, "Nickname %s too long in fingerprint file. Skipping.", nickname);
+      continue;
     }
-    if (!crypto_pk_check_fingerprint_syntax(fingerprint)) {
-      log(LOG_WARNING, "Invalid fingerprint on line %d of fingerprint file",
-          lineno);
-      goto err;
+    if(strlen(fingerprint) != FINGERPRINT_LEN ||
+       !crypto_pk_check_fingerprint_syntax(fingerprint)) {
+      log_fn(LOG_WARNING, "Invalid fingerprint (nickname %s, fingerprint %s). Skipping.",
+             nickname, fingerprint);
+      continue;
     }
     for (i = 0; i < n_fingerprints_tmp; ++i) {
       if (0==strcasecmp(fingerprint_list_tmp[i].nickname, nickname)) {
-        log(LOG_WARNING, "Duplicate nickname on line %d of fingerprint file", lineno);
-        goto err;
+        log(LOG_WARNING, "Duplicate nickname %s. Skipping.",nickname);
+        break; /* out of the for. the 'if' below means skip to the next line. */
       }
     }
-    fingerprint_list_tmp[n_fingerprints_tmp].nickname = strdup(nickname);
-    fingerprint_list_tmp[n_fingerprints_tmp].fingerprint = strdup(fingerprint);
-    ++n_fingerprints_tmp;
+    if(i == n_fingerprints_tmp) { /* not a duplicate */
+      fingerprint_list_tmp[n_fingerprints_tmp].nickname = strdup(nickname);
+      fingerprint_list_tmp[n_fingerprints_tmp].fingerprint = strdup(fingerprint);
+      ++n_fingerprints_tmp;
+    }
   }
-  /* replace the global fingerprints list. */
-  dirserv_free_fingerprint_list();
-  memcpy(fingerprint_list, fingerprint_list_tmp, 
-         sizeof(fingerprint_entry_t)*n_fingerprints_tmp);
-  n_fingerprints = n_fingerprints_tmp;
-  return 0; 
-
- err:
+  fclose(file);
+  if(result == 0) { /* eof; replace the global fingerprints list. */
+    dirserv_free_fingerprint_list();
+    memcpy(fingerprint_list, fingerprint_list_tmp,
+           sizeof(fingerprint_entry_t)*n_fingerprints_tmp);
+    n_fingerprints = n_fingerprints_tmp;
+    return 0;
+  }
+  /* error */
+  log_fn(LOG_WARNING, "Error reading from fingerprint file");
   for (i = 0; i < n_fingerprints_tmp; ++i) {
     free(fingerprint_list_tmp[i].nickname);
     free(fingerprint_list_tmp[i].fingerprint);
   }
   return -1;
-#undef BUF_LEN
 }    
 
 /* return 1 if router's identity and nickname match. */
@@ -118,20 +88,17 @@ dirserv_router_fingerprint_is_known(const routerinfo_t *router)
     }
   }
   
-  if (!ent) {
-    /* No such server known */
+  if (!ent) { /* No such server known */
     return 0;
   }
   if (crypto_pk_get_fingerprint(router->identity_pkey, fp)) {
-    /* XXX Error computing fingerprint: log */
+    log_fn(LOG_WARNING,"error computing fingerprint");
     return 0;
   }
   if (0==strcasecmp(ent->fingerprint, fp)) {
-    /* Right fingerprint. */
-    return 1;
+    return 1; /* Right fingerprint. */
   } else {
-    /* Wrong fingerprint. */
-    return 0;
+    return 0; /* Wrong fingerprint. */
   }
 }
 

src/or/main.c

@@ -912,10 +912,11 @@ void daemonize(void) {
 }
 
 int tor_main(int argc, char *argv[]) {
-  int retval = 0;
 
-  if(getconfig(argc,argv,&options))
-    exit(1);
+  if(getconfig(argc,argv,&options)) {
+    log_fn(LOG_ERR,"Reading config file failed. exiting.");
+    return -1;
+  }
   log_set_severity(options.loglevel);     /* assign logging severity level from options */
   global_read_bucket = options.TotalBandwidth; /* start it at 1 second of traffic */
 
@@ -936,10 +937,9 @@ int tor_main(int argc, char *argv[]) {
 
   crypto_global_init();
   crypto_seed_rng();
-  retval = do_main_loop();
+  do_main_loop();
   crypto_global_cleanup();
-  
-  return retval;
+  return -1;
 }
 
 /*