laying the groundwork for dynamic router lists

revamped the router reading section

reference counting for crypto pk env's (so we can dup them)

we now read and write pem pk keys from string rather than from FILE*,
  in anticipation of fetching directories over a socket
  (so now on startup we slurp in the whole file, then parse it as a string)

fixed a bug in the proxy side, where you could get some circuits
  wedged if they showed up while the connection was being made


svn:r110

8 files changed, 286 insertions, 342 deletions

diff --git a/src/or/circuit.c b/src/or/circuit.c
index ceb841eb8..bc5bd108c 100644
--- a/src/or/circuit.c
+++ b/src/or/circuit.c
@@ -241,10 +241,14 @@ int circuit_init(circuit_t *circ, int aci_type) {
   return 0;
 }
 
-circuit_t *circuit_get_by_naddr_nport(uint32_t naddr, uint16_t nport) {
-  circuit_t *circ;
+circuit_t *circuit_enumerate_by_naddr_nport(circuit_t *circ, uint32_t naddr, uint16_t nport) {
 
-  for(circ=global_circuitlist;circ;circ = circ->next) {
+  if(!circ) /* use circ if it's defined, else start from the beginning */
+    circ = global_circuitlist; 
+  else
+    circ = circ->next;
+
+  for( ;circ;circ = circ->next) {
     if(circ->n_addr == naddr && circ->n_port == nport)
        return circ;
   }
diff --git a/src/or/connection.c b/src/or/connection.c
index d8f7add2f..4eeeae020 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -125,6 +125,11 @@ void connection_free(connection_t *conn) {
       crypto_free_cipher_env(conn->b_crypto);
   }
 
+  if (conn->pkey)
+    crypto_free_pk_env(conn->pkey);
+  if (conn->prkey)
+    crypto_free_pk_env(conn->prkey);
+
   if(conn->s > 0) {
     log(LOG_INFO,"connection_free(): closing fd %d.",conn->s);
     close(conn->s);
@@ -175,7 +180,8 @@ int connection_create_listener(crypto_pk_env_t *prkey, struct sockaddr_in *local
 
   /* remember things so you can tell the baby sockets */
   memcpy(&conn->local,local,sizeof(struct sockaddr_in));
-  conn->prkey = prkey;
+  if(prkey)
+    conn->prkey = crypto_pk_dup_key(prkey);
 
   log(LOG_DEBUG,"connection_create_listener(): Listening on local port %u.",ntohs(local->sin_port));
 
@@ -214,7 +220,8 @@ int connection_handle_listener_read(connection_t *conn, int new_type, int new_st
 
   /* learn things from parent, so we can perform auth */
   memcpy(&newconn->local,&conn->local,sizeof(struct sockaddr_in));
-  newconn->prkey = conn->prkey;
+  if(conn->prkey)
+    newconn->prkey = crypto_pk_dup_key(conn->prkey);
   newconn->address = strdup(inet_ntoa(remote.sin_addr)); /* remember the remote address */
 
   if(connection_add(newconn) < 0) { /* no space, forget it */
diff --git a/src/or/connection_ap.c b/src/or/connection_ap.c
index 838fe737e..a680f5025 100644
--- a/src/or/connection_ap.c
+++ b/src/or/connection_ap.c
@@ -191,7 +191,7 @@ int ap_handshake_establish_circuit(connection_t *conn, unsigned int *route, int
   log(LOG_DEBUG,"ap_handshake_establish_circuit(): Looking for firsthop '%s:%u'",
       firsthop->address,firsthop->or_port);
   n_conn = connection_twin_get_by_addr_port(firsthop->addr,firsthop->or_port);
-  if(!n_conn) { /* not currently connected */
+  if(!n_conn || n_conn->state != OR_CONN_STATE_OPEN) { /* not currently connected */
     circ->n_addr = firsthop->addr;
     circ->n_port = firsthop->or_port;
     if(global_role & ROLE_OR_CONNECT_ALL) { /* we would be connected if he were up. but he's not. */
@@ -199,14 +199,15 @@ int ap_handshake_establish_circuit(connection_t *conn, unsigned int *route, int
       circuit_close(circ); 
       return -1;
     }
-    
-    /* ok, launch the connection */
-    n_conn = connect_to_router_as_op(firsthop);
-    if(!n_conn) { /* connect failed, forget the whole thing */
-      log(LOG_DEBUG,"ap_handshake_establish_circuit(): connect to firsthop failed. Closing.");
-      circuit_close(circ);
-      return -1;
-    }   
+
+    if(!n_conn) { /* launch the connection */
+      n_conn = connect_to_router_as_op(firsthop);
+      if(!n_conn) { /* connect failed, forget the whole thing */
+        log(LOG_DEBUG,"ap_handshake_establish_circuit(): connect to firsthop failed. Closing.");
+        circuit_close(circ);
+        return -1;
+      }   
+    }
     conn->state = AP_CONN_STATE_OR_WAIT;
     connection_stop_reading(conn); /* Stop listening for input from the AP! */
     return 0; /* return success. The onion/circuit/etc will be taken care of automatically
@@ -219,21 +220,31 @@ int ap_handshake_establish_circuit(connection_t *conn, unsigned int *route, int
   }
 }
 
-/* find the circ that's waiting on me, if any, and get it to send its onion */
-int ap_handshake_n_conn_open(connection_t *or_conn) {
+/* find circuits that are waiting on me, if any, and get them to send the onion */
+void ap_handshake_n_conn_open(connection_t *or_conn) {
   circuit_t *circ;
+  connection_t *p_conn;
 
   log(LOG_DEBUG,"ap_handshake_n_conn_open(): Starting.");
-  circ = circuit_get_by_naddr_nport(or_conn->addr, or_conn->port);
-  if(!circ)
-    return 0; /* i'm ok with that. no need to close the connection or anything. */
-
-  if(circ->p_conn->state != AP_CONN_STATE_OR_WAIT) {
-    log(LOG_DEBUG,"Bug: ap_handshake_n_conn_open() got an ap_conn not in OR_WAIT state.");
+  circ = circuit_enumerate_by_naddr_nport(NULL, or_conn->addr, or_conn->port);
+  for(;;) {
+    if(!circ)
+      return;
+
+    p_conn = circ->p_conn;
+    if(p_conn->state != AP_CONN_STATE_OR_WAIT) {
+      log(LOG_WARNING,"Bug: ap_handshake_n_conn_open() got an ap_conn not in OR_WAIT state.");
+    }
+    connection_start_reading(p_conn); /* resume listening for reads */
+    log(LOG_DEBUG,"ap_handshake_n_conn_open(): Found circ, sending onion.");
+    if(ap_handshake_send_onion(p_conn, or_conn, circ)<0) {
+      log(LOG_DEBUG,"ap_handshake_n_conn_open(): circuit marked for closing.");
+      p_conn->marked_for_close = 1;
+      return; /* XXX will want to try the rest too */
+    } else {
+      circ = circuit_enumerate_by_naddr_nport(circ, or_conn->addr, or_conn->port);
+    }
   }
-  connection_start_reading(circ->p_conn); /* resume listening for reads */
-  log(LOG_DEBUG,"ap_handshake_n_conn_open(): Found circ, sending onion.");
-  return ap_handshake_send_onion(circ->p_conn, or_conn, circ);
 }
 
 int ap_handshake_send_onion(connection_t *ap_conn, connection_t *n_conn, circuit_t *circ) {
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index c16296a61..53bf38837 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -154,9 +154,10 @@ connection_t *connection_or_connect(routerinfo_t *router, crypto_pk_env_t *prkey
 
   /* set up conn so it's got all the data we need to remember */
   conn->addr = router->addr, conn->port = router->or_port; /* NOTE we store or_port here always */
-  conn->prkey = prkey;
-  conn->bandwidth = router->min; /* kludge, should make a router->bandwidth and use that */
-  conn->pkey = router->pkey;
+  if(prkey)
+    conn->prkey = crypto_pk_dup_key(prkey);
+  conn->bandwidth = router->bandwidth;
+  conn->pkey = crypto_pk_dup_key(router->pkey);
   conn->address = strdup(router->address);
   memcpy(&conn->local,local,sizeof(struct sockaddr_in));
 
@@ -331,8 +332,8 @@ int or_handshake_op_finished_sending_keys(connection_t *conn) {
   conn->state = OR_CONN_STATE_OPEN;
   connection_init_timeval(conn);
   connection_watch_events(conn, POLLIN); /* give it a default, tho the ap_handshake call may change it */
-  return ap_handshake_n_conn_open(conn); /* send the pending onion */
-
+  ap_handshake_n_conn_open(conn); /* send the pending onions */
+  return 0;
 }
 
 /*
@@ -601,14 +602,14 @@ int or_handshake_server_process_auth(connection_t *conn) {
   /* update link info */
   bandwidth = ntohl(*(uint32_t *)(buf+28));
 
-  conn->bandwidth = router->min; /* FIXME, should make a router->bandwidth and use that */
+  conn->bandwidth = router->bandwidth;
 
   if (conn->bandwidth > bandwidth)
     conn->bandwidth = bandwidth;
 
   /* copy all relevant info to conn */
   conn->addr = router->addr, conn->port = router->or_port;
-  conn->pkey = router->pkey;
+  conn->pkey = crypto_pk_dup_key(router->pkey);
   conn->address = strdup(router->address);
 
   /* generate a nonce */
diff --git a/src/or/main.c b/src/or/main.c
index ab3238a02..ccf98f655 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -87,12 +87,6 @@ int connection_remove(connection_t *conn) {
   return 0;  
 }
 
-int pkey_cmp(crypto_pk_env_t *a, crypto_pk_env_t *b) {
-  /* return 0 if a and b are "the same key". Return non-0 otherwise. */
-
-  return crypto_pk_cmp_keys(a, b);
-}
-
 connection_t *connection_twin_get_by_addr_port(uint32_t addr, uint16_t port) {
   /* Find a connection to the router described by addr and port,
    *   or alternately any router which knows its key.
@@ -119,7 +113,7 @@ connection_t *connection_twin_get_by_addr_port(uint32_t addr, uint16_t port) {
   for(i=0;i<nfds;i++) {
     conn = connection_array[i];
     assert(conn);
-    if(connection_state_is_open(conn) && !pkey_cmp(conn->pkey, router->pkey)) {
+    if(connection_state_is_open(conn) && !crypto_pk_cmp_keys(conn->pkey, router->pkey)) {
       log(LOG_INFO,"connection_twin_get_by_addr_port(): Found twin (%s).",conn->address);
       return conn;
     }
@@ -412,7 +406,7 @@ int do_main_loop(void) {
   int poll_result;
 
   /* load the routers file */
-  router_array = getrouters(options.RouterFile,&rarray_len, options.ORPort);
+  router_array = router_get_list_from_file(options.RouterFile,&rarray_len, options.ORPort);
   if (!router_array)
   {
     log(LOG_ERR,"Error loading router list.");
@@ -426,7 +420,7 @@ int do_main_loop(void) {
       log(LOG_ERR,"Error creating a crypto environment.");
       return -1;
     }
-    if (crypto_pk_read_private_key_filename(prkey, options.PrivateKeyFile))
+    if (crypto_pk_read_private_key_from_filename(prkey, options.PrivateKeyFile))
     {
       log(LOG_ERR,"Error loading private key.");
       return -1;
diff --git a/src/or/onion.c b/src/or/onion.c
index 52b785bb0..f1b635759 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -118,7 +118,7 @@ unsigned int *new_route(double cw, routerinfo_t **rarray, int rarray_len, int *r
       goto next_i_loop;
     }
     for(j=0;j<i;j++) {
-      if(!pkey_cmp(rarray[i]->pkey, rarray[j]->pkey)) {
+      if(!crypto_pk_cmp_keys(rarray[i]->pkey, rarray[j]->pkey)) {
         /* these guys are twins. so we've already counted him. */
         log(LOG_DEBUG,"Nope, %d is a twin of %d.",i,j);
         goto next_i_loop;
@@ -158,7 +158,7 @@ unsigned int *new_route(double cw, routerinfo_t **rarray, int rarray_len, int *r
     choice = choice % (rarray_len);
     log(LOG_DEBUG,"new_route(): Contemplating router %u.",choice);
     if(choice == oldchoice ||
-      (oldchoice < rarray_len && !pkey_cmp(rarray[choice]->pkey, rarray[oldchoice]->pkey)) ||
+      (oldchoice < rarray_len && !crypto_pk_cmp_keys(rarray[choice]->pkey, rarray[oldchoice]->pkey)) ||
       ((global_role & ROLE_OR_CONNECT_ALL) && !connection_twin_get_by_addr_port(rarray[choice]->addr, rarray[choice]->or_port))) {
       /* Same router as last choice, or router twin,
        *   or no routers with that key are connected to us.
diff --git a/src/or/or.h b/src/or/or.h
index d6c051b3c..dc39956c1 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -30,6 +30,7 @@
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 #include <sys/time.h>
+#include <sys/stat.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <errno.h>
@@ -244,12 +245,13 @@ typedef struct
   uint16_t or_port;
   uint16_t op_port;
   uint16_t ap_port;
+  uint16_t dir_port;
  
   crypto_pk_env_t *pkey; /* public RSA key */
  
   /* link info */
-  uint32_t min;
-  uint32_t max;
+  uint32_t bandwidth;
+
 //  struct timeval  min_interval;
  
   /* time when last data was sent to that router */
@@ -392,7 +394,7 @@ aci_t get_unique_aci_by_addr_port(uint32_t addr, uint16_t port, int aci_type);
 
 circuit_t *circuit_get_by_aci_conn(aci_t aci, connection_t *conn);
 circuit_t *circuit_get_by_conn(connection_t *conn);
-circuit_t *circuit_get_by_naddr_nport(uint32_t naddr, uint16_t nport);
+circuit_t *circuit_enumerate_by_naddr_nport(circuit_t *start, uint32_t naddr, uint16_t nport);
 
 int circuit_deliver_data_cell(cell_t *cell, circuit_t *circ, connection_t *conn, int crypt_type);
 int circuit_crypt(circuit_t *circ, char *in, int inlen, char crypt_type);
@@ -487,8 +489,7 @@ int ap_handshake_create_onion(connection_t *conn);
 int ap_handshake_establish_circuit(connection_t *conn, unsigned int *route, int routelen, char *onion,
 		                                   int onionlen, crypt_path_t **cpath);
 
-/* find the circ that's waiting on me, if any, and get it to send its onion */
-int ap_handshake_n_conn_open(connection_t *or_conn);
+void ap_handshake_n_conn_open(connection_t *or_conn);
 
 int ap_handshake_send_onion(connection_t *ap_conn, connection_t *or_conn, circuit_t *circ);
 
@@ -553,7 +554,6 @@ int connection_add(connection_t *conn);
 int connection_remove(connection_t *conn);
 void connection_set_poll_socket(connection_t *conn);
 
-int pkey_cmp(crypto_pk_env_t *a, crypto_pk_env_t *b);
 connection_t *connection_twin_get_by_addr_port(uint32_t addr, uint16_t port);
 connection_t *connection_exact_get_by_addr_port(uint32_t addr, uint16_t port);
 
@@ -625,10 +625,6 @@ tracked_onion_t *id_tracked_onion(unsigned char *onion, uint32_t onionlen, track
 
 /********************************* routers.c ***************************/
 
-routerinfo_t **getrouters(char *routerfile, int *listlenp, uint16_t or_listenport);
-void delete_routerlist(routerinfo_t *list);
-/* create an NULL-terminated array of pointers pointing to elements of a router list */
-routerinfo_t **make_rarray(routerinfo_t* list, int *len);
-
+routerinfo_t **router_get_list_from_file(char *routerfile, int *len, uint16_t or_listenport);
 
 #endif
diff --git a/src/or/routers.c b/src/or/routers.c
index 6838b13c7..4f9b0846e 100644
--- a/src/or/routers.c
+++ b/src/or/routers.c
@@ -9,13 +9,20 @@
  * Matej Pfajfar <mp292@cam.ac.uk>
  */
 
-#define OR_ROUTERLIST_SEPCHARS " \t\n"
-#define OR_PUBLICKEY_BEGIN_TAG "-----BEGIN RSA PUBLIC KEY-----\n"
+#define OR_PUBLICKEY_END_TAG "-----END RSA PUBLIC KEY-----\n"
 
 #include "or.h"
 
 extern int global_role; /* from main.c */
 
+/* static function prototypes */
+static int router_is_me(uint32_t or_address, uint16_t or_listenport, uint16_t my_or_listenport);
+static void routerlist_free(routerinfo_t *list);
+static routerinfo_t **make_rarray(routerinfo_t* list, int *len);
+static char *eat_whitespace(char *s);
+static char *find_whitespace(char *s);
+static routerinfo_t *router_get_entry_from_string(char **s);
+
 /* private function, to determine whether the current entry in the router list is actually us */
 static int router_is_me(uint32_t or_address, uint16_t or_listenport, uint16_t my_or_listenport)
 {
@@ -35,12 +42,12 @@ static int router_is_me(uint32_t or_address, uint16_t or_listenport, uint16_t my
   
   /* obtain local host information */
   if (gethostname(localhostname,512) < 0) {
-    log(LOG_ERR,"Error obtaining local hostname.");
+    log(LOG_ERR,"router_is_me(): Error obtaining local hostname.");
     return -1;
   }
   localhost = gethostbyname(localhostname);
   if (!localhost) {
-    log(LOG_ERR,"Error obtaining local host info.");
+    log(LOG_ERR,"router_is_me(): Error obtaining local host info.");
     return -1;
   }
   
@@ -71,7 +78,7 @@ static int router_is_me(uint32_t or_address, uint16_t or_listenport, uint16_t my
 }
 
 /* delete a list of routers from memory */
-void delete_routerlist(routerinfo_t *list)
+static void routerlist_free(routerinfo_t *list)
 {
   routerinfo_t *tmp = NULL;
   
@@ -91,10 +98,17 @@ void delete_routerlist(routerinfo_t *list)
   return;
 }
 
+void rarray_free(routerinfo_t **list) {
+  if(!list)
+    return;
+  routerlist_free(*list);
+  free(list);
+}
+
 /* create a NULL-terminated array of pointers pointing to elements of a router list */
 /* this is done in two passes through the list - inefficient but irrelevant as this is
  * only done once when op/or start up */
-routerinfo_t **make_rarray(routerinfo_t* list, int *len)
+static routerinfo_t **make_rarray(routerinfo_t* list, int *len)
 {
   routerinfo_t *tmp=NULL;
   int listlen = 0;
@@ -135,303 +149,220 @@ routerinfo_t **make_rarray(routerinfo_t* list, int *len)
   return array;
 }
 
+
 /* load the router list */
-routerinfo_t **getrouters(char *routerfile, int *len, uint16_t or_listenport)
+routerinfo_t **router_get_list_from_file(char *routerfile, int *len, uint16_t or_listenport)
 {
-  int retval = 0;
-  char *retp = NULL;
-  routerinfo_t *router=NULL, *routerlist=NULL, *lastrouter=NULL;
-  FILE *rf; /* router file */
-  fpos_t fpos;
-  char line[512];
-  char *token;
-  char *errtest; /* detecting errors in strtoul() calls */
-  struct hostent *rent;
+  routerinfo_t *routerlist=NULL;
+  routerinfo_t *router;
+  int fd; /* router file */
+  struct stat statbuf;
+  char *string;
+  char *tmps;
 
   assert(routerfile && len);
   
   if (strcspn(routerfile,CONFIG_LEGAL_FILENAME_CHARACTERS) != 0) {
-    log(LOG_ERR,"Filename %s contains illegal characters.",routerfile);
+    log(LOG_ERR,"router_get_list_from_file(): Filename %s contains illegal characters.",routerfile);
     return NULL;
   }
   
+  if(stat(routerfile, &statbuf) < 0) {
+    log(LOG_ERR,"router_get_list_from_file(): Could not stat %s.",routerfile);
+    return NULL;
+  }
+
   /* open the router list */
-  rf = fopen(routerfile,"r");
-  if (!rf) {
-    log(LOG_ERR,"Could not open %s.",routerfile);
+  fd = open(routerfile,O_RDONLY,0);
+  if (fd<0) {
+    log(LOG_ERR,"router_get_list_from_file(): Could not open %s.",routerfile);
+    return NULL;
+  }
+
+  string = malloc(statbuf.st_size+1);
+  if(!string) {
+    log(LOG_ERR,"router_get_list_from_file(): Out of memory.");
+    return NULL;
+  }
+
+  if(read(fd,string,statbuf.st_size) != statbuf.st_size) {
+    log(LOG_ERR,"router_get_list_from_file(): Couldn't read all %d bytes of file '%s'.",statbuf.st_size,routerfile);
     return NULL;
   }
+  close(fd);
   
-  retp = fgets(line,512,rf);
-  while (retp) {
-    log(LOG_DEBUG,"getrouters():Line :%s",line);
-    token = (char *)strtok(line,OR_ROUTERLIST_SEPCHARS);
-    if (token)
-    {
-      log(LOG_DEBUG,"getrouters():Token : %s",token);
-      if (token[0] != '#') /* ignore comment lines */
-      {
-	router = malloc(sizeof(routerinfo_t));
-	if (!router)
-	{
-	  log(LOG_ERR,"Could not allocate memory.");
-	  fclose(rf);
-	  delete_routerlist(routerlist);
-	  return NULL;
-	}
-	
-	/* read the address */
-	router->address = malloc(strlen(token)+1);
-	if (!router->address)
-	{
-	  log(LOG_ERR,"Could not allocate memory.");
-	  fclose(rf);
-	  free((void *)router);
-	  delete_routerlist(routerlist);
-	  return NULL;
-	}
-	strcpy(router->address,token);
-	
-	rent = (struct hostent *)gethostbyname(router->address);
-	if (!rent)
-	{
-	  log(LOG_ERR,"Could not get address for router %s.",router->address);
-	  fclose(rf);
-	  free((void *)router->address);
-	  free((void *)router);
-	  delete_routerlist(routerlist);
-	  return NULL;
-	}
-
-	memcpy(&router->addr, rent->h_addr,rent->h_length);
-	
-	/* read the port */
-	token = (char *)strtok(NULL,OR_ROUTERLIST_SEPCHARS);
-	if (token)
-	{
-	  log(LOG_DEBUG,"getrouters():Token :%s",token);
-	  router->or_port = (uint16_t)strtoul(token,&errtest,0);
-	  if ((*token != '\0') && (*errtest == '\0')) /* conversion was successful */
-	  {
-/* FIXME patch from RD. We should make it actually read these. */
-	    router->op_port = router->or_port + 10;
-	    router->ap_port = router->or_port + 20;
-	    
-	    /* read min bandwidth */
-	    token = (char *)strtok(NULL,OR_ROUTERLIST_SEPCHARS);
-	    if (token) /* min bandwidth */
-	    {
-	      router->min = (uint32_t)strtoul(token,&errtest,0);
-	      if ((*token != '\0') && (*errtest == '\0')) /* conversion was successful */
-	      {
-		if (router->min) /* must not be zero */
-		{
-		  /* read max bandwidth */
-		  token = (char *)strtok(NULL,OR_ROUTERLIST_SEPCHARS);
-		  if (token) /* max bandwidth */
-		  {
-		    router->max = (uint32_t)strtoul(token,&errtest,0);
-		    if ((*token != '\0') && (*errtest == '\0')) /* conversion was successful */
-		    {
-		      if (router->max) /* must not be zero */
-		      {
-			/* check that there is a public key entry for that router */
-			retval = fgetpos(rf, &fpos); /* save the current file position
-						      * we wil return to it later if we find a public key */
-			if (retval == -1)
-			{
-			  log(LOG_ERR,"Could not save position in %s.",routerfile);
-			  free((void *)router->address);
-			  free((void *)router);
-			  fclose(rf);
-			  delete_routerlist(routerlist);
-			  return NULL;
-			}
-			do /* read through to the next non-empty line */
-			{
-			  retp=fgets(line,512,rf);
-			  if (!retp)
-			  {
-			    log(LOG_ERR,"Could not find a public key entry for router %s:%u.",
-				router->address,router->or_port);
-			    free((void *)router->address);
-			    free((void *)router);
-			    fclose(rf);
-			    delete_routerlist(routerlist);
-			    return NULL;
-			  }
-			  log(LOG_DEBUG,"getrouters():Line:%s",line);
-			  if ((*line != '#') && ( strspn(line,OR_ROUTERLIST_SEPCHARS) != strlen(line) ))
-			  {
-			    break;
-			  }
-			} while (1);
-			
-			if (!strcmp(line,OR_PUBLICKEY_BEGIN_TAG)) /* we've got the public key */
-			{
-			  retval = fsetpos(rf,&fpos); /* get us back to where we were otherwise crypto lib won't find the key */
-			  if (retval == -1)
-			  {
-			    log(LOG_ERR,"Could not set position in %s.",routerfile);
-			    free((void *)router->address);
-			    free((void *)router);
-			    fclose(rf);
-			    delete_routerlist(routerlist);
-			    return NULL;
-			  }
-			}
-			else /* we found something else; this isn't right */
-			{
-			  log(LOG_ERR,"Could not find a public key entry for router %s:%u.",
-			      router->address,router->or_port);
-			  free((void *)router->address);
-			  free((void *)router);
-			  fclose(rf);
-			  delete_routerlist(routerlist);
-			  return NULL;
-			}
-			
-			log(LOG_DEBUG,"getrouters():Reading the key ...");
-			/* read the public key into router->pkey */
-			router->pkey = crypto_new_pk_env(CRYPTO_PK_RSA);
-			if (crypto_pk_read_public_key(router->pkey, rf)) /* something went wrong */
-			{
-			  log(LOG_ERR,"Could not read public key for router %s:%u.", 
-			      router->address,router->or_port);
-			  free((void *)router->address);
-			  free((void *)router);
-			  fclose(rf);
-			  delete_routerlist(routerlist);
-			  return NULL;
-			}
-			else /* read the key */
-			{
-			  log(LOG_DEBUG,"getrouters():Public key size = %u.", crypto_pk_keysize(router->pkey));
-			  if (crypto_pk_keysize(router->pkey) != 128) /* keys MUST be 1024 bits in size */
-			  {
-			    log(LOG_ERR,"Key for router %s:%u is not 1024 bits. All keys must be exactly 1024 bits long.",router->address,router->or_port);
-			    free((void *)router->address);
-			    crypto_free_pk_env(router->pkey);
-			    free((void *)router);
-			    fclose(rf);
-			    delete_routerlist(routerlist);
-			    return NULL;
-			  }
-			  
-			  /* check that this router doesn't actually represent us */
-			  retval = router_is_me(router->addr, router->or_port, or_listenport);
-			  if (!retval) { /* this isn't us, continue */
-			    router->next = NULL;
-			    /* save the entry into the routerlist linked list */
-			    if (!routerlist) /* this is the first entry */
-				    routerlist = router;
-			    else
-				    lastrouter->next = (void *)router;
-			    lastrouter = router;
-			  }
-			  else if (retval == 1) /* this is us, ignore */
-			  {
-			    log(LOG_DEBUG,"getrouters(): This entry is actually me. Ignoring.");
-			    free((void *)router->address);
-			    crypto_free_pk_env(router->pkey);
-			    free((void *)router);
-			  }
-			  else /* router_is_me() returned an error */
-			  {
-			    free((void *)router->address);
-			    crypto_free_pk_env(router->pkey);
-			    free((void *)router);
-			    fclose(rf);
-			    delete_routerlist(routerlist);
-			    return NULL;
-			  }
-			}
-		      }
-		      else /* maximum link utilisation is zero */
-		      {
-			log(LOG_ERR,"Entry for router %s doesn't contain a valid maximum bandwidth entry (must be > 0).",router->address);
-			free((void *)router->address);
-			free((void *)router);
-			fclose(rf);
-			delete_routerlist(routerlist);
-			return NULL;
-		      }
-		    }
-		    else
-		    {
-		      log(LOG_ERR,"Entry for router %s doesn't seem to contain a valid maximum bandwidth entry.",router->address);
-		      free((void *)router->address);
-		      free((void *)router);
-		      fclose(rf);
-		      delete_routerlist(routerlist);
-		      return NULL;
-		    }
-		  }
-		  else
-		  {
-		    log(LOG_ERR,"Entry for router %s doesn't seem to contain a maximum bandwidth entry.",router->address);
-		    free((void *)router->address);
-		    free((void *)router);
-		    fclose(rf);
-		    delete_routerlist(routerlist);
-		    return NULL;
-		  }
-		}
-		else
-		{
-		  log(LOG_ERR,"Entry for router %s doesn't contain a valid minimum bandwidth entry (must be > 0).",router->address);
-		  free((void *)router->address);
-		  free((void *)router);
-		  fclose(rf);
-		  delete_routerlist(routerlist);
-		  return NULL;
-		}
-	      }
-	      else
-	      {
-		log(LOG_ERR,"Entry for router %s doesn't seem to contain a valid minimum bandwidth entry.",router->address);
-		free((void *)router->address);
-		free((void *)router);
-		fclose(rf);
-		delete_routerlist(routerlist);
-		return NULL;
-	      }
-	    }
-	    else
-	    {
-	      log(LOG_ERR,"Entry for router %s doesn't seem to contain a minimum bandwidth entry.",router->address);
-	      free((void *)router->address);
-	      free((void *)router);
-	      fclose(rf);
-	      delete_routerlist(routerlist);
-	      return NULL;
-	    }
-	  }
-	  else
-	  {
-	    log(LOG_ERR,"Entry for router %s doesn't seem to contain a valid port number.",router->address);
-	    free((void *)router->address);
-	    free((void *)router);
-	    fclose(rf);
-	    delete_routerlist(routerlist);
-	    return NULL;
-	  }
-	}
-	else
-	{
-	  log(LOG_ERR,"Entry for router %s doesn't seem to contain a port number.",router->address);
-	  free((void *)router->address);
-	  free((void *)router);
-	  fclose(rf);
-	  delete_routerlist(routerlist);
-	  return NULL;
-	}
-      }
+  string[statbuf.st_size] = 0; /* null terminate it */
+  tmps = string;
+  while(*tmps) { /* while not at the end of the string */
+    router = router_get_entry_from_string(&tmps);
+    if(router == NULL) {
+      routerlist_free(routerlist);
+      free(string);
+      return NULL;
+    }
+    if(!router_is_me(router->addr, router->or_port, or_listenport)) {
+      router->next = routerlist;
+      routerlist = router;
     }
-    retp=fgets(line,512,rf);
+    tmps = eat_whitespace(tmps);
   }
-  
-  fclose(rf);
+  free(string);
   return make_rarray(routerlist, len);
+} 
+
+/* return the first char of s that is not whitespace and not a comment */
+static char *eat_whitespace(char *s) {
+  assert(s);
+
+  while(isspace(*s) || *s == '#') {
+    while(isspace(*s))
+      s++;
+    if(*s == '#') { /* read to a \n or \0 */
+      while(*s && *s != '\n')
+        s++;
+      if(!*s)
+        return s;
+    }
+  }
+  return s;
+}
+
+/* return the first char of s that is whitespace or '#' or '\0 */
+static char *find_whitespace(char *s) {
+  assert(s);
+
+  while(*s && !isspace(*s) && *s != '#')
+    s++;
+
+  return s;
+}
+
+/* reads a single router entry from s.
+ * updates s so it points to after the router it just read.
+ * mallocs a new router, returns it if all goes well, else returns NULL.
+ */
+static routerinfo_t *router_get_entry_from_string(char **s) {
+  routerinfo_t *router;
+  char *next;
+  struct hostent *rent;
+
+  router = malloc(sizeof(routerinfo_t));
+  if (!router) {
+    log(LOG_ERR,"router_get_entry_from_string(): Could not allocate memory.");
+    return NULL;
+  }
+  memset(router,0,sizeof(routerinfo_t)); /* zero it out first */
+
+#define NEXT_TOKEN(s, next)    \
+  *s = eat_whitespace(*s);     \
+  next = find_whitespace(*s);  \
+  if(!*next) {                 \
+    goto router_read_failed;   \
+  }                            \
+  *next = 0;
+
+  /* read router->address */
+  NEXT_TOKEN(s, next);
+  router->address = strdup(*s);
+  *s = next+1;
+
+  rent = (struct hostent *)gethostbyname(router->address);
+  if (!rent) {
+    log(LOG_ERR,"router_get_entry_from_string(): Could not get address for router %s.",router->address);
+    goto router_read_failed;
+  }
+  assert(rent->h_length == 4);
+  memcpy(&router->addr, rent->h_addr,rent->h_length);
+
+  /* read router->or_port */
+  NEXT_TOKEN(s, next);
+  router->or_port = atoi(*s);
+  if(!router->or_port) {
+    log(LOG_ERR,"router_get_entry_from_string(): or_port '%s' unreadable or 0. Failing.",*s);
+    goto router_read_failed;
+  }
+  *s = next+1;
+  
+  /* read router->op_port */
+  NEXT_TOKEN(s, next);
+  router->op_port = atoi(*s);
+  *s = next+1;
+  
+  /* read router->ap_port */
+  NEXT_TOKEN(s, next);
+  router->ap_port = atoi(*s);
+  *s = next+1;
+  
+  /* read router->dir_port */
+  NEXT_TOKEN(s, next);
+  router->dir_port = atoi(*s);
+  *s = next+1;
+
+  /* read router->bandwidth */
+  NEXT_TOKEN(s, next);
+  router->bandwidth = atoi(*s);
+  if(!router->bandwidth) {
+    log(LOG_ERR,"router_get_entry_from_string(): bandwidth '%s' unreadable or 0. Failing.",*s);
+    goto router_read_failed;
+  }
+  *s = next+1;
+
+  log(LOG_DEBUG,"or_port %d, op_port %d, ap_port %d, dir_port %d, bandwidth %d.",
+    router->or_port, router->op_port, router->ap_port, router->dir_port, router->bandwidth);
+
+  *s = eat_whitespace(*s); 
+  next = strstr(*s,OR_PUBLICKEY_END_TAG);
+  router->pkey = crypto_new_pk_env(CRYPTO_PK_RSA);
+  if(!next || !router->pkey) {
+    log(LOG_ERR,"router_get_entry_from_string(): Couldn't find pk in string");
+    goto router_read_failed;
+  }
+  
+  /* now advance *s so it's at the end of this router entry */
+  next = strchr(next, '\n');
+  assert(next); /* can't fail, we just checked it was here */
+  *next = 0;
+  log(LOG_DEBUG,"Key about to be read is: '%s'",*s);
+  if((crypto_pk_read_public_key_from_string(router->pkey, *s, strlen(*s))<0)) {
+    log(LOG_ERR,"router_get_entry_from_string(): Couldn't read pk from string");
+    goto router_read_failed;
+  }
+  log(LOG_DEBUG,"router_get_entry_from_string(): Public key size = %u.", crypto_pk_keysize(router->pkey));
+
+  if (crypto_pk_keysize(router->pkey) != 128) { /* keys MUST be 1024 bits in size */
+    log(LOG_ERR,"Key for router %s:%u is not 1024 bits. All keys must be exactly 1024 bits long.",
+      router->address,router->or_port);
+    goto router_read_failed;
+  }
+
+//  test_write_pkey(router->pkey);  
+
+  *s = next+1;
+
+  /* success */
+  return(router);
+
+router_read_failed:
+  if(router->address)
+    free(router->address);
+  if(router->pkey)
+    crypto_free_pk_env(router->pkey);
+  free(router);
+  return NULL;
+}
+
+#if 0
+void test_write_pkey(crypto_pk_env_t *pkey) {
+  char *string;
+  int len;
+
+  log(LOG_DEBUG,"Trying test write.");
+  if(crypto_pk_write_public_key_to_string(pkey,&string,&len)<0) {
+    log(LOG_DEBUG,"router_get_entry_from_string(): write pkey to string failed\n");
+    return;
+  }
+  log(LOG_DEBUG,"I did it: len %d, string '%s'.",len,string);
+  free(string);
 }
+#endif