[Asterisk-code-review] channel: Remove old epoll support and fixed max number of fi... (asterisk[master])

Anonymous Coward asteriskteam at digium.com
Wed Mar 29 12:45:49 CDT 2017


Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5283 )

Change subject: channel: Remove old epoll support and fixed max number of file descriptors.
......................................................................


channel: Remove old epoll support and fixed max number of file descriptors.

This change removes the old epoll support which has not been used or
maintained in quite some time.

The fixed number of file descriptors on a channel has also been removed.
File descriptors are now contained in a growable vector. This can be
used like before by specifying a specific position to store a file
descriptor at or using a new API call, ast_channel_fd_add, which adds
a file descriptor to the channel and returns its position.

Tests have been added which cover the growing behavior of the vector
and the new API call.

ASTERISK-26885

Change-Id: I1a754b506c009b83dfdeeb08c2d2815db30ef928
---
M apps/app_dial.c
M apps/app_queue.c
M include/asterisk/channel.h
M main/channel.c
M main/channel_internal_api.c
M main/dial.c
A tests/test_channel.c
7 files changed, 201 insertions(+), 405 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, but someone else must approve
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/apps/app_dial.c b/apps/app_dial.c
index 1cb9181..c8fcf46 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -1186,9 +1186,6 @@
 	int prestart = num.busy + num.congestion + num.nochan;
 	int orig = *to;
 	struct ast_channel *peer = NULL;
-#ifdef HAVE_EPOLL
-	struct chanlist *epollo;
-#endif
 	struct chanlist *outgoing = AST_LIST_FIRST(out_chans);
 	/* single is set if only one destination is enabled */
 	int single = outgoing && !AST_LIST_NEXT(outgoing, node);
@@ -1226,12 +1223,6 @@
 	}
 
 	is_cc_recall = ast_cc_is_recall(in, &cc_recall_core_id, NULL);
