[Asterisk-Dev] for review: ast_waitfor*()

[Luigi Rizzo]

as mentioned yesterday I have considerably simplified (both in terms
of size and complexity) the ast_waitfor*() functions.
I am attaching them here for review, before doing an additional
cleanup and submitting a patch through the bug repository.

There is stil one issue (which also applies to the original code):

	the first part of the function, scanning channel descriptor for
	timeouts, protects access to the channel using c[x]->lock.
	The remaining parts do not (nor they did in the original code).
	So which part is incorrect ?

Also you will note that this function has some code that looks
at  c[x]->generator to possibly poll on an additional file descriptor
when available (it lets you use musiconhold even without ztdummy).
I'd rather use one of the slots in c[x]->fds[] if possible.
Can someone tell me which slots in fds[] are reserved (it seems
AST_MAX_FDS-1 and AST_MAX_FDS-2 are) and which one i can use for that ?

	thanks
	luigi
-------------- next part --------------
/* if fd is a valid descriptor, set *pfd with the descriptor
 * Return 1 (not -1!) if added, 0 otherwise (so we can add the
 * return value to the index into the array)
 */
static int add_fd(struct pollfd *pfd, int fd)
{
	if (fd < 0)
		return 0;
	pfd->fd = fd;
	pfd->events = POLLIN | POLLPRI;
	return 1;
}

int ast_waitfor_n_fd(int *fds, int n, int *ms, int *exception)
{
	int winner = -1;
	ast_waitfor_nandfds(NULL, 0, fds, n, exception, &winner, ms);
	return winner;
}

struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds, int nfds, 
	int *exception, int *outfd, int *ms)
{
	/* Wait for x amount of time on a file descriptor to have input.  */
	struct timeval start;
	struct pollfd *pfds;
	int res;
	long rms;
	int x, y, max, have_timeout = 0;
	time_t now = 0;
	long i, whentohangup = 0;
	struct ast_channel *winner = NULL;
	int sz;
	int *ci, *fdno;	/* map pfds entries to channel index and fd index */

	if (outfd)
		*outfd = -1;
	if (exception)
		*exception = 0;
	
	sz = (n * (AST_MAX_FDS+1) + nfds);	/* XXX room for generator */
	pfds = alloca(sizeof(struct pollfd) * sz);
	ci = alloca(sizeof(int *) * sz);	/* pfds to channel map */
	fdno = alloca(sizeof(int *) * sz);	/* pfds to fd index */

	if (!pfds || !ci || !fdno) {
		ast_log(LOG_ERROR, "Out of memory\n");
		return NULL;
	}
	for (x=0; x < sz; x++)
		ci[x] = -1;	/* no channel linked to this pfd */

	/*
	 * Perform any pending masquerades, and take the min timeout.
	 */
	for (x=0; x<n; x++) {
		ast_mutex_lock(&c[x]->lock);
		if (c[x]->masq) {
			if (ast_do_masquerade(c[x])) {
				ast_log(LOG_WARNING, "Masquerade failed\n");
				*ms = -1;
				ast_mutex_unlock(&c[x]->lock);
				return NULL;
			}
		}
		i = c[x]->whentohangup;
		if (i && (!have_timeout || i < whentohangup)) {
			have_timeout = 1;
			whentohangup = i;
		}
		ast_mutex_unlock(&c[x]->lock);
	}

	rms = *ms;
	
	if (have_timeout) {
		i = (whentohangup - time(NULL)) * 1000; /* timeout in milliseconds */
		if (*ms < 0 || i < *ms)
			rms =  i;
	}
	/*
	 * Build the pollfd array, putting the channels' fds first,
	 * followed by individual fds. Order is important because
	 * individual fd's must have priority over channel fds.
	 */
	max = 0;
	for (x=0;x<n;x++) {
		struct ast_generator *g = c[x]->generator;
		int gfd;

		if (g && g->magic == AST_GEN_MAGIC &&
				(gfd = g->get_fd(c[x], c[x]->generatordata)) >= 0) {
			if (add_fd(&pfds[max], gfd)) {
				fdno[max] = AST_MAX_FDS;	/* XXX not really used */
				ci[max++] = x;  /* channel x is linked to this pfds */
			}
		}

		for (y=0;y<AST_MAX_FDS;y++)
			if (add_fd(&pfds[max], c[x]->fds[y])) {
				fdno[max] = y;	/* fd y is linked to this pfds */
				ci[max++] = x;  /* channel x is linked to this pfds */
			}
		CHECK_BLOCKING(c[x]);
	}
	/* followed by the individual fds */
	for (x=0;x<nfds; x++)
		max += add_fd(&pfds[max], fds[x]);
	if (*ms > 0) 
		start = ast_tvnow();	/* timestamp before the poll */
	res = poll(pfds, max, rms);
	if (res < 0) {
		for (x=0;x<n;x++) 
			ast_clear_flag(c[x], AST_FLAG_BLOCKING);
		/* Simulate a timeout if we were interrupted */
		*ms = (errno != EINTR) ? -1 : 0;
		return NULL;
	} else if (res == 0) {	/* no fds signalled */
		/* set ms = 0 since we may not have an exact timeout. */
		*ms = 0;
	}

	if (have_timeout)	/* take a timestamp */
		time(&now);
	/* first, check for timeouts on channels */
	for (x=0;x<n;x++) {
		ast_clear_flag(c[x], AST_FLAG_BLOCKING);
		if (c[x]->whentohangup && now > c[x]->whentohangup) {
			c[x]->_softhangup |= AST_SOFTHANGUP_TIMEOUT;
			if (winner == NULL)
				winner = c[x];
		}
	}
	/*
	 * Then check if any channel or fd has a pending event.
	 * Remember to check channels first and fds last, as they
	 * must have priority on setting 'winner'
	 */
	for (x = 0; x < max; x++) {
		res = pfds[x].revents;
		if (res == 0)
			continue;
		if (ci[x] >= 0) {	/* this is a channel */
			winner = c[ci[x]];	/* override previous winners */
			if (res & POLLPRI)
				ast_set_flag(winner, AST_FLAG_EXCEPTION);
			else
				ast_clear_flag(winner, AST_FLAG_EXCEPTION);
			winner->fdno = fdno[x];
		} else { /* this is an fd */
			if (outfd)
				*outfd = pfds[x].fd;
			if (exception)
				*exception = (res & POLLPRI) ? -1 : 0;
			winner = NULL;
		}
	}
	if (*ms > 0) {
		*ms -= ast_tvdiff(ast_tvnow(), start);
		if (*ms < 0)
			*ms = 0;
	}
	return winner;
}

struct ast_channel *ast_waitfor_n(struct ast_channel **c, int n, int *ms)
{
	return ast_waitfor_nandfds(c, n, NULL, 0, NULL, NULL, ms);
}

int ast_waitfor(struct ast_channel *c, int ms)
{
	struct ast_channel *chan;
	int oldms = ms;
	chan = ast_waitfor_n(&c, 1, &ms);
	if (ms < 0) {
		return (oldms < 0) ? 0 : -1;
	}
	return ms;
}