From 4d70094b6ebf9557cb7b22ebad6037ab4e221ff8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 6 Feb 2008 19:34:32 +0000 Subject: [PATCH] r17951@catbus: nickm | 2008-02-06 14:34:13 -0500 Add more documentation; change the behavior of read_to_buf_tls to be more consistent. Note a longstanding problem with current read/write interfaces. svn:r13407 --- src/common/mempool.c | 3 +++ src/or/buffers.c | 41 ++++++++++++++++++++++++----------------- src/or/connection.c | 6 +++--- src/or/rephist.c | 5 ++++- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/common/mempool.c b/src/common/mempool.c index e4e81b759d..e33edd1eed 100644 --- a/src/common/mempool.c +++ b/src/common/mempool.c @@ -400,6 +400,9 @@ mp_pool_new(size_t item_size, size_t chunk_capacity) void mp_pool_clean(mp_pool_t *pool, int n) { + /* XXXX020 this is stupid. We shouldn't care about empty chunks if there + * is lots of space in used chunks. */ + mp_chunk_t *chunk, **first_to_free; if (n < 0) { /* As said in the documentation, "negative n" means "leave an additional diff --git a/src/or/buffers.c b/src/or/buffers.c index 940c6ccb46..2a89505b92 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -548,7 +548,10 @@ buf_add_chunk_with_capacity(buf_t *buf, size_t capacity, int capped) return chunk; } -/** DOCDOC */ +/** Read up to at_most bytes from the socket fd into + * chunk (which must be on buf/b>). If we get an EOF, set + * *reached_eof to 1. Return -1 on error, 0 on eof or blocking, + * and the number of bytes read otherwise. */ static INLINE int read_to_chunk(buf_t *buf, chunk_t *chunk, int fd, size_t at_most, int *reached_eof) @@ -580,7 +583,8 @@ read_to_chunk(buf_t *buf, chunk_t *chunk, int fd, size_t at_most, } } -/** DOCDOC */ +/** As read_to_chunk(), but return (negative) error code on error, blocking, + * or TLS, and the number of bytes read otherwise. */ static INLINE int read_to_chunk_tls(buf_t *buf, chunk_t *chunk, tor_tls_t *tls, size_t at_most) @@ -597,16 +601,17 @@ read_to_chunk_tls(buf_t *buf, chunk_t *chunk, tor_tls_t *tls, } /** Read from socket s, writing onto end of buf. Read at most - * at_most bytes, resizing the buffer as necessary. If recv() - * returns 0, set *reached_eof to 1 and return 0. Return -1 on error; - * else return the number of bytes read. Return 0 if recv() would - * block. - * - * DOCDOC revise + * at_most bytes, growing the buffer as necessary. If recv() returns 0 + * (because of EOF), set *reached_eof to 1 and return 0. Return -1 on + * error; else return the number of bytes read. */ +/* XXXX020 indicate "read blocked" somehow? */ int read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof) { + /* XXXX020 It's stupid to overload the return values for these functions: + * "error status" and "number of bytes read" are not mutually exclusive. + */ int r = 0; size_t total_read = 0; @@ -639,7 +644,8 @@ read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof) return r; } -/** As read_to_buf, but reads from a TLS connection. +/** As read_to_buf, but reads from a TLS connection, and returns a TLS + * status value rather than the number of bytes read. * * Using TLS on OR connections complicates matters in two ways. * @@ -657,8 +663,6 @@ read_to_buf(int s, size_t at_most, buf_t *buf, int *reached_eof) * Second, the TLS stream's events do not correspond directly to network * events: sometimes, before a TLS stream can read, the network must be * ready to write -- or vice versa. - * - * DOCDOC revise */ int read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf) @@ -686,20 +690,20 @@ read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf) if (r < 0) return r; /* Error */ else if ((size_t)r < readlen) /* eof, block, or no more to read. */ - return r + total_read; + return r; total_read += r; } return r; } -/** Helper for flush_buf(): try to write sz bytes from buffer - * buf onto socket s. On success, deduct the bytes written - * from *buf_flushlen. - * Return the number of bytes written on success, -1 on failure. +/** Helper for flush_buf(): try to write sz bytes from chunk + * chunk of buffer buf onto socket s. On success, deduct + * the bytes written from *buf_flushlen. Return the number of bytes + * written on success, 0 on blocking, -1 on failure. */ static INLINE int flush_chunk(int s, buf_t *buf, chunk_t *chunk, size_t sz, - size_t *buf_flushlen) + size_t *buf_flushlen) { int write_result; @@ -764,6 +768,9 @@ flush_chunk_tls(tor_tls_t *tls, buf_t *buf, chunk_t *chunk, int flush_buf(int s, buf_t *buf, size_t sz, size_t *buf_flushlen) { + /* XXXX020 It's stupid to overload the return values for these functions: + * "error status" and "number of bytes flushed" are not mutually exclusive. + */ int r; size_t flushed = 0; tor_assert(buf_flushlen); diff --git a/src/or/connection.c b/src/or/connection.c index daee148080..2a268f5e25 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1912,6 +1912,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read) conn->state > OR_CONN_STATE_PROXY_READING) { int pending; or_connection_t *or_conn = TO_OR_CONN(conn); + size_t initial_size; if (conn->state == OR_CONN_STATE_TLS_HANDSHAKING || conn->state == OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING) { /* continue handshaking even if global token bucket is empty */ @@ -1924,6 +1925,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read) conn->s,(int)buf_datalen(conn->inbuf), tor_tls_get_pending_bytes(or_conn->tls), at_most); + initial_size = buf_datalen(conn->inbuf); /* else open, or closing */ result = read_to_buf_tls(or_conn->tls, at_most, conn->inbuf); if (TOR_TLS_IS_ERROR(result) || result == TOR_TLS_CLOSE) @@ -1963,11 +1965,9 @@ connection_read_to_buf(connection_t *conn, int *max_to_read) if (r2<0) { log_warn(LD_BUG, "apparently, reading pending bytes can fail."); return -1; - } else { - result += r2; } } - + result = (int)(buf_datalen(conn->inbuf)-initial_size); tor_tls_get_n_raw_bytes(or_conn->tls, &n_read, &n_written); log_debug(LD_GENERAL, "After TLS read of %d: %ld read, %ld written", result, (long)n_read, (long)n_written); diff --git a/src/or/rephist.c b/src/or/rephist.c index 8908e03294..0e451a1523 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -19,6 +19,7 @@ static void bw_arrays_init(void); static void predicted_ports_init(void); static void hs_usage_init(void); +/**DOCDOC*/ uint64_t rephist_total_alloc=0; uint32_t rephist_total_num=0; @@ -732,8 +733,10 @@ find_next_with(smartlist_t *sl, int i, const char *prefix) return -1; } +/** How many bad times has parse_possibly_bad_iso_time parsed? */ static int n_bogus_times = 0; -/** DOCDOC */ +/** Parse the ISO-formatted time in s into *time_out, but + * rounds any pre-1970 date to Jan 1, 1970. */ static int parse_possibly_bad_iso_time(const char *s, time_t *time_out) {