[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [obfsproxy/master] Take a chain saw to the overcomplicated argument segmenting code in main.c. Push the call to proto_params_init inside the call to create_listener.



commit 8bffcf5549bbe27b95bff0ef357174c7028c3149
Author: Zack Weinberg <zackw@xxxxxxxxx>
Date:   Mon Aug 1 16:50:21 2011 -0700

    Take a chain saw to the overcomplicated argument segmenting code in main.c. Push the call to proto_params_init inside the call to create_listener.
---
 src/main.c            |  289 ++++++++++++++++++-------------------------------
 src/network.c         |   15 ++--
 src/network.h         |    2 +-
 src/test/tester.py.in |    4 +-
 src/util.c            |    9 ++
 src/util.h            |    3 +
 6 files changed, 127 insertions(+), 195 deletions(-)

diff --git a/src/main.c b/src/main.c
index 1459af3..14125f8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -4,6 +4,7 @@
 
 #include "util.h"
 
+#include "container.h"
 #include "crypt.h"
 #include "network.h"
 #include "protocol.h"
@@ -15,27 +16,18 @@
 #include <event2/event.h>
 #include <event2/dns.h>
 
-/* The character that seperates multiple listeners in the cli */
-#define SEPARATOR "+"
-/* Totally arbitrary. */
-#define MAXPROTOCOLS 20
-
-static void usage(void) ATTR_NORETURN;
-static int handle_obfsproxy_args(const char **argv);
-
-static struct event_base *the_event_base=NULL;
+static struct event_base *the_event_base;
 
 /**
    Prints the obfsproxy usage instructions then exits.
 */
-static void
+static void ATTR_NORETURN
 usage(void)
 {
   int i;
-  fprintf(stderr,
-          "Usage: obfsproxy protocol_name [protocol_args] protocol_options %s protocol_name ...\n"
-          "* Available protocols:\n",
-          SEPARATOR);
+  fputs("Usage: obfsproxy protocol_name [protocol_args] protocol_options "
+        "protocol_name ...\n"
+        "* Available protocols:\n", stderr);
   /* this is awful. */
   for (i=0;i<n_supported_protocols;i++)
     fprintf(stderr,"[%s] ", supported_protocols[i]->name);
@@ -83,11 +75,7 @@ handle_signal_cb(evutil_socket_t fd, short what, void *arg)
   }
 }
 
-/**
-   Stops obfsproxy's event loop.
-
-   Final cleanup happens in main().
-*/
+/** Stop obfsproxy's event loop. Final cleanup happens in main(). */
 void
 finish_shutdown(void)
 {
@@ -95,36 +83,21 @@ finish_shutdown(void)
   event_base_loopexit(the_event_base, NULL);
 }
 
-/**
-   This function visits 'n_options' command line arguments off 'argv'
-   and writes them in 'options_string'.
-*/
-static void
-populate_options(char **options_string,
-                 const char **argv, int n_options)
+/** Return 1 if 'name' is the name of a supported protocol, otherwise 0. */
+static int
+is_supported_protocol(const char *name)
 {
-  int g;
-  for (g=0;g<=n_options-1;g++)
-    options_string[g] = (char*) argv[g];
-}
+  int i;
+  for (i = 0; i < n_supported_protocols; i++)
+    if (!strcmp(name, supported_protocols[i]->name))
+      return 1;
 