-
-#ifdef HAVE_EPOLL
-	AST_LIST_TRAVERSE(out_chans, epollo, node) {
-		ast_poll_channel_add(in, epollo->chan);
-	}
-#endif
 
 	while ((*to = ast_remaining_ms(start, orig)) && !peer) {
 		struct chanlist *o;
@@ -1359,9 +1350,6 @@
 			f = ast_read(winner);
 			if (!f) {
 				ast_channel_hangupcause_set(in, ast_channel_hangupcause(c));
-#ifdef HAVE_EPOLL
-				ast_poll_channel_del(in, c);
-#endif
 				ast_channel_publish_dial(in, c, NULL, ast_hangup_cause_to_dial_status(ast_channel_hangupcause(c)));
 				ast_hangup(c);
 				c = o->chan = NULL;
@@ -1785,13 +1773,6 @@
 		ast_verb(3, "Nobody picked up in %d ms\n", orig);
 		publish_dial_end_event(in, out_chans, NULL, "NOANSWER");
 	}
-
-#ifdef HAVE_EPOLL
-	AST_LIST_TRAVERSE(out_chans, epollo, node) {
-		if (epollo->chan)
-			ast_poll_channel_del(in, epollo->chan);
-	}
-#endif
 
 	if (is_cc_recall) {
 		ast_cc_completed(in, "Recall completed!");
diff --git a/apps/app_queue.c b/apps/app_queue.c
index 3886b7c..b394084 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -4794,9 +4794,6 @@
 	char membername[80] = "";
 	long starttime = 0;
 	long endtime = 0;
-#ifdef HAVE_EPOLL
-	struct callattempt *epollo;
-#endif
 	char *inchan_name;
 	struct timeval start_time_tv = ast_tvnow();
 	int canceled_by_caller = 0; /* 1 when caller hangs up or press digit or press * */
@@ -4806,13 +4803,6 @@
 	ast_channel_unlock(qe->chan);
 
 	starttime = (long) time(NULL);
-#ifdef HAVE_EPOLL
-	for (epollo = outgoing; epollo; epollo = epollo->q_next) {
-		if (epollo->chan) {
-			ast_poll_channel_add(in, epollo->chan);
-		}
-	}
-#endif
 
 	while ((*to = ast_remaining_ms(start_time_tv, orig)) && !peer) {
 		int numlines, retry, pos = 1;
@@ -5324,14 +5314,6 @@
 
 		publish_dial_end_event(qe->chan, outgoing, NULL, "NOANSWER");
 	}
-
-#ifdef HAVE_EPOLL
-	for (epollo = outgoing; epollo; epollo = epollo->q_next) {
-		if (epollo->chan) {
-			ast_poll_channel_del(in, epollo->chan);
-		}
-	}
-#endif
 
 	return peer;
 }
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 9a3a967..391e58c 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -183,7 +183,8 @@
 
 #define DATASTORE_INHERIT_FOREVER	INT_MAX
 
-#define AST_MAX_FDS		11
+#define AST_MAX_FDS		11	/*!< original maximum number of file descriptors */
+#define AST_EXTENDED_FDS	12	/*!< the start of extended file descriptor positions */
 /*
  * We have AST_MAX_FDS file descriptors in a channel.
  * Some of them have a fixed use:
@@ -2402,12 +2403,6 @@
 /*! Set the file descriptor on the channel */
 void ast_channel_set_fd(struct ast_channel *chan, int which, int fd);
 
-/*! Add a channel to an optimized waitfor */
-void ast_poll_channel_add(struct ast_channel *chan0, struct ast_channel *chan1);
-
-/*! Delete a channel from an optimized waitfor */
-void ast_poll_channel_del(struct ast_channel *chan0, struct ast_channel *chan1);
-
 /*! Start a tone going */
 int ast_tonepair_start(struct ast_channel *chan, int freq1, int freq2, int duration, int vol);
 /*! Stop a tone from playing */
@@ -4300,11 +4295,30 @@
 int ast_channel_fd(const struct ast_channel *chan, int which);
 int ast_channel_fd_isset(const struct ast_channel *chan, int which);
 
-/* epoll data internal accessors */
-#ifdef HAVE_EPOLL
-struct ast_epoll_data *ast_channel_internal_epfd_data(const struct ast_channel *chan, int which);
-void ast_channel_internal_epfd_data_set(struct ast_channel *chan, int which , struct ast_epoll_data *value);
-#endif
+/*!
+ * \since 15
+ * \brief Retrieve the number of file decriptor positions present on the channel
+ *
+ * \param chan The channel to get the count of
+ *
+ * \pre chan is locked
+ *
+ * \return The number of file descriptor positions
+ */
+int ast_channel_fd_count(const struct ast_channel *chan);
+
+/*!
+ * \since 15
+ * \brief Add a file descriptor to the channel without a fixed position
+ *
+ * \param chan The channel to add the file descriptor to
+ * \param value The file descriptor
+ *
+ * \pre chan is locked
+ *
+ * \return The position of the file descriptor
+ */
+int ast_channel_fd_add(struct ast_channel *chan, int value);
 
 pthread_t ast_channel_blocker(const struct ast_channel *chan);
 void ast_channel_blocker_set(struct ast_channel *chan, pthread_t value);
diff --git a/main/channel.c b/main/channel.c
index 15c7fa4..31f3639 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -78,20 +78,11 @@
 /*** DOCUMENTATION
  ***/
 
-#ifdef HAVE_EPOLL
-#include <sys/epoll.h>
-#endif
-
 #if defined(KEEP_TILL_CHANNEL_PARTY_NUMBER_INFO_NEEDED)
 #if defined(HAVE_PRI)
 #include "libpri.h"
 #endif	/* defined(HAVE_PRI) */
 #endif	/* defined(KEEP_TILL_CHANNEL_PARTY_NUMBER_INFO_NEEDED) */
-
-struct ast_epoll_data {
-	struct ast_channel *chan;
-	int which;
-};
 
 /* uncomment if you have problems with 'monitoring' synchronized files */
 #if 0
@@ -850,10 +841,6 @@
 	ast_channel_internal_alertpipe_clear(tmp);
 	ast_channel_internal_fd_clear_all(tmp);
 
-#ifdef HAVE_EPOLL
-	ast_channel_epfd_set(tmp, epoll_create(25));
-#endif
-
 	if (!(schedctx = ast_sched_context_create())) {
 		ast_log(LOG_WARNING, "Channel allocation failed: Unable to create schedule context\n");
 		/* See earlier channel creation abort comment above. */
@@ -1058,9 +1045,6 @@
 	ast_channel_timingfd_set(tmp, -1);
 	ast_channel_internal_alertpipe_clear(tmp);
 	ast_channel_internal_fd_clear_all(tmp);
-#ifdef HAVE_EPOLL
-	ast_channel_epfd_set(tmp, -1);
-#endif
 
 	ast_channel_hold_state_set(tmp, AST_CONTROL_UNHOLD);
 
@@ -2223,9 +2207,6 @@
 static void ast_channel_destructor(void *obj)
 {
 	struct ast_channel *chan = obj;
-#ifdef HAVE_EPOLL
-	int i;
-#endif
 	struct ast_var_t *vardata;
 	struct ast_frame *f;
 	struct varshead *headp;
@@ -2323,14 +2304,6 @@
 		ast_timer_close(ast_channel_timer(chan));
 		ast_channel_timer_set(chan, NULL);
 	}
-#ifdef HAVE_EPOLL
-	for (i = 0; i < AST_MAX_FDS; i++) {
-		if (ast_channel_internal_epfd_data(chan, i)) {
-			ast_free(ast_channel_internal_epfd_data(chan, i));
-		}
-	}
-	close(ast_channel_epfd(chan));
-#endif
 	while ((f = AST_LIST_REMOVE_HEAD(ast_channel_readq(chan), frame_list)))
 		ast_frfree(f);
 
@@ -2481,79 +2454,7 @@
 /*! Set the file descriptor on the channel */
 void ast_channel_set_fd(struct ast_channel *chan, int which, int fd)
 {
-#ifdef HAVE_EPOLL
-	struct epoll_event ev;
-	struct ast_epoll_data *aed = NULL;
-
-	if (ast_channel_fd_isset(chan, which)) {
-		epoll_ctl(ast_channel_epfd(chan), EPOLL_CTL_DEL, ast_channel_fd(chan, which), &ev);
-		aed = ast_channel_internal_epfd_data(chan, which);
-	}
-
-	/* If this new fd is valid, add it to the epoll */
-	if (fd > -1) {
-		if (!aed && (!(aed = ast_calloc(1, sizeof(*aed)))))
-			return;
-
-		ast_channel_internal_epfd_data_set(chan, which, aed);
-		aed->chan = chan;
-		aed->which = which;
-
-		ev.events = EPOLLIN | EPOLLPRI | EPOLLERR | EPOLLHUP;
-		ev.data.ptr = aed;
-		epoll_ctl(ast_channel_epfd(chan), EPOLL_CTL_ADD, fd, &ev);
-	} else if (aed) {
-		/* We don't have to keep around this epoll data structure now */
-		ast_free(aed);
-		ast_channel_epfd_data_set(chan, which, NULL);
-	}
-#endif
 	ast_channel_internal_fd_set(chan, which, fd);
-	return;
-}
-
-/*! Add a channel to an optimized waitfor */
-void ast_poll_channel_add(struct ast_channel *chan0, struct ast_channel *chan1)
-{
-#ifdef HAVE_EPOLL
-	struct epoll_event ev;
-	int i = 0;
-
-	if (ast_channel_epfd(chan0) == -1)
-		return;
-
-	/* Iterate through the file descriptors on chan1, adding them to chan0 */
-	for (i = 0; i < AST_MAX_FDS; i++) {
-		if (!ast_channel_fd_isset(chan1, i)) {
-			continue;
-		}
-		ev.events = EPOLLIN | EPOLLPRI | EPOLLERR | EPOLLHUP;
-		ev.data.ptr = ast_channel_internal_epfd_data(chan1, i);
-		epoll_ctl(ast_channel_epfd(chan0), EPOLL_CTL_ADD, ast_channel_fd(chan1, i), &ev);
-	}
-
-#endif
-	return;
-}
-
-/*! Delete a channel from an optimized waitfor */
-void ast_poll_channel_del(struct ast_channel *chan0, struct ast_channel *chan1)
-{
-#ifdef HAVE_EPOLL
-	struct epoll_event ev;
-	int i = 0;
-
-	if (ast_channel_epfd(chan0) == -1)
-		return;
-
-	for (i = 0; i < AST_MAX_FDS; i++) {
-		if (!ast_channel_fd_isset(chan1, i)) {
-			continue;
-		}
-		epoll_ctl(ast_channel_epfd(chan0), EPOLL_CTL_DEL, ast_channel_fd(chan1, i), &ev);
-	}
-
-#endif
 	return;
 }
 
@@ -3061,20 +2962,15 @@
 }
 
 /*! \brief Wait for x amount of time on a file descriptor to have input.  */
-#ifdef HAVE_EPOLL
-static struct ast_channel *ast_waitfor_nandfds_classic(struct ast_channel **c, int n, int *fds, int nfds,
-					int *exception, int *outfd, int *ms)
-#else
 struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds, int nfds,
 					int *exception, int *outfd, int *ms)
