[tor-commits] [obfsproxy/master] Use smartlist instead of dll for the connections and listeners lists.

nickm at torproject.org nickm at torproject.org
Fri Sep 9 17:08:56 UTC 2011


commit ec5edc941ef13787f1baae0e741a05536d374571
Author: Zack Weinberg <zackw at 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





More information about the tor-commits mailing list