Commit 7511fbf9 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Resolve some XXXs


svn:r1889
parent 9da17146
Loading
Loading
Loading
Loading
+6 −2
Original line number Diff line number Diff line
@@ -375,7 +375,10 @@ tor_tls_context_new(crypto_pk_env_t *identity,
    SSL_CTX_free(result->ctx);
  if (result)
    free(result);
  /* leak certs XXXX ? */
  if (cert)
    X509_free(cert);
  if (idcert)
    X509_free(cert);
  return -1;
}

@@ -641,7 +644,8 @@ tor_tls_verify(tor_tls *tls, crypto_pk_env_t *identity_key)
  if (id_pkey)
    EVP_PKEY_free(id_pkey);

  /* XXXX This should never get invoked, but let's make sure for now. */
  /* This should never get invoked, but let's make sure in case OpenSSL
   * acts unexpectedly. */
  tls_log_errors(LOG_WARN, "finishing tor_tls_verify");

  return r;
+22 −1
Original line number Diff line number Diff line
@@ -1323,7 +1323,28 @@ write_str_to_file(const char *fname, const char *str)
    return -1;
  }
  fclose(file);
  /* XXXX This won't work on windows: you can't use rename to replace a file.*/

#ifdef MS_WINDOWS
  /* On Windows, rename doesn't replace.  We could call ReplaceFile, but
   * that's hard, and we can probably sneak by without atomicity. */
  switch (file_status(fname)) {
    case FN_ERROR:
      log(LOG_WARN, "Error replacing %s: %s", fname, strerror(errno));
      return -1;
    case FN_DIR:
      log(LOG_WARN, "Error replacing %s: is directory", fname);
      return -1;
    case FN_FILE:
      if (unlink(fname)) {
        log(LOG_WARN, "Error replacing %s while removing old copy: %s",
            fname, strerror(errno));
        return -1;
      }
      break;
    case FN_NOENT:
      ;
  }
#endif
  if (rename(tempname, fname)) {
    log(LOG_WARN, "Error replacing %s: %s", fname, strerror(errno));
    return -1;
+5 −3
Original line number Diff line number Diff line
@@ -77,10 +77,12 @@
#define tor_close_socket(s) close(s)
#endif


/* XXXX This isn't so on windows. */
/** Legal characters in a filename */
#ifdef MS_WINDOWS
#define CONFIG_LEGAL_FILENAME_CHARACTERS "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_/\\ "
#else
#define CONFIG_LEGAL_FILENAME_CHARACTERS "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_/ "
#endif

size_t strlcat(char *dst, const char *src, size_t siz);
size_t strlcpy(char *dst, const char *src, size_t siz);
+16 −30
Original line number Diff line number Diff line
@@ -91,29 +91,13 @@ static INLINE void buf_remove_from_front(buf_t *buf, size_t n) {
  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)
    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;

  if (buf_ensure_capacity(buf,buf->len+1)<0)
    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(char *string, size_t string_len, buf_t *buf) {
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 @@ int fetch_from_buf_http(buf_t *buf,
  }

#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) {
+11 −14
Original line number Diff line number Diff line
@@ -421,23 +421,21 @@ void assert_cpath_layer_ok(const crypt_path_t *cp)
static void
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 @@ void assert_circuit_ok(const circuit_t *c)
    }
  }
  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) {
Loading