-#endif
 {
 	struct timeval start = { 0 , 0 };
 	struct pollfd *pfds = NULL;
 	int res;
 	long rms;
 	int x, y, max;
-	int sz;
+	int sz = nfds;
 	struct timeval now = { 0, 0 };
 	struct timeval whentohangup = { 0, 0 }, diff;
 	struct ast_channel *winner = NULL;
@@ -3088,14 +2984,6 @@
 	}
 	if (exception) {
 		*exception = 0;
-	}
-
-	if ((sz = n * AST_MAX_FDS + nfds)) {
-		pfds = ast_alloca(sizeof(*pfds) * sz);
-		fdmap = ast_alloca(sizeof(*fdmap) * sz);
-	} else {
-		/* nothing to allocate and no FDs to check */
-		return NULL;
 	}
 
 	for (x = 0; x < n; x++) {
@@ -3114,8 +3002,17 @@
 			if (ast_tvzero(whentohangup) || ast_tvcmp(diff, whentohangup) < 0)
 				whentohangup = diff;
 		}
+		sz += ast_channel_fd_count(c[x]);
 		ast_channel_unlock(c[x]);
 	}
+
+	if (!sz) {
+		return NULL;
+	}
+
+	pfds = ast_alloca(sizeof(*pfds) * sz);
+	fdmap = ast_alloca(sizeof(*fdmap) * sz);
+
 	/* Wait full interval */
 	rms = *ms;
 	/* INT_MAX, not LONG_MAX, because it matters on 64-bit */
