[or-cvs] Resolve some XXXs

Nick Mathewson nickm at seul.org
Tue May 18 15:35:24 UTC 2004


Update of /home/or/cvsroot/src/or
In directory moria.mit.edu:/tmp/cvs-serv29718/src/or

Modified Files:
	buffers.c circuitlist.c dirserv.c main.c rendcommon.c 
	rendservice.c rephist.c routerparse.c 
Log Message:
Resolve some XXXs

Index: buffers.c
===================================================================
RCS file: /home/or/cvsroot/src/or/buffers.c,v
retrieving revision 1.92
retrieving revision 1.93
diff -u -d -r1.92 -r1.93
--- buffers.c	10 May 2004 07:27:29 -0000	1.92
+++ buffers.c	18 May 2004 15:35:21 -0000	1.93
@@ -91,29 +91,13 @@
   buf_shrink_if_underfull(buf);
 }
 
-/** Find the first instance of the str_len byte string <b>str</b> on the
- * buf_len byte string <b>bufstr</b>.  Strings are not necessary
- * NUL-terminated. If none exists, return -1.  Otherwise, return index
- * of the first character in bufstr _after_ the first instance of str.
- */
-/* XXXX The way this function is used, we could always get away with
- * XXXX assuming that str is NUL terminated, and use strstr instead. */
-static int find_mem_in_mem(const char *str, int str_len,
-                           const char *bufstr, int buf_len)
+/** Make sure that the memory in buf ends with a zero byte. */
+static INLINE int buf_nul_terminate(buf_t *buf)
 {
-  const char *location;
-  const char *last_possible = bufstr + buf_len - str_len;
-
-  tor_assert(str && str_len > 0 && bufstr);
-
-  if(buf_len < str_len)
+  if (buf_ensure_capacity(buf,buf->len+1)<0)
     return -1;
-
-  for(location = bufstr; location <= last_possible; location++)
-    if((*location == *str) && !memcmp(location+1, str+1, str_len-1))
-      return location-bufstr+str_len;
-
-  return -1;
+  buf->mem[buf->len] = '\0';
+  return 0;
 }
 
 /** Create and return a new buf with capacity <b>size</b>.
@@ -366,19 +350,22 @@
 int fetch_from_buf_http(buf_t *buf,
                         char **headers_out, int max_headerlen,
                         char **body_out, int *body_used, int max_bodylen) {
-  char *headers, *body;
-  int i;
+  char *headers, *body, *p;
   int headerlen, bodylen, contentlen;
 
   assert_buf_ok(buf);
 
   headers = buf->mem;
-  i = find_mem_in_mem("\r\n\r\n", 4, buf->mem, buf->datalen);
-  if(i < 0) {
+  if (buf_nul_terminate(buf)<0) {
+    log_fn(LOG_WARN,"Couldn't nul-terminate buffer");
+    return -1;
+  }
+  body = strstr(headers,"\r\n\r\n");
+  if (!body) {
     log_fn(LOG_DEBUG,"headers not all here yet.");
     return 0;
   }
-  body = buf->mem+i;
+  body += 4; /* Skip the the CRLFCRLF */
   headerlen = body-headers; /* includes the CRLFCRLF */
   bodylen = buf->datalen - headerlen;
   log_fn(LOG_DEBUG,"headerlen %d, bodylen %d.", headerlen, bodylen);
@@ -393,10 +380,9 @@
   }
 
 #define CONTENT_LENGTH "\r\nContent-Length: "