-/**
-   Return 0 if 'name' is the name of a supported protocol, otherwise
-   return -1.
-*/
-static int
-is_supported_protocol(const char *name) {
-  int f;
-  for (f=0;f<n_supported_protocols;f++) {
-    if (!strcmp(name,supported_protocols[f]->name))
-      return 0;
-  }
-  return -1;
+  return 0;
 }
 
 /**
-   Receives argv[1] as 'argv' and scans from thereafter for any
-   obfsproxy optional arguments and tries to set them in effect.
+   Receives 'argv' and scans for any obfsproxy optional arguments and
+   tries to set them in effect.
 
    If it succeeds it returns the number of argv arguments its caller
    should skip to get past the optional arguments we already handled.
@@ -135,7 +108,7 @@ handle_obfsproxy_args(const char **argv)
 {
   int logmethod_set=0;
   int logsev_set=0;
-  int i=0;
+  int i=1;
 
   while (argv[i] &&
          !strncmp(argv[i],"--",2)) {
@@ -186,121 +159,81 @@ main(int argc, const char **argv)
   struct event *sig_int;
   struct event *sig_term;
 
-  /* Yes, these are three stars right there. This is an array of
-     arrays of strings! Every element of the array is an array of
-     strings that contains all the options of a protocol.
-     At runtime it should look like this:
-     char protocol_options[<number of protocols>][<number of options>][<length of option>]
-  */
-  char ***protocol_options = NULL;
-  /* This is an array of integers! Each integer is the number of
-     options of the respective protocol. */
-  int *n_options_array = NULL;
-  /* This is an integer! It contains the number of protocols that we
-     managed to recognize, by their protocol name.  Of course it's not
-     the *actual* actual_protocols since some of them could have wrong
-     options or arguments, but this will be resolved per-protocol by
-     proto_params_init(). */
-  int actual_protocols=0;
-
-  int start;
-  int end;
-  int n_options;
-  int i;
+  /* Array of argument counts, one per listener. */
+  int *listener_argcs = NULL;
 
-  /* The number of protocols. */
-  unsigned int n_protocols=1;
-  /* An array which holds the position in argv of the command line
-     options for each protocol. */
-  unsigned int *protocols=NULL;
-  /* keeps track of allocated space for the protocols array */
-  unsigned int n_alloc;
+  /* Array of pointers into argv. Each points to the beginning of a
+     sequence of options for a particular listener. */
+  const char *const **listener_argvs = NULL;
 
-  if (argc < 2) {
-    usage();
-  }
+  /* Total number of listeners requested on the command line. */
+  unsigned int n_listeners;
 
-  /** "Points" to the first argv string after the optional obfsproxy
-      arguments. Normally this should be where the protocols start. */
-  int start_of_protocols;
-  /** Handle optional obfsproxy arguments. */
-  start_of_protocols = handle_obfsproxy_args(&argv[1]);
-
-  protocols = xzalloc((n_protocols + 1) * sizeof(int));
-  n_alloc = n_protocols+1;
-
-  /* Populate protocols and calculate n_protocols. */
-  for (i=start_of_protocols;i<argc;i++) {
-    if (!strcmp(argv[i],SEPARATOR)) {
-      protocols[n_protocols] = i;
-      n_protocols++;
-
-      /* Do we need to expand the protocols array? */
-      if (n_alloc <= n_protocols) {
-        n_alloc *= 2;
-        protocols = xrealloc(protocols, sizeof(int)*(n_alloc));
-      }
-    }
-  }
+  /* Total number of listeners successfully created. */
+  unsigned int n_good_listeners;
 
