Commit 418c2e1b authored Feb 15, 2008 by Nick Mathewson

r14181@tombo: nickm | 2008-02-15 16:48:17 -0500

 Fix all but 2 DOCDOC items; defer many XXX020s (particularly those where fixing them would fix no bugs at the risk of introducing some bugs).


svn:r13529

parent 861c79dc

src/common/compat.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -340,7 +340,7 @@ typedef struct tor_addr_t
		} addr;
		} tor_addr_t;

		/* XXXX020 rename these. */
		/* XXXX021 rename these. */
		static INLINE uint32_t IPV4IP(const tor_addr_t *a);
		static INLINE uint32_t IPV4IPh(const tor_addr_t *a);
		static INLINE uint32_t IPV4MAPh(const tor_addr_t *a);

src/or/buffers.c

+8 −15

Original line number	Diff line number	Diff line
		@@ -119,10 +119,8 @@ typedef struct chunk_freelist_t {

		/** Static array of freelists, sorted by alloc_len, terminated by an entry
		* with alloc_size of 0. */
		/*XXXX020 tune these values. And all allocation sizes, really. /
		static chunk_freelist_t freelists[] = {
		FL(256, 1024, 16), FL(512, 1024, 16), FL(1024, 512, 8), FL(4096, 256, 8),
		FL(8192, 128, 4), FL(16384, 64, 4), FL(32768, 32, 2), FL(65536, 16, 2),
		FL(4096, 256, 8), FL(8192, 128, 4), FL(16384, 64, 4), FL(32768, 32, 2),
		FL(0, 0, 0)
		};
		#undef FL
		@@ -180,7 +178,7 @@ chunk_new_with_alloc_size(size_t alloc)
		freelist->lowest_length = freelist->cur_length;
		++freelist->n_hit;
		} else {
		/* XXXX020 take advantage of tor_malloc_roundup, once we know how that
		/* XXXX021 take advantage of tor_malloc_roundup, once we know how that
		* affects freelists. */
		if (freelist)
		++freelist->n_alloc;
		@@ -244,8 +242,6 @@ chunk_grow(chunk_t *chunk, size_t sz)
		static INLINE size_t
		preferred_chunk_size(size_t target)
		{
		/* XXXX020 use log2 code, maybe. */
		/* XXXX020 or make sizing code more fine-grained! */
		size_t sz = MIN_CHUNK_ALLOC;
		while (CHUNK_SIZE_WITH_ALLOC(sz) < target) {
		sz <<= 1;
		@@ -418,10 +414,7 @@ buf_pullup(buf_t *buf, size_t bytes, int nulterminate)
		}

		/** Resize buf so it won't hold extra memory that we haven't been
		* using lately (that is, since the last time we called buf_shrink).
		* Try to shrink the buf until it is the largest factor of two that
		* can contain <b>buf</b>->highwater, but never smaller than
		* MIN_LAZY_SHRINK_SIZE.
		* using lately.
		*/
		void
		buf_shrink(buf_t *buf)
		@@ -454,8 +447,8 @@ buf_remove_from_front(buf_t *buf, size_t n)
		check();
		}

		/** Create and return a new buf with capacity <b>size</b>.
		* (Used for testing). */
		/** Create and return a new buf with default chunk capacity <b>size</b>.
		*/
		buf_t *
		buf_new_with_capacity(size_t size)
		{
		@@ -609,11 +602,11 @@ read_to_chunk_tls(buf_t buf, chunk_t chunk, tor_tls_t *tls,
		* (because of EOF), set *<b>reached_eof</b> to 1 and return 0. Return -1 on
		* error; else return the number of bytes read.
		*/
		/* XXXX020 indicate "read blocked" somehow? */
		/* XXXX021 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:
		/* XXXX021 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;
		@@ -777,7 +770,7 @@ 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:
		/* XXXX021 It's stupid to overload the return values for these functions:
		* "error status" and "number of bytes flushed" are not mutually exclusive.
		*/
		int r;

src/or/circuitlist.c

+1 −1

Original line number	Diff line number	Diff line
		@@ -806,7 +806,7 @@ origin_circuit_t *
		circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
		int flags)
		{
		/*XXXX020 arma: The purpose argument is ignored. Can that possibly be
		/*XXXX021 arma: The purpose argument is ignored. Can that possibly be
		* right? */
		/* XXXX <arma> i don't know of any actual bugs that this causes. since i
		* think we only call the function for purposes where we want it to do what

src/or/connection_edge.c

+18 −10

Original line number	Diff line number	Diff line
		@@ -925,9 +925,12 @@ client_dns_set_reverse_addressmap(const char address, const char v,
		*
		* These options are configured by parse_virtual_addr_network().
		*/
		/DOCDOC options /
		/** Which network should we use for virtual IPv4 addresses? Only the first
		* bits of this value are fixed. */
		static uint32_t virtual_addr_network = 0x7fc00000u;
		/** How many bits of <b>virtual_addr_network</b> are fixed? */
		static maskbits_t virtual_addr_netmask_bits = 10;
		/** What's the next virtual address we will hand out? */
		static uint32_t next_virtual_addr = 0x7fc00000u;

		/** Read a netmask of the form 127.192.0.0/10 from "val", and check whether
		@@ -1944,7 +1947,7 @@ connection_ap_handshake_send_resolve(edge_connection_t *ap_conn)
		uint32_t a;
		size_t len = strlen(ap_conn->socks_request->address);
		char c = 0;
		/* XXXX020 This logic is a little ugly: we check for an in-addr.arpa ending
		/* XXXX021 This logic is a little ugly: we check for an in-addr.arpa ending
		* on the address. If we have one, the address is already in the right
		* order, so we'll leave it alone later. Otherwise, we reverse it and
		* turn it into an in-addr.arpa address. */
		@@ -1958,11 +1961,11 @@ connection_ap_handshake_send_resolve(edge_connection_t *ap_conn)
		return -1;
		}
		if (c) {
		/* this path happens on DNS. Can we unify? XXXX020 */
		/* this path happens on DNS. Can we unify? XXXX021 */
		ap_conn->socks_request->address[len-13] = c;
		strlcpy(inaddr_buf, ap_conn->socks_request->address, sizeof(inaddr_buf));
		} else {
		/* this path happens on tor-resolve. Can we unify? XXXX020 */
		/* this path happens on tor-resolve. Can we unify? XXXX021 */
		a = ntohl(in.s_addr);
		tor_snprintf(inaddr_buf, sizeof(inaddr_buf), "%d.%d.%d.%d.in-addr.arpa",
		(int)(uint8_t)((a )&0xff),
		@@ -2085,12 +2088,15 @@ tell_controller_about_resolved_result(edge_connection_t *conn,
		}
		}

		/** Send an answer to an AP connection that has requested a DNS lookup
		* via SOCKS. The type should be one of RESOLVED_TYPE_(IPV4\|IPV6\|HOSTNAME) or
		* -1 for unreachable; the answer should be in the format specified
		* in the socks extensions document.
		* DOCDOC ttl expires
		/** Send an answer to an AP connection that has requested a DNS lookup via
		* SOCKS. The type should be one of RESOLVED_TYPE_(IPV4\|IPV6\|HOSTNAME) or -1
		* for unreachable; the answer should be in the format specified in the socks
		* extensions document. <b>ttl</b> is the ttl for the answer, or -1 on
		* certain errors or for values that didn't come via DNS. <b>expires</b> is
		* a time when the answer expires, or -1 or TIME_MAX if there's a good TTL.
		**/
		/* XXXX021 the use of the ttl and expires fields is nutty. Let's make this
		* interface and those that use it less ugly. */
		void
		connection_ap_handshake_socks_resolved(edge_connection_t *conn,
		int answer_type,
		@@ -2341,7 +2347,9 @@ connection_exit_begin_conn(cell_t cell, circuit_t circ)
		address = tor_strdup(or_circ->p_conn->_base.address);
		else
		address = tor_strdup("127.0.0.1");
		port = 1; /XXXX020 set this to something sensible? - NM/
		port = 1; /* XXXX This value is never actually used anywhere, and there
		* isn't "really" a connection here. But we
		* need to set it to something nonzero. */
		} else {
		log_warn(LD_BUG, "Got an unexpected command %d", (int)rh.command);
		end_payload[0] = END_STREAM_REASON_INTERNAL;

src/or/connection_or.c

+1 −1

Original line number	Diff line number	Diff line
		@@ -404,7 +404,7 @@ connection_or_init_conn_from_address(or_connection_t *conn,
		/* Override the addr/port, so our log messages will make sense.
		* This is dangerous, since if we ever try looking up a conn by
		* its actual addr/port, we won't remember. Careful! */
		/* XXXX020 arma: this is stupid, and it's the reason we need real_addr
		/* XXXX021 arma: this is stupid, and it's the reason we need real_addr
		* to track is_canonical properly. What requires it? */
		/* XXXX <arma> i believe the reason we did this, originally, is because
		* we wanted to log what OR a connection was to, and if we logged the