[svn-commits] dvossel: branch dvossel/sip_nonblocking_tcp_client r225237 - in /team/dvossel...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Wed Oct 21 13:23:32 CDT 2009


Author: dvossel
Date: Wed Oct 21 13:23:27 2009
New Revision: 225237

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=225237
Log:
Addresses various reviewboard comments

Modified:
    team/dvossel/sip_nonblocking_tcp_client/apps/app_externalivr.c
    team/dvossel/sip_nonblocking_tcp_client/channels/chan_sip.c
    team/dvossel/sip_nonblocking_tcp_client/include/asterisk/tcptls.h
    team/dvossel/sip_nonblocking_tcp_client/main/tcptls.c

Modified: team/dvossel/sip_nonblocking_tcp_client/apps/app_externalivr.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/sip_nonblocking_tcp_client/apps/app_externalivr.c?view=diff&rev=225237&r1=225236&r2=225237
==============================================================================
--- team/dvossel/sip_nonblocking_tcp_client/apps/app_externalivr.c (original)
+++ team/dvossel/sip_nonblocking_tcp_client/apps/app_externalivr.c Wed Oct 21 13:23:27 2009
@@ -457,10 +457,7 @@
 		ivr_desc.local_address.sin_family = AF_INET;
 		ivr_desc.local_address.sin_port = htons(port);
 		memcpy(&ivr_desc.local_address.sin_addr.s_addr, hp.hp.h_addr, hp.hp.h_length);
-		if ((ser = ast_tcptls_client_create(&ivr_desc))) {
-			ser = ast_tcptls_client_start(ser);
-		}
-		if (!ser) {
+		if (!(ser = ast_tcptls_client_create(&ivr_desc)) || !(ser = ast_tcptls_client_start(ser))) {
 			goto exit;
 		}
 		res = eivr_comm(chan, u, ser->fd, ser->fd, -1, pipe_delim_args, flags);

Modified: team/dvossel/sip_nonblocking_tcp_client/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/sip_nonblocking_tcp_client/channels/chan_sip.c?view=diff&rev=225237&r1=225236&r2=225237
==============================================================================
--- team/dvossel/sip_nonblocking_tcp_client/channels/chan_sip.c (original)
+++ team/dvossel/sip_nonblocking_tcp_client/channels/chan_sip.c Wed Oct 21 13:23:27 2009
@@ -2121,12 +2121,12 @@
 struct tcptls_packet {
 	AST_LIST_ENTRY(tcptls_packet) entry;
 	struct ast_str *data;
-	int len;
+	size_t len;
 };
 /*! \brief Definition of a thread that handles a socket */
 struct sip_threadinfo {
 	int stop;
-	int alert_pipe[2]; /*! Used to alert tcptls thread when packet is ready to write */
+	int alert_pipe[2]; /*! Used to alert tcptls thread when packet is ready to be written */
 	pthread_t threadid;
 	struct ast_tcptls_session_instance *tcptls_session;
 	enum sip_transport type;	/*!< We keep a copy of the type here so we can display it in the connection list */
@@ -2165,7 +2165,7 @@
 #endif
 
 /*! \brief  The table of TCP threads */
-static struct ao2_container *threadl;
+static struct ao2_container *threadt;
 
 /*! \brief  The peer list: Users, Peers and Friends */
 static struct ao2_container *peers;
@@ -2258,14 +2258,14 @@
 }
 
 
-static int threadl_hash_cb(const void *obj, const int flags)
+static int threadt_hash_cb(const void *obj, const int flags)
 {
 	const struct sip_threadinfo *th = obj;
 
-	return abs((int) th->tcptls_session->remote_address.sin_addr.s_addr);
-}
-
-static int threadl_cmp_cb(void *obj, void *arg, int flags)
+	return (int) th->tcptls_session->remote_address.sin_addr.s_addr;
+}
+
+static int threadt_cmp_cb(void *obj, void *arg, int flags)
 {
 	struct sip_threadinfo *th = obj, *th2 = arg;
 
@@ -2922,21 +2922,21 @@
 {
 	struct tcptls_packet *packet = obj;
 
-	if (packet->data) {
-		ast_free(packet->data);
-	}
+	ast_free(packet->data);
 }
 
 static void sip_tcptls_client_args_destructor(void *obj)
 {
 	struct ast_tcptls_session_args *args = obj;
-
 	if (args->tls_cfg) {
-		ast_free(args->tls_cfg);
-	}
-	if (!ast_strlen_zero(args->name)) {
-		ast_free((char *) args->name);
-	}
+		ast_free(args->tls_cfg->certfile);
+		ast_free(args->tls_cfg->pvtfile);
+		ast_free(args->tls_cfg->cipher);
+		ast_free(args->tls_cfg->cafile);
+		ast_free(args->tls_cfg->capath);
+	}
+	ast_free(args->tls_cfg);
+	ast_free((char *) args->name);
 }
 
 static void sip_threadinfo_destructor(void *obj)
