[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.
nickm at torproject.org
nickm at torproject.org
Fri Sep 9 17:08:58 UTC 2011
commit 8bffcf5549bbe27b95bff0ef357174c7028c3149
Author: Zack Weinberg <zackw at panix.com>
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);
More information about the tor-commits
mailing list