commit 6e8c2a3e46e3578b0268d422b1753995d18fb734 Author: Nick Mathewson nickm@torproject.org Date: Tue Jan 17 16:35:07 2012 -0500
Use SOCKET_OK macros in even more places
Add a TOR_INVALID_SOCKET macro to wrap -1/INVALID_SOCKET.
Partial work for bug4533. --- changes/tor_socket_tests | 8 ++++++++ src/common/compat.c | 14 +++++++------- src/common/compat.h | 8 ++++++++ src/or/buffers.c | 4 ++-- src/or/connection.c | 16 ++++++++-------- src/or/cpuworker.c | 4 ++-- src/or/dns.c | 8 ++++---- 7 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/changes/tor_socket_tests b/changes/tor_socket_tests new file mode 100644 index 0000000..5857d20 --- /dev/null +++ b/changes/tor_socket_tests @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Find more places in the code that should have been testing for + invalid sockets using the SOCKET_OK macro. Required for a fix + for bug 4533. Bugfix on 0.2.2.28-beta. + + o Code simplification and refactoring: + - Use a TOR_INVALID_SOCKET macro when initializing a socket to an + invalid value, rather than just -1. diff --git a/src/common/compat.c b/src/common/compat.c index 3af43e8..7f89037 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1053,17 +1053,17 @@ tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]) r = socketpair(family, type, protocol, fd); if (r == 0) { #if !defined(SOCK_CLOEXEC) && defined(FD_CLOEXEC) - if (fd[0] >= 0) + if (SOCKET_OK(fd[0])) fcntl(fd[0], F_SETFD, FD_CLOEXEC); - if (fd[1] >= 0) + if (SOCKET_OK(fd[1])) fcntl(fd[1], F_SETFD, FD_CLOEXEC); #endif socket_accounting_lock(); - if (fd[0] >= 0) { + if (SOCKET_OK(fd[0])) { ++n_sockets_open; mark_socket_open(fd[0]); } - if (fd[1] >= 0) { + if (SOCKET_OK(fd[1])) { ++n_sockets_open; mark_socket_open(fd[1]); } @@ -1100,7 +1100,7 @@ tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]) }
listener = tor_open_socket(AF_INET, type, 0); - if (listener < 0) + if (!SOCKET_OK(listener)) return -tor_socket_errno(-1); memset(&listen_addr, 0, sizeof(listen_addr)); listen_addr.sin_family = AF_INET; @@ -1113,7 +1113,7 @@ tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]) goto tidy_up_and_fail;
connector = tor_open_socket(AF_INET, type, 0); - if (connector < 0) + if (!SOCKET_OK(connector)) goto tidy_up_and_fail; /* We want to find out the port number to connect to. */ size = sizeof(connect_addr); @@ -1128,7 +1128,7 @@ tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]) size = sizeof(listen_addr); acceptor = tor_accept_socket(listener, (struct sockaddr *) &listen_addr, &size); - if (acceptor < 0) + if (!SOCKET_OK(acceptor)) goto tidy_up_and_fail; if (size != sizeof(listen_addr)) goto abort_tidy_up_and_fail; diff --git a/src/common/compat.h b/src/common/compat.h index 2db8107..1b79565 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -399,11 +399,19 @@ typedef int socklen_t; #endif
#ifdef MS_WINDOWS +/* XXX Actually, this should arguably be SOCKET; we use intptr_t here so that + * any inadvertant checks for the socket being <= 0 or > 0 will probably + * still work. */ #define tor_socket_t intptr_t #define SOCKET_OK(s) ((unsigned)(s) != INVALID_SOCKET) +#define TOR_INVALID_SOCKET INVALID_SOCKET #else +/** Type used for a network socket. */ #define tor_socket_t int +/** Macro: true iff 's' is a possible value for a valid initialized socket. */ #define SOCKET_OK(s) ((s) >= 0) +/** Error/uninitialized value for a tor_socket_t. */ +#define TOR_INVALID_SOCKET (-1) #endif
int tor_close_socket(tor_socket_t s); diff --git a/src/or/buffers.c b/src/or/buffers.c index 7c49423..dbecf12 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -690,7 +690,7 @@ read_to_buf(tor_socket_t s, size_t at_most, buf_t *buf, int *reached_eof,
check(); tor_assert(reached_eof); - tor_assert(s >= 0); + tor_assert(SOCKET_OK(s));
while (at_most > total_read) { size_t readlen = at_most - total_read; @@ -858,7 +858,7 @@ flush_buf(tor_socket_t s, buf_t *buf, size_t sz, size_t *buf_flushlen) int r; size_t flushed = 0; tor_assert(buf_flushlen); - tor_assert(s >= 0); + tor_assert(SOCKET_OK(s)); tor_assert(*buf_flushlen <= buf->datalen); tor_assert(sz <= *buf_flushlen);
diff --git a/src/or/connection.c b/src/or/connection.c index 3e44516..14dfe0a 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -372,7 +372,7 @@ connection_init(time_t now, connection_t *conn, int type, int socket_family) break; }
- conn->s = -1; /* give it a default of 'not used' */ + conn->s = TOR_INVALID_SOCKET; /* give it a default of 'not used' */ conn->conn_array_index = -1; /* also default to 'not used' */ conn->global_identifier = n_connections_allocated++;
@@ -395,8 +395,8 @@ connection_init(time_t now, connection_t *conn, int type, int socket_family) void connection_link_connections(connection_t *conn_a, connection_t *conn_b) { - tor_assert(conn_a->s < 0); - tor_assert(conn_b->s < 0); + tor_assert(! SOCKET_OK(conn_a->s)); + tor_assert(! SOCKET_OK(conn_b->s));
conn_a->linked = 1; conn_b->linked = 1; @@ -540,7 +540,7 @@ _connection_free(connection_t *conn) if (SOCKET_OK(conn->s)) { log_debug(LD_NET,"closing fd %d.",(int)conn->s); tor_close_socket(conn->s); - conn->s = -1; + conn->s = TOR_INVALID_SOCKET; }
if (conn->type == CONN_TYPE_OR && @@ -625,7 +625,7 @@ connection_about_to_close_connection(connection_t *conn) /** Return true iff connection_close_immediate() has been called on this * connection. */ #define CONN_IS_CLOSED(c) \ - ((c)->linked ? ((c)->linked_conn_is_closed) : ((c)->s < 0)) + ((c)->linked ? ((c)->linked_conn_is_closed) : (! SOCKET_OK(c->s)))
/** Close the underlying socket for <b>conn</b>, so we don't try to * flush it. Must be used in conjunction with (right before) @@ -651,7 +651,7 @@ connection_close_immediate(connection_t *conn)
if (SOCKET_OK(conn->s)) tor_close_socket(conn->s); - conn->s = -1; + conn->s = TOR_INVALID_SOCKET; if (conn->linked) conn->linked_conn_is_closed = 1; if (conn->outbuf) @@ -958,7 +958,7 @@ connection_create_listener(const struct sockaddr *listensockaddr, goto err; } s = tor_open_socket(AF_UNIX, SOCK_STREAM, 0); - if (s < 0) { + if (! SOCKET_OK(s)) { log_warn(LD_NET,"Socket creation failed: %s.", strerror(errno)); goto err; } @@ -1331,7 +1331,7 @@ connection_connect(connection_t *conn, const char *address, }
s = tor_open_socket(protocol_family,SOCK_STREAM,IPPROTO_TCP); - if (s < 0) { + if (! SOCKET_OK(s)) { *socket_error = tor_socket_errno(-1); log_warn(LD_NET,"Error creating network socket: %s", tor_socket_strerror(*socket_error)); diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index e4460c5..ed31d35 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -329,8 +329,8 @@ spawn_cpuworker(void) return -1; }
- tor_assert(fdarray[0] >= 0); - tor_assert(fdarray[1] >= 0); + tor_assert(SOCKET_OK(fdarray[0])); + tor_assert(SOCKET_OK(fdarray[1]));
fd = fdarray[0]; spawn_func(cpuworker_main, (void*)fdarray); diff --git a/src/or/dns.c b/src/or/dns.c index beb110a..7a505b0 100644 --- a/src/or/dns.c +++ b/src/or/dns.c @@ -454,7 +454,7 @@ purge_expired_resolves(time_t now) pend = resolve->pending_connections; resolve->pending_connections = pend->next; /* Connections should only be pending if they have no socket. */ - tor_assert(pend->conn->_base.s == -1); + tor_assert(pend->conn->_base.s == TOR_INVALID_SOCKET); pendconn = pend->conn; connection_edge_end(pendconn, END_STREAM_REASON_TIMEOUT); circuit_detach_stream(circuit_get_by_edge_conn(pendconn), pendconn); @@ -681,7 +681,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, uint8_t is_reverse = 0; int r; assert_connection_ok(TO_CONN(exitconn), 0); - tor_assert(exitconn->_base.s == -1); + tor_assert(exitconn->_base.s == TOR_INVALID_SOCKET); assert_cache_ok(); tor_assert(oncirc);
@@ -849,7 +849,7 @@ assert_all_pending_dns_resolves_ok(void) pend; pend = pend->next) { assert_connection_ok(TO_CONN(pend->conn), 0); - tor_assert(pend->conn->_base.s == -1); + tor_assert(pend->conn->_base.s == TOR_INVALID_SOCKET); tor_assert(!connection_in_array(TO_CONN(pend->conn))); } } @@ -955,7 +955,7 @@ dns_cancel_pending_resolve(const char *address) pend->conn->_base.state = EXIT_CONN_STATE_RESOLVEFAILED; pendconn = pend->conn; assert_connection_ok(TO_CONN(pendconn), 0); - tor_assert(pendconn->_base.s == -1); + tor_assert(pendconn->_base.s == TOR_INVALID_SOCKET); if (!pendconn->_base.marked_for_close) { connection_edge_end(pendconn, END_STREAM_REASON_RESOLVEFAILED); }