commit ec5edc941ef13787f1baae0e741a05536d374571 Author: Zack Weinberg zackw@panix.com Date: Tue Jul 19 10:46:23 2011 -0700

Use smartlist instead of dll for the connections and listeners lists. --- src/main.c | 5 +-- src/network.c | 141 ++++++++++++++++++++++++++++----------------------------- src/network.h | 3 +- src/util.c | 112 --------------------------------------------- src/util.h | 49 +++++++------------- 5 files changed, 88 insertions(+), 222 deletions(-)

diff --git a/src/main.c b/src/main.c index a7aa298..cf811fa 100644 --- a/src/main.c +++ b/src/main.c @@ -372,10 +372,7 @@ main(int argc, const char **argv) log_info("Exiting.");

close_obfsproxy_logfile(); - - free_all_listeners(); /* free all listeners in our listener dll */ - - /* We are exiting. Clean everything. */ + free_all_listeners(); free(protocol_options); free(n_options_array); free(protocols); diff --git a/src/network.c b/src/network.c index 35e4eaf..4a85478 100644 --- a/src/network.c +++ b/src/network.c @@ -7,6 +7,7 @@ #define NETWORK_PRIVATE #include "network.h"

+#include "container.h" #include "main.h" #include "socks.h" #include "protocol.h" @@ -24,19 +25,16 @@ #include <ws2tcpip.h> /* socklen_t */ #endif

-/** Doubly linked list holding all our listeners. */ -static dll_t listener_list = DLL_INIT(); +/** All our listeners. */ +static smartlist_t *listeners;

struct listener_t { - dll_node_t dll_node; struct evconnlistener *listener; protocol_params_t *proto_params; };

-/** Doubly linked list holding all connections. */ -static dll_t conn_list = DLL_INIT(); -/** Active connection counter */ -static int n_connections=0; +/** All active connections. */ +static smartlist_t *connections;

/** Flag toggled when obfsproxy is shutting down. It blocks new connections and shutdowns when the last connection is closed. */ @@ -64,7 +62,7 @@ static void output_event_cb(struct bufferevent *bev, short what, void *arg);

If 'barbaric' is set, we forcefully close all open connections and finish shutdown. - + (Only called by signal handlers) */ void @@ -73,31 +71,32 @@ start_shutdown(int barbaric) if (!shutting_down) shutting_down=1;

- if (!n_connections) { - finish_shutdown(); - return; - } + if (barbaric) + close_all_connections();

- if (barbaric) { - if (n_connections) - close_all_connections(); - return; + if (connections && smartlist_len(connections) == 0) { + smartlist_free(connections); + connections = NULL; } -} + + if (!connections) + finish_shutdown(); +}