@@ -2960,7 +2960,7 @@
 	}
 }
 
-/*! brief\ creates a sip_threadinfo object and links it into the threadl table. */
+/*! \brief creates a sip_threadinfo object and links it into the threadt table. */
 static struct sip_threadinfo *sip_threadinfo_create(struct ast_tcptls_session_instance *tcptls_session, int transport)
 {
 	struct sip_threadinfo *th;
@@ -2973,17 +2973,18 @@
 
 	if (pipe(th->alert_pipe) == -1) {
 		ao2_t_ref(th, -1, "Failed to open alert pipe on sip_threadinfo");
+		ast_log(LOG_ERROR, "Could not create sip alert pipe in tcptls thread, error %s\n", strerror(errno));
 		return NULL;
 	}
 	ao2_t_ref(tcptls_session, +1, "tcptls_session ref for sip_threadinfo object");
 	th->tcptls_session = tcptls_session;
 	th->type = transport ? transport : (tcptls_session->ssl ? SIP_TRANSPORT_TLS: SIP_TRANSPORT_TCP);
-	ao2_t_link(threadl, th, "Adding new tcptls helper thread");
+	ao2_t_link(threadt, th, "Adding new tcptls helper thread");
 	ao2_t_ref(th, -1, "Decrementing threadinfo ref from alloc, only table ref remains");
 	return th;
 }
 