@@ -3135,12 +3032,12 @@
 	 */
 	max = 0;
 	for (x = 0; x < n; x++) {
-		for (y = 0; y < AST_MAX_FDS; y++) {
+		ast_channel_lock(c[x]);
+		for (y = 0; y < ast_channel_fd_count(c[x]); y++) {
 			fdmap[max].fdno = y;  /* fd y is linked to this pfds */
 			fdmap[max].chan = x;  /* channel x is linked to this pfds */
 			max += ast_add_fd(&pfds[max], ast_channel_fd(c[x], y));
 		}
-		ast_channel_lock(c[x]);
 		CHECK_BLOCKING(c[x]);
 		ast_channel_unlock(c[x]);
 	}
@@ -3233,205 +3130,6 @@
 	}
 	return winner;
 }
-
-#ifdef HAVE_EPOLL
-static struct ast_channel *ast_waitfor_nandfds_simple(struct ast_channel *chan, int *ms)
-{
-	struct timeval start = { 0 , 0 };
-	int res = 0;
-	struct epoll_event ev[1];
-	long diff, rms = *ms;
-	struct ast_channel *winner = NULL;
-	struct ast_epoll_data *aed = NULL;
-
-	ast_channel_lock(chan);
-	/* Figure out their timeout */
-	if (!ast_tvzero(*ast_channel_whentohangup(chan))) {
-		if ((diff = ast_tvdiff_ms(*ast_channel_whentohangup(chan), ast_tvnow())) < 0) {
-			/* They should already be hungup! */
-			ast_channel_softhangup_internal_flag_add(chan, AST_SOFTHANGUP_TIMEOUT);
-			ast_channel_unlock(chan);
-			return NULL;
-		}
-		/* If this value is smaller then the current one... make it priority */
-		if (rms > diff) {
-			rms = diff;
-		}
-	}
-
-	ast_channel_unlock(chan);
-
-	/* Time to make this channel block... */
-	CHECK_BLOCKING(chan);
-
-	if (*ms > 0) {
-		start = ast_tvnow();
-	}
-
-	/* We don't have to add any file descriptors... they are already added, we just have to wait! */
-	res = epoll_wait(ast_channel_epfd(chan), ev, 1, rms);
-
-	/* Stop blocking */
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
-
-	/* Simulate a timeout if we were interrupted */
-	if (res < 0) {
-		if (errno != EINTR) {
-			*ms = -1;
-		}
-		return NULL;
-	}
-
-	/* If this channel has a timeout see if it expired */
-	if (!ast_tvzero(*ast_channel_whentohangup(chan))) {
-		if (ast_tvdiff_ms(ast_tvnow(), *ast_channel_whentohangup(chan)) >= 0) {
-			ast_channel_softhangup_internal_flag_add(chan, AST_SOFTHANGUP_TIMEOUT);
-			winner = chan;
-		}
-	}
-
-	/* No fd ready, reset timeout and be done for now */
-	if (!res) {
-		*ms = 0;
-		return winner;
-	}
-
-	/* See what events are pending */
-	aed = ev[0].data.ptr;
-	ast_channel_fdno_set(chan, aed->which);
-	if (ev[0].events & EPOLLPRI) {
-		ast_set_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION);
-	} else {
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION);
-	}
-
-	if (*ms > 0) {
-		*ms -= ast_tvdiff_ms(ast_tvnow(), start);
-		if (*ms < 0) {
-			*ms = 0;
-		}
-	}
-
-	return chan;
-}
-
-static struct ast_channel *ast_waitfor_nandfds_complex(struct ast_channel **c, int n, int *ms)
-{
-	struct timeval start = { 0 , 0 };
-	int res = 0, i;
-	struct epoll_event ev[25] = { { 0, } };
-	struct timeval now = { 0, 0 };
-	long whentohangup = 0, diff = 0, rms = *ms;
-	struct ast_channel *winner = NULL;
-
-	for (i = 0; i < n; i++) {
-		ast_channel_lock(c[i]);
-		if (!ast_tvzero(*ast_channel_whentohangup(c[i]))) {
-			if (whentohangup == 0) {
-				now = ast_tvnow();
-			}
-			if ((diff = ast_tvdiff_ms(*ast_channel_whentohangup(c[i]), now)) < 0) {
-				ast_channel_softhangup_internal_flag_add(c[i], AST_SOFTHANGUP_TIMEOUT);
-				ast_channel_unlock(c[i]);
-				return c[i];
-			}
-			if (!whentohangup || whentohangup > diff) {
-				whentohangup = diff;
-			}
-		}
-		ast_channel_unlock(c[i]);
-		CHECK_BLOCKING(c[i]);
-	}
-
-	rms = *ms;
-	if (whentohangup) {
-		rms = whentohangup;
-		if (*ms >= 0 && *ms < rms) {
-			rms = *ms;
-		}
-	}
-
-	if (*ms > 0) {
-		start = ast_tvnow();
-	}
-
-	res = epoll_wait(ast_channel_epfd(c[0]), ev, 25, rms);
-
-	for (i = 0; i < n; i++) {
-		ast_clear_flag(ast_channel_flags(c[i]), AST_FLAG_BLOCKING);
-	}
-
-	if (res < 0) {
-		if (errno != EINTR) {
-			*ms = -1;
-		}
-		return NULL;
-	}
-
-	if (whentohangup) {
-		now = ast_tvnow();
-		for (i = 0; i < n; i++) {
-			if (!ast_tvzero(*ast_channel_whentohangup(c[i])) && ast_tvdiff_ms(now, *ast_channel_whentohangup(c[i])) >= 0) {
-				ast_channel_softhangup_internal_flag_add(c[i], AST_SOFTHANGUP_TIMEOUT);
-				if (!winner) {
-					winner = c[i];
-				}
-			}
-		}
-	}
-
-	if (!res) {
-		*ms = 0;
-		return winner;
-	}
-
-	for (i = 0; i < res; i++) {
-		struct ast_epoll_data *aed = ev[i].data.ptr;
-
-		if (!ev[i].events || !aed) {
-			continue;
-		}
-
-		winner = aed->chan;
-		if (ev[i].events & EPOLLPRI) {
-			ast_set_flag(ast_channel_flags(winner), AST_FLAG_EXCEPTION);
-		} else {
-			ast_clear_flag(ast_channel_flags(winner), AST_FLAG_EXCEPTION);
-		}
-		ast_channel_fdno_set(winner, aed->which);
-	}
-
-	if (*ms > 0) {
-		*ms -= ast_tvdiff_ms(ast_tvnow(), start);
-		if (*ms < 0) {
-			*ms = 0;
-		}
-	}
-
-	return winner;
-}
-
-struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds, int nfds,
-					int *exception, int *outfd, int *ms)
-{
-	/* Clear all provided values in one place. */
-	if (outfd) {
-		*outfd = -99999;
-	}
-	if (exception) {
-		*exception = 0;
-	}
-
-	/* If no epoll file descriptor is available resort to classic nandfds */
-	if (!n || nfds || ast_channel_epfd(c[0]) == -1) {
-		return ast_waitfor_nandfds_classic(c, n, fds, nfds, exception, outfd, ms);
-	} else if (!nfds && n == 1) {
-		return ast_waitfor_nandfds_simple(c[0], ms);
-	} else {
-		return ast_waitfor_nandfds_complex(c, n, ms);
-	}
-}
-#endif
 
 struct ast_channel *ast_waitfor_n(struct ast_channel **c, int n, int *ms)
 {
@@ -6852,6 +6550,7 @@
 	int origstate;
 	unsigned int orig_disablestatecache;
 	unsigned int clone_disablestatecache;
+	int generator_fd;
 	int visible_indication;
 	int clone_hold_state;
 	int moh_is_playing;
@@ -7042,8 +6741,13 @@
 	/* Keep the same parkinglot. */
 	ast_channel_parkinglot_set(original, ast_channel_parkinglot(clonechan));
 
-	/* Copy the FD's other than the generator fd */
-	for (x = 0; x < AST_MAX_FDS; x++) {
+	/* Clear all existing file descriptors but retain the generator */
+	generator_fd = ast_channel_fd(original, AST_GENERATOR_FD);
+	ast_channel_internal_fd_clear_all(original);
+	ast_channel_set_fd(original, AST_GENERATOR_FD, generator_fd);
+
+	/* Copy all file descriptors present on clonechan to original, skipping generator */
+	for (x = 0; x < ast_channel_fd_count(clonechan); x++) {
 		if (x != AST_GENERATOR_FD)
 			ast_channel_set_fd(original, x, ast_channel_fd(clonechan, x));
 	}
diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index d7ae8f9..b3c0a48 100644
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -48,6 +48,7 @@
 #include "asterisk/stringfields.h"
 #include "asterisk/stream.h"
 #include "asterisk/test.h"
+#include "asterisk/vector.h"
 
 /*!
  * \brief Channel UniqueId structure
@@ -96,9 +97,6 @@
 							 *   in the CHANNEL dialplan function */
 	struct ast_channel_monitor *monitor;		/*!< Channel monitoring */
 	ast_callid callid;			/*!< Bound call identifier pointer */
-#ifdef HAVE_EPOLL
-	struct ast_epoll_data *epfd_data[AST_MAX_FDS];
-#endif
 	struct ao2_container *dialed_causes;		/*!< Contains tech-specific and Asterisk cause data from dialed channels */
 
 	AST_DECLARE_STRING_FIELDS(
@@ -167,7 +165,7 @@
 	unsigned long insmpl;				/*!< Track the read/written samples for monitor use */
 	unsigned long outsmpl;				/*!< Track the read/written samples for monitor use */
 
-	int fds[AST_MAX_FDS];				/*!< File descriptors for channel -- Drivers will poll on
+	AST_VECTOR(, int) fds;				/*!< File descriptors for channel -- Drivers will poll on
 							 *   these file descriptors, so at least one must be non -1.
 							 *   See \arg \ref AstFileDesc */
 	int softhangup;				/*!< Whether or not we have been hung up...  Do not set this value
@@ -197,9 +195,6 @@
 	struct ast_format *rawreadformat;         /*!< Raw read format (before translation) */
 	struct ast_format *rawwriteformat;        /*!< Raw write format (after translation) */
 	unsigned int emulate_dtmf_duration;		/*!< Number of ms left to emulate DTMF for */
-#ifdef HAVE_EPOLL
-	int epfd;
-#endif
 	int visible_indication;                         /*!< Indication currently playing on the channel */
 	int hold_state;							/*!< Current Hold/Unhold state */
 
@@ -597,17 +592,6 @@
 	chan->amaflags = value;
 	ast_channel_publish_snapshot(chan);
 }
-
-#ifdef HAVE_EPOLL
-int ast_channel_epfd(const struct ast_channel *chan)
-{
-	return chan->epfd;
-}
-void ast_channel_epfd_set(struct ast_channel *chan, int value)
-{
-	chan->epfd = value;
-}
-#endif
 int ast_channel_fdno(const struct ast_channel *chan)
 {
 	return chan->fdno;
@@ -1432,38 +1416,55 @@
 /* file descriptor array accessors */
 void ast_channel_internal_fd_set(struct ast_channel *chan, int which, int value)
 {
-	chan->fds[which] = value;
+	int pos;
+
+	/* This ensures that if the vector has to grow with unused positions they will be
+	 * initialized to -1.
+	 */
+	for (pos = AST_VECTOR_SIZE(&chan->fds); pos < which; pos++) {
+		AST_VECTOR_REPLACE(&chan->fds, pos, -1);
+	}
+
+	AST_VECTOR_REPLACE(&chan->fds, which, value);
 }
 void ast_channel_internal_fd_clear(struct ast_channel *chan, int which)
 {
-	ast_channel_internal_fd_set(chan, which, -1);
+	if (which >= AST_VECTOR_SIZE(&chan->fds)) {
+		return;
+	}
+
+	AST_VECTOR_REPLACE(&chan->fds, which, -1);
 }
 void ast_channel_internal_fd_clear_all(struct ast_channel *chan)
 {
-	int i;
-	for (i = 0; i < AST_MAX_FDS; i++) {
-		ast_channel_internal_fd_clear(chan, i);
-	}
+	AST_VECTOR_RESET(&chan->fds, AST_VECTOR_ELEM_CLEANUP_NOOP);
 }
 int ast_channel_fd(const struct ast_channel *chan, int which)
 {
-	return chan->fds[which];
+	return (which >= AST_VECTOR_SIZE(&chan->fds)) ? -1 : AST_VECTOR_GET(&chan->fds, which);
 }
 int ast_channel_fd_isset(const struct ast_channel *chan, int which)
 {
 	return ast_channel_fd(chan, which) > -1;
 }
 
-#ifdef HAVE_EPOLL
-struct ast_epoll_data *ast_channel_internal_epfd_data(const struct ast_channel *chan, int which)
+int ast_channel_fd_count(const struct ast_channel *chan)
 {
-	return chan->epfd_data[which];
+	return AST_VECTOR_SIZE(&chan->fds);
 }
-void ast_channel_internal_epfd_data_set(struct ast_channel *chan, int which , struct ast_epoll_data *value)
+
+int ast_channel_fd_add(struct ast_channel *chan, int value)
 {
-	chan->epfd_data[which] = value;
+	int pos = AST_EXTENDED_FDS;
+
+	while (ast_channel_fd_isset(chan, pos)) {
+		pos += 1;
+	}
+
+	AST_VECTOR_REPLACE(&chan->fds, pos, value);
+
+	return pos;
 }
-#endif
 
 pthread_t ast_channel_blocker(const struct ast_channel *chan)
 {
@@ -1617,6 +1618,8 @@
 		tmp->linkedid = tmp->uniqueid;
 	}
 
+	AST_VECTOR_INIT(&tmp->fds, AST_MAX_FDS);
+
 	return tmp;
 }
 
@@ -1693,6 +1696,8 @@
 	chan->topics = NULL;
 
 	ast_channel_internal_set_stream_topology(chan, NULL);
+
+	AST_VECTOR_FREE(&chan->fds);
 }
 
 void ast_channel_internal_finalize(struct ast_channel *chan)
diff --git a/main/dial.c b/main/dial.c
index cc2366e..d0492dc 100644
--- a/main/dial.c
+++ b/main/dial.c
@@ -479,9 +479,6 @@
 		ast_hangup(channel->owner);
 		channel->owner = NULL;
 	} else {
-		if (chan) {
-			ast_poll_channel_add(chan, channel->owner);
-		}
 		ast_channel_publish_dial(async ? NULL : chan, channel->owner, channel->device, NULL);
 		res = 1;
 		ast_verb(3, "Called %s\n", numsubst);
@@ -868,8 +865,6 @@
 				set_state(dial, AST_DIAL_RESULT_HANGUP);
 				break;
 			}