/** Closes all open connections. -*/ +*/ static void close_all_connections(void) { - /** Traverse the dll and close all connections */ - while (conn_list.head) { - conn_t *conn = DOWNCAST(conn_t, dll_node, conn_list.head); - conn_free(conn); /* removes it */ - } - obfs_assert(!n_connections); + if (!connections) + return; + SMARTLIST_FOREACH(connections, conn_t *, conn, + { conn_free(conn); }); + smartlist_free(connections); + connections = NULL; } + /** This function spawns a listener configured according to the provided 'protocol_params_t' object'. Returns the listener on @@ -107,7 +106,6 @@ close_all_connections(void) protocol_params_t object provided; if it fails, the protocol_params_t object is deallocated. */ - listener_t * listener_new(struct event_base *base, protocol_params_t *proto_params) @@ -127,12 +125,15 @@ listener_new(struct event_base *base,

if (!lsn->listener) { log_warn("Failed to create listener!"); - listener_free(lsn); + proto_params_free(lsn->proto_params); + free(lsn); return NULL; }

- /** If we don't have a connection dll, create one now. */ - dll_append(&listener_list, &lsn->dll_node); + /* If we don't have a listener list, create one now. */ + if (!listeners) + listeners = smartlist_create(); + smartlist_add(listeners, lsn);

return lsn; } @@ -140,17 +141,13 @@ listener_new(struct event_base *base, /** Deallocates listener_t 'lsn'. */ -void +static void listener_free(listener_t *lsn) { if (lsn->listener) evconnlistener_free(lsn->listener); if (lsn->proto_params) proto_params_free(lsn->proto_params); - - dll_remove(&listener_list, &lsn->dll_node); - - memset(lsn, 0xb0, sizeof(listener_t)); free(lsn); }

@@ -160,20 +157,14 @@ listener_free(listener_t *lsn) void free_all_listeners(void) { - static int called_already=0; - - if (called_already) + if (!listeners) return; - log_info("Closing all listeners.");

- /* Iterate listener doubly linked list and free them all. */ - while (listener_list.head) { - listener_t *listener = DOWNCAST(listener_t, dll_node, listener_list.head); - listener_free(listener); - } - - called_already++; + SMARTLIST_FOREACH(listeners, listener_t *, lsn, + { listener_free(lsn); }); + smartlist_free(listeners); + listeners = NULL; }

/** @@ -184,17 +175,13 @@ free_all_listeners(void) */ static void simple_listener_cb(struct evconnlistener *evcl, - evutil_socket_t fd, struct sockaddr *sourceaddr, + evutil_socket_t fd, struct sockaddr *sourceaddr, int socklen, void *arg) { listener_t *lsn = arg; struct event_base *base; conn_t *conn = xzalloc(sizeof(conn_t));

- n_connections++; /* If we call conn_free() later on error, it will decrement - * n_connections. Therefore, we had better increment it at - * the start. */ - log_debug("Got a connection attempt.");

conn->mode = lsn->proto_params->mode; @@ -266,14 +253,15 @@ simple_listener_cb(struct evconnlistener *evcl, bufferevent_enable(conn->output, EV_READ|EV_WRITE); }

- /* add conn to the linked list of connections */ - if (dll_append(&conn_list, &conn->dll_node)<0) - goto err; + /* add conn to the connection list */ + if (!connections) + connections = smartlist_create(); + smartlist_add(connections, conn);

log_debug("Connection setup completed. " - "We currently have %d connections!", n_connections); - + "We currently have %d connections!", smartlist_len(connections)); return; + err: if (conn) conn_free(conn); @@ -296,39 +284,48 @@ conn_free(conn_t *conn) if (conn->output) bufferevent_free(conn->output);

- /* remove conn from the linked list of connections */ - dll_remove(&conn_list, &conn->dll_node); - n_connections--; - memset(conn, 0x99, sizeof(conn_t)); free(conn); +}

- obfs_assert(n_connections>=0); +/** + Closes a fully open connection. +*/ +static void +close_conn(conn_t *conn) +{ + obfs_assert(connections); + smartlist_remove(connections, conn); + conn_free(conn); log_debug("Connection destroyed. " - "We currently have %d connections!", n_connections); + "We currently have %d connections!", smartlist_len(connections));

/** If this was the last connection AND we are shutting down, finish shutdown. */ - if (!n_connections && shutting_down) { - finish_shutdown(); + if (smartlist_len(connections) == 0) { + smartlist_free(connections); + connections = NULL; } + + if (!connections && shutting_down) + finish_shutdown(); }

/** Closes associated connection if the output evbuffer of 'bev' is empty. -*/ +*/ static void close_conn_on_flush(struct bufferevent *bev, void *arg) { conn_t *conn = arg;

- if (0 == evbuffer_get_length(bufferevent_get_output(bev))) - conn_free(conn); + if (evbuffer_get_length(bufferevent_get_output(bev)) == 0) + close_conn(conn); }

-/** - This callback is responsible for handling SOCKS traffic. +/** + This callback is responsible for handling SOCKS traffic. */ static void socks_read_cb(struct bufferevent *bev, void *arg) @@ -357,7 +354,7 @@ socks_read_cb(struct bufferevent *bev, void *arg)