-/*! brief\ used to indicate to a tcptls thread that data is ready to be written */
+/*! \brief used to indicate to a tcptls thread that data is ready to be written */
 static int sip_tcptls_write(struct ast_tcptls_session_instance *tcptls_session, const void *buf, size_t len)
 {
 	int res = len;
@@ -3000,16 +3001,10 @@
 
 	ast_mutex_lock(&tcptls_session->lock);
 
-	if (tcptls_session->fd == -1) {
-		goto tcptls_write_setup_error;
-	}
-	if (!(th = ao2_t_find(threadl, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread"))) {
-		goto tcptls_write_setup_error;
-	}
-	if (!(packet = ao2_alloc(sizeof(*packet), tcptls_packet_destructor))) {
-		goto tcptls_write_setup_error;
-	}
-	if (!(packet->data = ast_str_create(len))) {
+	if ((tcptls_session->fd == -1) ||
+		!(th = ao2_t_find(threadt, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread")) ||
+		!(packet = ao2_alloc(sizeof(*packet), tcptls_packet_destructor)) ||
+		!(packet->data = ast_str_create(len))) {
 		goto tcptls_write_setup_error;
 	}
 
@@ -3018,20 +3013,19 @@
 	packet->len = len;
 
 	/* alert tcptls thread handler that there is a packet to be sent.
-	 * must lock the thread info object to guarantee control of
-	 * alert_pipe fd and the packet queue */
+	 * must lock the thread info object to guarantee control of the
+	 * packet queue */
 	ao2_lock(th);
-	AST_LIST_INSERT_TAIL(&th->packet_q, packet, entry);
 	if (write(th->alert_pipe[1], &alert, sizeof(alert)) == -1) {
 		ast_log(LOG_ERROR, "write() to alert pipe failed: %s\n", strerror(errno));
-		AST_LIST_REMOVE_HEAD(&th->packet_q, entry);
 		ao2_t_ref(packet, -1, "could not write to alert pipe, remove packet");
 		packet = NULL;
 		res = XMIT_ERROR;
-	}
-
-	/* clean up locks, remove threadinfo ref from ao2_find */
+	} else { /* it is safe to queue the frame after issuing the alert when we hold the threadinfo lock */
+		AST_LIST_INSERT_TAIL(&th->packet_q, packet, entry);
+	}
 	ao2_unlock(th);
+
 	ast_mutex_unlock(&tcptls_session->lock);
 	ao2_t_ref(th, -1, "In sip_tcptls_write, unref threadinfo object after finding it");
 	return res;
@@ -3075,7 +3069,7 @@
 	 * 1. We own the parent session args for a client connection.  This pointer needs
 	 *    to be held on to so we can decrement it's ref count on thread destruction.
 	 * 2. The threadinfo object was created before this thread was launched, however
-	 *    it must be found within the threadl table.
+	 *    it must be found within the threadt table.
 	 * 3. Last, the tcptls_session must be started.
 	 */
 	if (!tcptls_session->client) {
@@ -3089,8 +3083,9 @@
 		};
 
 		if ((!(ca = tcptls_session->parent)) ||
-			(!(me = ao2_t_find(threadl, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread"))) ||
+			(!(me = ao2_t_find(threadt, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread"))) ||
 			(!(tcptls_session = ast_tcptls_client_start(tcptls_session)))) {
+			ast_log(LOG_ERROR, "No threadinfo object found for tcptls client thread \n");
 			goto cleanup;
 		}
 	}
@@ -3101,7 +3096,7 @@
 	/* set up pollfd to watch for reads on both the socket and the alert_pipe */
 	fds[0].fd = tcptls_session->fd;
 	fds[1].fd = me->alert_pipe[0];
-	fds[0].events = fds[1].events = POLLIN|POLLPRI;
+	fds[0].events = fds[1].events = POLLIN | POLLPRI;
 
 	if (!(req.data = ast_str_create(SIP_MIN_PACKET)))
 		goto cleanup;
@@ -3111,27 +3106,6 @@
 	for (;;) {
 		struct ast_str *str_save;
 
-		str_save = req.data;
-		memset(&req, 0, sizeof(req));
-		req.data = str_save;
-		ast_str_reset(req.data);
-
-		str_save = reqcpy.data;
-		memset(&reqcpy, 0, sizeof(reqcpy));
-		reqcpy.data = str_save;
-		ast_str_reset(reqcpy.data);
-
-		memset(buf, 0, sizeof(buf));
-
-		if (tcptls_session->ssl) {
-			set_socket_transport(&req.socket, SIP_TRANSPORT_TLS);
-			req.socket.port = htons(ourport_tls);
-		} else {
-			set_socket_transport(&req.socket, SIP_TRANSPORT_TCP);
-			req.socket.port = htons(ourport_tcp);
-		}
-		req.socket.fd = tcptls_session->fd;
-
 		res = ast_poll(fds, 2, -1); /* polls for both socket and alert_pipe */
 		if (res < 0) {
 			ast_debug(2, "SIP %s server :: ast_wait_for_input returned %d\n", tcptls_session->ssl ? "SSL": "TCP", res);
@@ -3141,7 +3115,8 @@
 		/* handle the socket event, check for both reads from the socket fd,
 		 * and writes from alert_pipe fd */
 		if (fds[0].revents) { /* there is data on the socket to be read */
-			struct ast_str *str_save;
+
+			fds[0].revents = 0;
 
 			/* clear request structure */
 			str_save = req.data;
@@ -3203,9 +3178,13 @@
 
 			req.socket.tcptls_session = tcptls_session;
 			handle_request_do(&req, &tcptls_session->remote_address);
-		} else if (fds[1].revents) { /* alert_pipe indicates there is data in the send queue to be sent */
+		}
+
+		if (fds[1].revents) { /* alert_pipe indicates there is data in the send queue to be sent */
 			enum sip_tcptls_alert alert;
 			struct tcptls_packet *packet;
+
+			fds[1].revents = 0;
 
 			if (read(me->alert_pipe[0], &alert, sizeof(alert)) == -1) {
 				ast_log(LOG_ERROR, "read() failed: %s\n", strerror(errno));
@@ -3238,7 +3217,7 @@
 
 cleanup:
 	if (me) {
-		ao2_t_unlink(threadl, me, "Removing tcptls helper thread, thread is closing");
+		ao2_t_unlink(threadt, me, "Removing tcptls helper thread, thread is closing");
 		ao2_t_ref(me, -1, "Removing tcp_helper_threads threadinfo ref");
 	}
 	if (reqcpy.data) {
@@ -14994,8 +14973,7 @@
 
 	ast_cli(a->fd, FORMAT2, "Host", "Port", "Transport", "Type");
 
-	ao2_lock(threadl);
-	i = ao2_iterator_init(threadl, 0);
+	i = ao2_iterator_init(threadt, 0);
 	while ((th = ao2_t_iterator_next(&i, "iterate through tcp threads for 'sip show tcp'"))) {
 		ast_cli(a->fd, FORMAT, ast_inet_ntoa(th->tcptls_session->remote_address.sin_addr),
 			ntohs(th->tcptls_session->remote_address.sin_port),
@@ -15003,7 +14981,6 @@
 			(th->tcptls_session->client ? "Client" : "Server"));
 		ao2_t_ref(th, -1, "decrement ref from iterator");
 	}
-	ao2_unlock(threadl);
 
 	return CLI_SUCCESS;
 #undef FORMAT
@@ -22926,12 +22903,10 @@
 	struct sip_threadinfo *th = obj;
 	struct sockaddr_in *s = arg;
 
-	if ((s->sin_family == th->tcptls_session->remote_address.sin_family) &&
-		(s->sin_addr.s_addr == th->tcptls_session->remote_address.sin_addr.s_addr) &&
-		(s->sin_port == th->tcptls_session->remote_address.sin_port)) {
-
+	if (!inaddrcmp(&th->tcptls_session->remote_address, s)) {
 		return CMP_MATCH | CMP_STOP;
 	}
+
 	return 0;
 }
 
@@ -22945,7 +22920,7 @@
 	struct sip_threadinfo *th;
 	struct ast_tcptls_session_instance *tcptls_instance = NULL;
 
-	if ((th = ao2_callback(threadl, 0, threadinfo_locate_cb, s))) {
+	if ((th = ao2_callback(threadt, 0, threadinfo_locate_cb, s))) {
 		tcptls_instance = (ao2_ref(th->tcptls_session, +1), th->tcptls_session);
 		ao2_t_ref(th, -1, "decrement ref from callback");
 	}
@@ -23014,10 +22989,10 @@
 
 	/* 3.  Create a new TCP/TLS client connection */
 	/* create new session arguments for the client connection */
-	if (!(ca = ao2_alloc(sizeof(*ca), sip_tcptls_client_args_destructor))) {
+	if (!(ca = ao2_alloc(sizeof(*ca), sip_tcptls_client_args_destructor)) ||
+		!(ca->name = ast_strdup(name))) {
 		goto create_tcptls_session_fail;
 	}
-	ca->name = ast_strdup(name);
 	ca->accept_fd = -1;
 	ca->remote_address = *(sip_real_dst(p));
 	/* if type is TLS, we need to create a tls cfg for this session arg */
@@ -23026,6 +23001,16 @@
 			goto create_tcptls_session_fail;
 		}
 		memcpy(ca->tls_cfg, &default_tls_cfg, sizeof(*ca->tls_cfg));
+
+		if (!(ca->tls_cfg->certfile = ast_strdup(default_tls_cfg.certfile)) ||
+			!(ca->tls_cfg->pvtfile = ast_strdup(default_tls_cfg.pvtfile)) ||
+			!(ca->tls_cfg->cipher = ast_strdup(default_tls_cfg.cipher)) ||
+			!(ca->tls_cfg->cafile = ast_strdup(default_tls_cfg.cafile)) ||
+			!(ca->tls_cfg->capath = ast_strdup(default_tls_cfg.capath))) {
+
+			goto create_tcptls_session_fail;
+		}
+
 		/* this host is used as the common name in ssl/tls */
 		if (!ast_strlen_zero(p->tohost)) {
 			ast_copy_string(ca->hostname, p->tohost, sizeof(ca->hostname));
@@ -23042,7 +23027,7 @@
 	/* client connections need to have the sip_threadinfo object created before
 	 * the thread is detached.  This ensures the alert_pipe is up before it will
 	 * be used.  Note that this function links the new threadinfo object into the
-	 * threadl container. */
+	 * threadt container. */
 	if (!(th = sip_threadinfo_create(s->tcptls_session, s->type))) {
 		goto create_tcptls_session_fail;
 	}
@@ -23069,7 +23054,7 @@
 		s->tcptls_session = NULL;
 	}
 	if (th) {
-		ao2_t_unlink(threadl, th, "Removing tcptls thread info object, thread failed to open");
+		ao2_t_unlink(threadt, th, "Removing tcptls thread info object, thread failed to open");
 	}
 
 	return -1;
@@ -26657,7 +26642,7 @@
 	peers = ao2_t_container_alloc(hash_peer_size, peer_hash_cb, peer_cmp_cb, "allocate peers");
 	peers_by_ip = ao2_t_container_alloc(hash_peer_size, peer_iphash_cb, peer_ipcmp_cb, "allocate peers_by_ip");
 	dialogs = ao2_t_container_alloc(hash_dialog_size, dialog_hash_cb, dialog_cmp_cb, "allocate dialogs");
-	threadl = ao2_t_container_alloc(hash_dialog_size, threadl_hash_cb, threadl_cmp_cb, "allocate threadl table");
+	threadt = ao2_t_container_alloc(hash_dialog_size, threadt_hash_cb, threadt_cmp_cb, "allocate threadt table");
 	
 	ASTOBJ_CONTAINER_INIT(&regl); /* Registry object list -- not searched for anything */
 	ASTOBJ_CONTAINER_INIT(&submwil); /* MWI subscription object list */
@@ -26788,7 +26773,7 @@
 		ast_tcptls_server_stop(&sip_tls_desc);
 
 	/* Kill all existing TCP/TLS threads */
-	i = ao2_iterator_init(threadl, 0);
+	i = ao2_iterator_init(threadt, 0);
 	while ((th = ao2_t_iterator_next(&i, "iterate through tcp threads for 'sip show tcp'"))) {
 		pthread_t thread = th->threadid;
 		th->stop = 1;
@@ -26796,6 +26781,7 @@
 		pthread_join(thread, NULL);
 		ao2_t_ref(th, -1, "decrement ref from iterator");
 	}
+	ao2_iterator_destroy(&i);
 
 	/* Hangup all dialogs if they have an owner */
 	i = ao2_iterator_init(dialogs, 0);
@@ -26848,7 +26834,7 @@
 	ao2_t_ref(peers, -1, "unref the peers table");
 	ao2_t_ref(peers_by_ip, -1, "unref the peers_by_ip table");
 	ao2_t_ref(dialogs, -1, "unref the dialogs table");
-	ao2_t_ref(threadl, -1, "unref the thread table");
+	ao2_t_ref(threadt, -1, "unref the thread table");
 
 	clear_sip_domains();
 	close(sipsock);

Modified: team/dvossel/sip_nonblocking_tcp_client/include/asterisk/tcptls.h
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/sip_nonblocking_tcp_client/include/asterisk/tcptls.h?view=diff&rev=225237&r1=225236&r2=225237
==============================================================================
--- team/dvossel/sip_nonblocking_tcp_client/include/asterisk/tcptls.h (original)
+++ team/dvossel/sip_nonblocking_tcp_client/include/asterisk/tcptls.h Wed Oct 21 13:23:27 2009
@@ -156,13 +156,13 @@
 #define LEN_T size_t
 #endif
 
-/*!
- * \brief A generic client routine for a TCP client
- * and starts a thread for handling accept()
- * \version 1.6.1 changed desc parameter to be of ast_tcptls_session_args type
- */
+/*! 
+  * \brief attempts to connect and start tcptls session, on error the tcptls_session's
+  * ref count is decremented, fd and file are closed, and NULL is returned.
+  */
 struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session);
 
+/* \brief Creates a client connection's ast_tcptls_session_instance. */
 struct ast_tcptls_session_instance *ast_tcptls_client_create(struct ast_tcptls_session_args *desc);
 
 void *ast_tcptls_server_root(void *);

Modified: team/dvossel/sip_nonblocking_tcp_client/main/tcptls.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/sip_nonblocking_tcp_client/main/tcptls.c?view=diff&rev=225237&r1=225236&r2=225237
==============================================================================
--- team/dvossel/sip_nonblocking_tcp_client/main/tcptls.c (original)
+++ team/dvossel/sip_nonblocking_tcp_client/main/tcptls.c Wed Oct 21 13:23:27 2009
@@ -358,9 +358,6 @@
 	return __ssl_setup(cfg, 0);
 }
 
-/*! brief\ attempts to connect and start tcptls session, on error the tcptls_session's
- * ref count is decremented, fd and file are closed, and NULL is returned.
- */
 struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session)
 {
 	struct ast_tcptls_session_args *desc;




More information about the svn-commits mailing list