SOCKS: various fixes to API and code

3 files changed, 43 insertions, 26 deletions

diff --git a/common.c b/common.c
index de00445..10db329 100644
--- a/common.c
+++ b/common.c
@@ -399,11 +399,11 @@ struct socks_connector
 
 	// Configuration:
 
-	const char *hostname;               ///< SOCKS server hostname
-	const char *service;                ///< SOCKS server service name or port
+	char *hostname;                     ///< SOCKS server hostname
+	char *service;                      ///< SOCKS server service name or port
 
-	const char *username;               ///< Username for authentication
-	const char *password;               ///< Password for authentication
+	char *username;                     ///< Username for authentication
+	char *password;                     ///< Password for authentication
 
 	struct socks_target *targets;       ///< Targets
 	struct socks_target *targets_tail;  ///< Tail of targets
@@ -835,12 +835,15 @@ socks_connector_on_error (void *user_data, const char *error)
 {
 	struct socks_connector *self = user_data;
 	// TODO: skip protocol on protocol failure
-	self->on_error (self->user_data, error);
+	if (self->on_error)
+		self->on_error (self->user_data, error);
 }
 
 static void
 socks_connector_start (struct socks_connector *self)
 {
+	hard_assert (!self->connector);
+
 	struct connector *connector =
 		self->connector = xcalloc (1, sizeof *connector);
 	connector_init (connector, self->socket_event.poller);
@@ -851,9 +854,17 @@ socks_connector_start (struct socks_connector *self)
 	connector->on_error      = socks_connector_on_error;
 	connector->on_failure    = socks_connector_on_failure;
 
-	// TODO: let's rather call on_error and on_failure instead on error
-	hard_assert (connector_add_target (connector,
-		self->hostname, self->service, NULL));
+	struct error *e = NULL;
+	if (!connector_add_target (connector, self->hostname, self->service, &e))
+	{
+		if (self->on_error)
+			self->on_error (self->user_data, e->message);
+		error_free (e);
+
+		socks_connector_destroy_connector (self);
+		socks_connector_fail (self);
+		return;
+	}
 
 	poller_timer_set (&self->timeout, 60 * 1000);
 	connector_step (connector);
@@ -926,7 +937,6 @@ socks_try_flush_write_buffer (struct socks_connector *self)
 			continue;
 
 		SOCKS_FAIL ("%s: %s", "send", strerror (errno));
-		return false;
 	}
 	return true;
 }
@@ -987,6 +997,11 @@ socks_connector_free (struct socks_connector *self)
 	str_free (&self->read_buffer);
 	str_free (&self->write_buffer);
 
+	free (self->hostname);
+	free (self->service);
+	free (self->username);
+	free (self->password);
+
 	LIST_FOR_EACH (struct socks_target, iter, self->targets)
 	{
 		socks_addr_free (&iter->address);
@@ -1027,14 +1042,22 @@ socks_connector_add_target (struct socks_connector *self,
 }
 
 static void
-socks_connector_run (struct socks_connector *self)
+socks_connector_run (struct socks_connector *self,
+	const char *host, const char *service,
+	const char *username, const char *password)
 {
-	// XXX: do we need some better error checking in here?
-	hard_assert (self->hostname);
 	hard_assert (self->targets);
+	hard_assert (host && service);
+
+	self->hostname = xstrdup (host);
+	self->service  = xstrdup (service);
+
+	if (username)  self->username = xstrdup (username);
+	if (password)  self->password = xstrdup (password);
 
 	self->targets_iter = self->targets;
 	self->protocol_iter = 0;
+	// XXX: this can fail immediately from an error creating the connector
 	socks_connector_start (self);
 }
 
diff --git a/degesch.c b/degesch.c
index d1fc783..a8d7151 100644
--- a/degesch.c
+++ b/degesch.c
@@ -4472,13 +4472,6 @@ irc_setup_connector_socks (struct server *s,
 	socks_connector_init (connector, &s->ctx->poller);
 	s->socks_conn = connector;
 
-	// FIXME: the SOCKS connector may outlive these values
-	connector->hostname = socks_host;
-	// FIXME: memory leak
-	connector->service  = xstrdup_printf ("%" PRIi64, socks_port_int);
-	connector->username = get_config_string (s->config, "socks_username");
-	connector->password = get_config_string (s->config, "socks_password");
-
 	connector->user_data     = s;
 	connector->on_connecting = irc_on_socks_connecting;
 	connector->on_error      = irc_on_connector_error;
@@ -4494,7 +4487,11 @@ irc_setup_connector_socks (struct server *s,
 			return false;
 	}
 
-	socks_connector_run (connector);
+	char *service = xstrdup_printf ("%" PRIi64, socks_port_int);
+	socks_connector_run (connector, socks_host, service,
+		get_config_string (s->config, "socks_username"),
+		get_config_string (s->config, "socks_password"));
+	free (service);
 	return true;
 }
 
diff --git a/zyklonb.c b/zyklonb.c
index 991ef13..2efc794 100644
--- a/zyklonb.c
+++ b/zyklonb.c
@@ -1708,11 +1708,6 @@ irc_establish_connection_socks (struct bot_context *ctx,
 	socks_connector_init (connector, poller);
 	data.succeeded = false;
 
-	connector->hostname = socks_host;
-	connector->service  = socks_port;
-	connector->username = str_map_find (&ctx->config, "socks_username");
-	connector->password = str_map_find (&ctx->config, "socks_password");
-
 	connector->on_connected  = irc_on_socks_connected;
 	connector->on_connecting = irc_on_socks_connecting;
 	connector->on_error      = irc_on_socks_error;
@@ -1721,7 +1716,9 @@ irc_establish_connection_socks (struct bot_context *ctx,
 
 	if (socks_connector_add_target (connector, host, service, e))
 	{
-		socks_connector_run (connector);
+		socks_connector_run (connector, socks_host, socks_port,
+			str_map_find (&ctx->config, "socks_username"),
+			str_map_find (&ctx->config, "socks_password"));
 		while (data.polling)
 			poller_run (poller);
 	}