if (r < 0) { /* XXXX send socks reply */ - conn_free(conn); + close_conn(conn); return; } bufferevent_disable(conn->input, EV_READ|EV_WRITE); @@ -372,7 +369,7 @@ socks_read_cb(struct bufferevent *bev, void *arg) if (socks_ret == SOCKS_INCOMPLETE) return; /* need to read more data. */ else if (socks_ret == SOCKS_BROKEN) - conn_free(conn); /* XXXX maybe send socks reply */ + close_conn(conn); /* XXXX maybe send socks reply */ else if (socks_ret == SOCKS_CMD_NOT_CONNECT) { bufferevent_enable(bev, EV_WRITE); bufferevent_disable(bev, EV_READ); @@ -398,7 +395,7 @@ plaintext_read_cb(struct bufferevent *bev, void *arg) if (proto_send(conn->proto, bufferevent_get_input(bev), bufferevent_get_output(other)) < 0) - conn_free(conn); + close_conn(conn); }

/** @@ -420,7 +417,7 @@ obfuscated_read_cb(struct bufferevent *bev, void *arg) bufferevent_get_output(other));

if (r == RECV_BAD) - conn_free(conn); + close_conn(conn); else if (r == RECV_SEND_PENDING) proto_send(conn->proto, bufferevent_get_input(conn->input), @@ -439,7 +436,7 @@ error_or_eof(conn_t *conn,

if (conn->flushing || ! conn->is_open || 0 == evbuffer_get_length(bufferevent_get_output(bev_flush))) { - conn_free(conn); + close_conn(conn); return; }

diff --git a/src/network.h b/src/network.h index e631697..7ba9afc 100644 --- a/src/network.h +++ b/src/network.h @@ -32,7 +32,6 @@ typedef struct listener_t listener_t;

listener_t *listener_new(struct event_base *base, struct protocol_params_t *params); -void listener_free(listener_t *listener); void free_all_listeners(void);

void start_shutdown(int barbaric); @@ -44,7 +43,6 @@ struct socks_state_t; struct protocol_t;

typedef struct conn_t { - dll_node_t dll_node; struct socks_state_t *socks_state; struct protocol_t *proto; /* ASN Do we like this here? We probably don't. But it's so convenient!! So convenient! */ @@ -54,6 +52,7 @@ typedef struct conn_t { unsigned int flushing : 1; unsigned int is_open : 1; } conn_t; + #endif

#endif diff --git a/src/util.c b/src/util.c index 9b9fa48..a4801a0 100644 --- a/src/util.c +++ b/src/util.c @@ -281,118 +281,6 @@ ascii_strlower(char *s) } }

-/************************ Doubly Linked List (DLL) ******************/ - -/** - Insert 'new_node' after 'node' in the doubly linked list 'list'. -*/ -static void -dll_insert_after(dll_t *list, dll_node_t *node, dll_node_t *new_node) -{ - obfs_assert(node); - obfs_assert(new_node); - - if (!list) - return; - - new_node->prev = node; - new_node->next = node->next; - if (!node->next) - list->tail = new_node; - else - node->next->prev = new_node; - node->next = new_node; -} - -/** - Insert 'new_node' before 'node' in the doubly linked list 'list'. -*/ -static void -dll_insert_before(dll_t *list, dll_node_t *node, dll_node_t *new_node) -{ - obfs_assert(node); - obfs_assert(new_node); - - if (!list) - return; - - new_node->prev = node->prev; - new_node->next = node; - if (!node->prev) - list->head = new_node; - else - node->prev->next = new_node; - node->prev = new_node; -} - -/** Initialize <b>list</b> as an empty list. */ -void -dll_init(dll_t *list) -{ - list->head = list->tail = NULL; -} - -/** - Insert 'node' in the beginning of the doubly linked 'list'. -*/ -static void -dll_insert_beginning(dll_t *list, dll_node_t *node) -{ - obfs_assert(node); - - if (!list) - return; - - if (!list->head) { - list->head = node; - list->tail = node; - node->prev = NULL; - node->next = NULL; - } else { - dll_insert_before(list, list->head, node); - } -} - -/** - Appends 'data' to the end of the doubly linked 'list'. - Returns 1 on success, -1 on fail. -*/ -int -dll_append(dll_t *list, dll_node_t *node) -{ - obfs_assert(list); - obfs_assert(node); - - if (!list->tail) - dll_insert_beginning(list, node); - else - dll_insert_after(list, list->tail, node); - - return 1; -} - -/** - Removes 'node' from the doubly linked list 'list'. - It frees the list node, but leaves its data intact. -*/ -void -dll_remove(dll_t *list, dll_node_t *node) -{ - obfs_assert(node); - - if (!list) - return; - - if (!node->prev) - list->head = node->next; - else - node->prev->next = node->next; - if (!node->next) - list->tail = node->prev; - else - node->next->prev = node->prev; -} - /************************ Logging Subsystem *************************/ /** The code of this section was to a great extent shamelessly copied off tor. It's basicaly a stripped down version of tor's logging diff --git a/src/util.h b/src/util.h index e44a711..a88d06c 100644 --- a/src/util.h +++ b/src/util.h @@ -6,10 +6,16 @@ #define UTIL_H

#include "config.h" -#include <stdarg.h> /* for va_list */ -#include <stddef.h> /* size_t, offsetof, NULL, etc */ +#include <stdarg.h> /* va_list */ +#include <stddef.h> /* size_t, ptrdiff_t, offsetof, NULL */ #include <stdint.h> /* intN_t, uintN_t */