-  i = find_mem_in_mem(CONTENT_LENGTH, strlen(CONTENT_LENGTH),
-                      headers, headerlen);
-  if(i > 0) {
-    contentlen = atoi(headers+i);
+  p = strstr(headers, CONTENT_LENGTH);
+  if (p) {
+    contentlen = atoi(p+strlen(CONTENT_LENGTH));
     /* if content-length is malformed, then our body length is 0. fine. */
     log_fn(LOG_DEBUG,"Got a contentlen of %d.",contentlen);
     if(bodylen < contentlen) {

Index: circuitlist.c
===================================================================
RCS file: /home/or/cvsroot/src/or/circuitlist.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -d -r1.2 -r1.3
--- circuitlist.c	13 May 2004 07:44:21 -0000	1.2
+++ circuitlist.c	18 May 2004 15:35:21 -0000	1.3
@@ -419,25 +419,23 @@
  * correct. Trigger an assert if anything is invalid.
  */
 static void
- assert_cpath_ok(const crypt_path_t *cp)
+assert_cpath_ok(const crypt_path_t *cp)
 {
-  while(cp->prev)
-    cp = cp->prev;
-  //XXX this is broken. cp is a doubly linked list.
+  const crypt_path_t *start = cp;
 
-  while(cp->next) {
+  do {
     assert_cpath_layer_ok(cp);
     /* layers must be in sequence of: "open* awaiting? closed*" */
-    if (cp->prev) {
-      if (cp->prev->state == CPATH_STATE_OPEN) {
-        tor_assert(cp->state == CPATH_STATE_CLOSED ||
-                   cp->state == CPATH_STATE_AWAITING_KEYS);
-      } else {
-        tor_assert(cp->state == CPATH_STATE_CLOSED);
+    if (cp != start) {
+      if (cp->state == CPATH_STATE_AWAITING_KEYS) {
+        tor_assert(cp->prev->state == CPATH_STATE_OPEN);
+      } else if (cp->state == CPATH_STATE_OPEN) {
+        tor_assert(cp->prev->state == CPATH_STATE_OPEN);
       }
     }
     cp = cp->next;
-  }
+    tor_assert(cp);
+  } while (cp != start);
 }
 
 /** Verify that circuit <b>c</b> has all of its invariants
@@ -479,8 +477,7 @@
     }
   }
   if (c->cpath) {
-//    assert_cpath_ok(c->cpath);
-// XXX the above call causes an infinite loop.
+    assert_cpath_ok(c->cpath);
   }
   if (c->purpose == CIRCUIT_PURPOSE_REND_ESTABLISHED) {
     if (!c->marked_for_close) {

Index: dirserv.c
===================================================================
RCS file: /home/or/cvsroot/src/or/dirserv.c,v
retrieving revision 1.47
retrieving revision 1.48
diff -u -d -r1.47 -r1.48
--- dirserv.c	17 May 2004 20:31:01 -0000	1.47
+++ dirserv.c	18 May 2004 15:35:21 -0000	1.48
@@ -331,9 +331,12 @@
     free_descriptor_entry(*desc_ent_ptr);
   } else {
     /* Add this at the end. */
-    log_fn(LOG_INFO,"Dirserv adding desc for nickname %s",ri->nickname);
-    desc_ent_ptr = &descriptor_list[n_descriptors++];
-    /* XXX check if n_descriptors is too big */
+    if (n_descriptors >= MAX_ROUTERS_IN_DIR) {
+      log_fn(LOG_WARN,"Too many descriptors in directory; can't add another.");
+    } else {
+      log_fn(LOG_INFO,"Dirserv adding desc for nickname %s",ri->nickname);
+      desc_ent_ptr = &descriptor_list[n_descriptors++];
+    }
   }
 
   (*desc_ent_ptr) = tor_malloc(sizeof(descriptor_entry_t));

Index: main.c
===================================================================
RCS file: /home/or/cvsroot/src/or/main.c,v
retrieving revision 1.268
retrieving revision 1.269
diff -u -d -r1.268 -r1.269
--- main.c	17 May 2004 20:31:01 -0000	1.268
+++ main.c	18 May 2004 15:35:21 -0000	1.269
@@ -215,8 +215,6 @@
     connection_handle_read(conn) < 0) {
       if (!conn->marked_for_close) {
         /* this connection is broken. remove it */
-        /* XXX This shouldn't ever happen anymore. */
-        /* XXX but it'll clearly happen on MS_WINDOWS from POLLERR, right? */
         log_fn(LOG_ERR,"Unhandled error on read for %s connection (fd %d); removing",
                CONN_TYPE_TO_STRING(conn->type), conn->s);
         connection_mark_for_close(conn);
@@ -289,7 +287,6 @@
     if(connection_speaks_cells(conn)) {
       if(conn->state == OR_CONN_STATE_OPEN) {
         retval = flush_buf_tls(conn->tls, conn->outbuf, &conn->outbuf_flushlen);
-        /* XXX actually, some non-zero results are maybe ok. which ones? */
       } else
         retval = -1; /* never flush non-open broken tls connections */
     } else {

Index: rendcommon.c
===================================================================
RCS file: /home/or/cvsroot/src/or/rendcommon.c,v
retrieving revision 1.32
retrieving revision 1.33
diff -u -d -r1.32 -r1.33
--- rendcommon.c	12 May 2004 20:58:27 -0000	1.32
+++ rendcommon.c	18 May 2004 15:35:21 -0000	1.33
@@ -44,7 +44,7 @@
   char *buf, *cp, *ipoint;
   int i, keylen, asn1len;
   keylen = crypto_pk_keysize(desc->pk);
-  buf = tor_malloc(keylen*2); /* XXXX */
+  buf = tor_malloc(keylen*2); /* Too long, but that's okay. */
   asn1len = crypto_pk_asn1_encode(desc->pk, buf, keylen*2);
   if (asn1len<0) {
     tor_free(buf);

Index: rendservice.c
===================================================================
RCS file: /home/or/cvsroot/src/or/rendservice.c,v
retrieving revision 1.69
retrieving revision 1.70
diff -u -d -r1.69 -r1.70
--- rendservice.c	18 May 2004 01:53:53 -0000	1.69
+++ rendservice.c	18 May 2004 15:35:21 -0000	1.70
@@ -482,7 +482,7 @@
   circuit_t *newcirc;
   cpath_build_state_t *newstate, *oldstate;
 
-  /* XXXX assert type and build_state */
+  tor_assert(oldcirc->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND);
 
   if (!oldcirc->build_state ||
       oldcirc->build_state->failure_count > MAX_REND_FAILURES) {

Index: rephist.c
===================================================================
RCS file: /home/or/cvsroot/src/or/rephist.c,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -d -r1.8 -r1.9
--- rephist.c	10 May 2004 04:34:48 -0000	1.8
+++ rephist.c	18 May 2004 15:35:21 -0000	1.9
@@ -154,10 +154,9 @@
 {
   or_history_t *hist;
   if(!nickname) {
-    /* XXX
-     * If conn has no nickname, it's either an OP, or it is an OR
+    /* If conn has no nickname, it's either an OP, or it is an OR
      * which didn't complete its handshake (or did and was unapproved).
-     * Ignore it. Is there anything better we could do?
+     * Ignore it.
      */
     return;
   }

Index: routerparse.c
===================================================================
RCS file: /home/or/cvsroot/src/or/routerparse.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -d -r1.1 -r1.2
--- routerparse.c	10 May 2004 17:30:51 -0000	1.1
+++ routerparse.c	18 May 2004 15:35:21 -0000	1.2
@@ -8,10 +8,9 @@
  * \brief Code to parse and validate router descriptors and directories.
  **/
 
-#define _GNU_SOURCE
-/* XXX this is required on rh7 to make strptime not complain. how bad
- * is this for portability?
+/* This is required on rh7 to make strptime not complain.
  */
+#define _GNU_SOURCE
 
 #include "or.h"
 
@@ -32,7 +31,7 @@
   K_SIGNED_DIRECTORY,
   K_SIGNING_KEY,
   K_ONION_KEY,
-  K_LINK_KEY, /* XXXX obsolete */
+  K_LINK_KEY, /* XXXX obsolete; remove in June. */
   K_ROUTER_SIGNATURE,
   K_PUBLISHED,
   K_RUNNING_ROUTERS,



More information about the tor-commits mailing list