-			if (chan)
-				ast_poll_channel_del(chan, channel->owner);
 			ast_channel_publish_dial(chan, who, channel->device, ast_hangup_cause_to_dial_status(ast_channel_hangupcause(who)));
 			ast_hangup(who);
 			channel->owner = NULL;
@@ -890,8 +885,6 @@
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (!channel->owner || channel->owner == who)
 				continue;
-			if (chan)
-				ast_poll_channel_del(chan, channel->owner);
 			ast_channel_publish_dial(chan, channel->owner, channel->device, "CANCEL");
 			ast_hangup(channel->owner);
 			channel->cause = AST_CAUSE_ANSWERED_ELSEWHERE;
@@ -915,8 +908,6 @@
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (!channel->owner)
 				continue;
-			if (chan)
-				ast_poll_channel_del(chan, channel->owner);
 			ast_channel_publish_dial(chan, channel->owner, channel->device, "CANCEL");
 			ast_hangup(channel->owner);
 			channel->cause = AST_CAUSE_NORMAL_CLEARING;
diff --git a/tests/test_channel.c b/tests/test_channel.c
new file mode 100644
index 0000000..854aff7
--- /dev/null
+++ b/tests/test_channel.c
@@ -0,0 +1,119 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Digium, Inc.
+ *
+ * Joshua Colp <jcolp at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*!
+ * \file
+ * \brief Channel unit tests
+ *
+ * \author Joshua Colp <jcolp at digium.com>
+ *
+ */
+
+/*** MODULEINFO
+	<depend>TEST_FRAMEWORK</depend>
+	<support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+
+#include "asterisk/module.h"
+#include "asterisk/test.h"
+#include "asterisk/channel.h"
+
+AST_TEST_DEFINE(set_fd_grow)
+{
+	struct ast_channel *mock_channel;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	int pos;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "set_fd_grow";
+		info->category = "/main/channel/";
+		info->summary = "channel setting file descriptor with growth test";
+		info->description =
+			"Test that setting a file descriptor on a high position of a channel results in -1 set on any new positions";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = ast_channel_alloc(0, AST_STATE_DOWN, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, "TestChannel");
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	ast_channel_set_fd(mock_channel, AST_EXTENDED_FDS + 10, 1);
+	ast_test_validate_cleanup(test, ast_channel_fd_count(mock_channel) == AST_EXTENDED_FDS + 11, res, done);
+
+	for (pos = AST_EXTENDED_FDS; (pos < AST_EXTENDED_FDS + 10); pos++) {
+		ast_test_validate_cleanup(test, ast_channel_fd(mock_channel, pos) == -1, res, done);
+	}
+
+done:
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
+AST_TEST_DEFINE(add_fd)
+{
+	struct ast_channel *mock_channel;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	int pos;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "add_fd";
+		info->category = "/main/channel/";
+		info->summary = "channel adding file descriptor test";
+		info->description =
+			"Test that adding a file descriptor to a channel places it in the expected position";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = ast_channel_alloc(0, AST_STATE_DOWN, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, "TestChannel");
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	pos = ast_channel_fd_add(mock_channel, 1);
+	ast_test_validate_cleanup(test, pos == AST_EXTENDED_FDS, res, done);
+
+	ast_channel_set_fd(mock_channel, pos, -1);
+	ast_test_validate_cleanup(test, ast_channel_fd(mock_channel, pos) == -1, res, done);
+
+done:
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
+static int unload_module(void)
+{
+	AST_TEST_UNREGISTER(set_fd_grow);
+	AST_TEST_UNREGISTER(add_fd);
+	return 0;
+}
+
+static int load_module(void)
+{
+	AST_TEST_REGISTER(set_fd_grow);
+	AST_TEST_REGISTER(add_fd);
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Channel Unit Tests");

-- 
To view, visit https://gerrit.asterisk.org/5283
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1a754b506c009b83dfdeeb08c2d2815db30ef928
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list