+struct sockaddr; +struct event_base; +struct evdns_base; + +/***** Type annotations. *****/ + #ifndef __GNUC__ #define __attribute__(x) /* nothing */ #endif @@ -19,10 +25,6 @@ #define ATTR_PRINTF_3 __attribute__((format(printf, 3, 4))) #define ATTR_PURE __attribute__((pure))

-struct sockaddr; -struct event_base; -struct evdns_base; - /***** Memory allocation. *****/

/* Because this isn't Tor and functions named "tor_whatever" would be @@ -37,11 +39,16 @@ void *xmemdup(const void *ptr, size_t size) ATTR_MALLOC; char *xstrdup(const char *s) ATTR_MALLOC; char *xstrndup(const char *s, size_t maxsize) ATTR_MALLOC;

+/***** Pseudo-inheritance. *****/ + +#define DOWNCAST(container_type, element, ptr) \ + (container_type*)( ((char*)ptr) - offsetof(container_type, element) ) + /***** Math. *****/

unsigned int ui64_log2(uint64_t u64);

-/***** Network functions stuff. *****/ +/***** Network functions. *****/

int resolve_address_port(const char *address, int nodns, int passive, @@ -52,7 +59,7 @@ int resolve_address_port(const char *address, struct evdns_base *get_evdns_base(void); int init_evdns_base(struct event_base *base);

-/***** String functions stuff. *****/ +/***** String functions. *****/

static inline int ascii_isspace(unsigned char c) { @@ -72,31 +79,9 @@ int obfs_snprintf(char *str, size_t size, const char *format, ...) ATTR_PRINTF_3;

-/***** Doubly Linked List stuff. *****/ - -#define DOWNCAST(container_type, element, ptr) \ - (container_type*)( ((char*)ptr) - offsetof(container_type, element) ) - -/** A doubly linked list node. - [algorithms ripped off Wikipedia (Doubly_linked_list) ] */ -typedef struct dll_node_t { - struct dll_node_t *next, *prev; -} dll_node_t; - -/** A doubly linked list. */ -typedef struct dll_t { - struct dll_node_t *head; - struct dll_node_t *tail; -} dll_t; - -void dll_init(dll_t *list); -int dll_append(dll_t *list, dll_node_t *node); -void dll_remove(dll_t *list, dll_node_t *node); -#define DLL_INIT() { NULL, NULL } - -/***** Logging subsystem stuff. *****/ +/***** Logging. *****/

-/** Logging methods */ +/** Log destinations */

/** Spit log messages on stdout. */ #define LOG_METHOD_STDOUT 1