-  /* protocols[0] points right before the first option of the first
-     protocol. */
-  protocols[0] = start_of_protocols;
-  /* protocols[n_protocols] points right after the last command line
-     option. */
-  protocols[n_protocols] = argc;
-
-  log_debug("Found %d protocol(s).", n_protocols);
-
-  /* We now allocate enough space for our parsing adventures.
-     We first allocate space for n_protocols pointers in protocol_options,
-     that point to arrays carrying the options of the protocols.
-     Finally, we allocate enough space on the n_options_array so that
-     we can put the number of options there.
-  */
-  protocol_options = xzalloc(n_protocols * sizeof(char**));
-  n_options_array = xzalloc(n_protocols * sizeof(int));
-
-  /* Iterate through protocols. */
-  for (i=0;i<n_protocols;i++) {
-    log_debug("Parsing protocol %d.", i+1);
-    /* This "points" to the first argument of this protocol in argv. */
-    start = protocols[i]+1;
-    /* This "points" to the last argument of this protocol in argv. */
-    end = protocols[i+1]-1;
-    /* This is the number of options of this protocol. */
-    n_options = end-start+1;
-
-    if (start >= end) {
-      log_warn("No protocol options were given on protocol %d.", i+1);
-      continue;
-    }
+  /* Index of the first argv string after the optional obfsproxy
+      arguments. Normally this should be where the listeners start. */
+  int start_of_listeners;
 
-    /* First option should be protocol_name. See if we support it. */
-    if (is_supported_protocol(argv[start])<0) {
-      log_warn("We don't support protocol: %s", argv[start]);
-      continue;
-    }
+  int cl, i;
+
+  /* Handle optional obfsproxy arguments. */
+  start_of_listeners = handle_obfsproxy_args(argv);
 
-    actual_protocols++;
+  if (!is_supported_protocol(argv[start_of_listeners]))
+    usage();
 
-    /* Allocate space for the array carrying the options of this
-       protocol. */
-    protocol_options[actual_protocols-1] = xzalloc(n_options * sizeof(char*));
+  /* Count number of listeners and allocate space for the listener-
+     argument arrays. We already know there's at least one. */
+  n_listeners = 1;
+  for (i = start_of_listeners+1; i < argc; i++)
+    if (is_supported_protocol(argv[i]))
+      n_listeners++;
 
-    /* Write the number of options to the correct place in n_options_array[]. */
-    n_options_array[actual_protocols-1] = n_options;
+  log_debug("%d listener%s on command line.",
+            n_listeners, n_listeners == 1 ? "" : "s");
+  listener_argcs = xzalloc(n_listeners * sizeof(int));
+  listener_argvs = xzalloc(n_listeners * sizeof(char **));
+
+  /* Each listener's argument vector consists of the entries in argv
+     from its recognized protocol name, up to but not including
+     the next recognized protocol name. */
+  cl = 1;
+  listener_argvs[0] = &argv[start_of_listeners];
+  for (i = start_of_listeners + 1; i < argc; i++)
+    if (is_supported_protocol(argv[i])) {
+      listener_argcs[cl-1] = i - (listener_argvs[cl-1] - argv);
+      if (listener_argcs[cl-1] == 1)
+        log_warn("No arguments to listener %d", cl);
+
+      listener_argvs[cl] = &argv[i];
+      cl++;
+    }
 
-    /* Finally! Let's fill protocol_options. */
-    populate_options(protocol_options[actual_protocols-1],
-                     &argv[start], n_options);
+  listener_argcs[cl-1] = argc - (listener_argvs[cl-1] - argv);
+  if (listener_argcs[cl-1] == 1)
+    log_warn("No arguments to listener %d", cl);
+
+  obfs_assert(cl == n_listeners);
+
+  if (log_do_debug()) {
+    smartlist_t *s = smartlist_create();
+    char *joined;
+    for (cl = 0; cl < n_listeners; cl++) {
+      smartlist_clear(s);
+      for (i = 0; i < listener_argcs[cl]; i++)
+        smartlist_add(s, (void *)listener_argvs[cl][i]);
+      joined = smartlist_join_strings(s, " ", 0, NULL);
+      log_debug("Listener %d: %s", cl+1, joined);
+    }
+    smartlist_free(s);
   }
 
-  /* Excellent. Now we should have protocol_options populated with all
-     the protocol options we got from the user. */
+  /* argv has been chunked; proceed with initialization. */
 
-  /*  Ugly method to fix a Windows problem:
-      http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */
+  /* Ugly method to fix a Windows problem:
+     http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */
 #ifdef _WIN32
   WSADATA wsaData;
   WSAStartup(0x101, &wsaData);
@@ -308,24 +241,21 @@ main(int argc, const char **argv)
 
   /* Initialize crypto */
   if (initialize_crypto() < 0) {
-    log_warn("Can't initialize crypto; failing");
-    return 1;
+    log_error("Failed to initialize cryptography.");
   }
 
   /* Initialize libevent */
   the_event_base = event_base_new();
   if (!the_event_base) {
-    log_warn("Can't initialize Libevent; failing");
-    return 1;
+    log_error("Failed to initialize networking.");
   }
 
   /* ASN should this happen only when SOCKS is enabled? */
-  if (init_evdns_base(the_event_base) < 0) {
-    log_warn("Can't initialize evdns; failing");
-    return 1;
+  if (init_evdns_base(the_event_base)) {
+    log_error("Failed to initialize DNS resolver.");
   }
 
-  /* Handle signals */
+  /* Handle signals. */
 #ifdef SIGPIPE
    signal(SIGPIPE, SIG_IGN);
 #endif
@@ -334,37 +264,27 @@ main(int argc, const char **argv)
   sig_term = evsignal_new(the_event_base, SIGTERM,
                           handle_signal_cb, NULL);
   if (event_add(sig_int,NULL) || event_add(sig_term,NULL)) {
-    log_warn("We can't even add events for signals! Exiting.");
+    log_error("Failed to initialize signal handling.");
     return 1;
   }
 
-  /*Let's open a new listener for each protocol. */
-  int h;
-  int n_listeners=0;
-  protocol_params_t *proto_params=NULL;
-  for (h=0;h<actual_protocols;h++) {
-    log_debug("Spawning listener %d!", h+1);
-
-    /** normally free'd in listener_free() */
-    proto_params = proto_params_init(n_options_array[h],
-                                     (const char *const *)protocol_options[h]);
-    if (proto_params && create_listener(the_event_base, proto_params)) {
-      log_info("Succesfully created listener %d.", h+1);
-      n_listeners++;
-    }
-
-    /** Free the space allocated for this protocol's options. */
-    free(protocol_options[h]);
-  }
+  /* Open a new listener for each protocol. */
+  n_good_listeners = 0;
+  for (cl = 0; cl < n_listeners; cl++)
+    if (create_listener(the_event_base,
+                        listener_argcs[cl], listener_argvs[cl]))
+      n_good_listeners++;
 
-  log_debug("From the original %d protocols only %d "
-            "were parsed from main.c. In the end only "
-            "%d survived.",
-            n_protocols, actual_protocols,n_listeners);
+  /* If the number of usable listeners is not equal to the complete
+     set specified on the command line, we have a usage error.
+     Diagnostics have already been issued.  */
+  log_debug("%d recognized listener%s on command line, %d with valid config",
+            n_listeners, n_listeners == 1 ? "" : "s", n_good_listeners);
+  if (n_listeners != n_good_listeners)
+    return 2;
 
-  /* run the event loop if at least one listener was created. */
-  if (n_listeners)
-    event_base_dispatch(the_event_base);
+  /* We are go for launch. */
+  event_base_dispatch(the_event_base);
 
   log_info("Exiting.");
 
@@ -377,9 +297,8 @@ main(int argc, const char **argv)
   cleanup_crypto();
 
   close_obfsproxy_logfile();
-  free(protocol_options);
-  free(n_options_array);
-  free(protocols);
+  free(listener_argvs);
+  free(listener_argcs);
 
   return 0;
 }
diff --git a/src/network.c b/src/network.c
index c9eafce..c327403 100644
--- a/src/network.c
+++ b/src/network.c
@@ -126,18 +126,19 @@ close_all_connections(void)
 
 /**
    This function spawns a listener configured according to the
-   provided 'protocol_params_t' object'.  Returns 1 on success, 0 on
-   failure.  (No, you can't have the listener object. It's private.)
-
-   Regardless of success or failure, the protocol_params_t is consumed.
+   provided argument subvector.  Returns 1 on success, 0 on failure.
+   (No, you can't have the listener object. It's private.)
 */
 int
-create_listener(struct event_base *base, protocol_params_t *params)
+create_listener(struct event_base *base, int argc, const char *const *argv)
 {
   const unsigned flags =
     LEV_OPT_CLOSE_ON_FREE|LEV_OPT_CLOSE_ON_EXEC|LEV_OPT_REUSEABLE;
   evconnlistener_cb callback;
-  listener_t *lsn = xzalloc(sizeof(listener_t));
+  listener_t *lsn;
+  protocol_params_t *params = proto_params_init(argc, argv);
+  if (!params)
+    return 0;
 
   switch (params->mode) {
   case LSN_SIMPLE_CLIENT: callback = simple_client_listener_cb; break;
@@ -145,7 +146,7 @@ create_listener(struct event_base *base, protocol_params_t *params)
   case LSN_SOCKS_CLIENT:  callback = socks_client_listener_cb;  break;
   default: obfs_abort();
   }
-
+  lsn = xzalloc(sizeof(listener_t));
   lsn->address = printable_address(params->listen_addr->ai_addr,
                                    params->listen_addr->ai_addrlen);
   lsn->proto_params = params;
diff --git a/src/network.h b/src/network.h
index 4ae96fc..54684a6 100644
--- a/src/network.h
+++ b/src/network.h
@@ -6,7 +6,7 @@
 #define NETWORK_H
 
 /* returns 1 on success, 0 on failure */
-int create_listener(struct event_base *base, protocol_params_t *params);
+int create_listener(struct event_base *base, int argc, const char *const *argv);
 void free_all_listeners(void);
 
 void start_shutdown(int barbaric);
diff --git a/src/test/tester.py.in b/src/test/tester.py.in
index 683724d..55d9477 100644
--- a/src/test/tester.py.in
+++ b/src/test/tester.py.in
@@ -356,7 +356,7 @@ class SocksBad(SocksTest, unittest.TestCase):
 #    obfs_args = ("obfs2",
 #                 "--dest=127.0.0.1:%d" % EXIT_PORT,
 #                 "server", "127.0.0.1:%d" % SERVER_PORT,
-#                 "+", "obfs2",
+#                 "obfs2",
 #                 "--dest=127.0.0.1:%d" % SERVER_PORT,
 #                 "client", "127.0.0.1:%d" % ENTRY_PORT)
 
@@ -364,7 +364,7 @@ class DirectDummy(DirectTest, unittest.TestCase):
     obfs_args = ("dummy", "server",
                  "127.0.0.1:%d" % SERVER_PORT,
                  "127.0.0.1:%d" % EXIT_PORT,
-                 "+", "dummy", "client",
+                 "dummy", "client",
                  "127.0.0.1:%d" % ENTRY_PORT,
                  "127.0.0.1:%d" % SERVER_PORT)
 
diff --git a/src/util.c b/src/util.c
index fa1a62c..3769aba 100644
--- a/src/util.c
+++ b/src/util.c
@@ -472,6 +472,15 @@ log_set_min_severity(const char* sev_string)
   return 0;
 }
 
+/** True if the minimum log severity is "debug".  Used in a few places
+    to avoid some expensive formatting work if we are going to ignore the
+    result. */
+int
+log_do_debug(void)
+{
+  return logging_min_sev == LOG_SEV_DEBUG;
+}
+
 /**
     Logging worker function.
     Accepts a logging 'severity' and a 'format' string and logs the
diff --git a/src/util.h b/src/util.h
index 5108b62..d2bbea9 100644
--- a/src/util.h
+++ b/src/util.h
@@ -138,6 +138,9 @@ int log_set_method(int method, const char *filename);
     'sev_string' may be "warn", "info", or "debug" (case-insensitively). */
 int log_set_min_severity(const char* sev_string);
 
+/** True if debug messages are being logged. */
+int log_do_debug(void);
+
 /** Close the logfile if it's open.  Ignores errors. */
 void close_obfsproxy_logfile(void);
 



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits