[Asterisk-code-review] core: Use eventfd for alert pipes on Linux when possible (asterisk[master])

George Joseph asteriskteam at digium.com
Tue Apr 25 09:34:48 CDT 2017


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/5499 )

Change subject: core: Use eventfd for alert pipes on Linux when possible
......................................................................


core: Use eventfd for alert pipes on Linux when possible

The primary win of switching to eventfd when possible is that it only
uses a single file descriptor while pipe() will use two. This means for
each bridge channel we're reducing the number of required file
descriptors by 1, and - if you're using timerfd - we also now have 1
less file descriptor per Asterisk channel.

The API is not ideal (passing int arrays), but this is the cleanest
approach I could come up with to maintain API/ABI.

I've also removed what I believe to be an erroneous code block that
checked the non-blocking flag on the pipe ends for each read. If the
file descriptor is 'losing' its non-blocking mode, it is because of a
bug somewhere else in our code.

In my testing I haven't seen any measurable difference in performance.

Change-Id: Iff0fb1573e7f7a187d5211ddc60aa8f3da3edb1d
---
M configure
M configure.ac
A include/asterisk/alertpipe.h
M include/asterisk/autoconfig.h.in
M include/asterisk/channel.h
A main/alertpipe.c
M main/bridge_channel.c
M main/channel_internal_api.c
M main/utils.c
9 files changed, 394 insertions(+), 197 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Jenkins2: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/configure b/configure
index dab7038..89de95a 100755
--- a/configure
+++ b/configure
@@ -18009,6 +18009,41 @@
 
 fi
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if we have usable eventfd support" >&5
+$as_echo_n "checking if we have usable eventfd support... " >&6; }
+if test "$cross_compiling" = yes; then :
+  { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "cannot run test program while cross compiling
+See \`config.log' for more details" "$LINENO" 5; }
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <sys/eventfd.h>
+int
+main ()
+{
+return eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE) == -1;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_run "$LINENO"; then :
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_EVENTFD 1" >>confdefs.h
+
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+  conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for compiler 'attribute pure' support" >&5
 $as_echo_n "checking for compiler 'attribute pure' support... " >&6; }
diff --git a/configure.ac b/configure.ac
index d8a567f..3bfb82e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1125,6 +1125,15 @@
 		AC_DEFINE([CONFIGURE_RAN_AS_ROOT], 1, [Some configure tests will unexpectedly fail if configure is run by a non-root user.  These may be able to be tested at runtime.]))
 fi
 
+AC_MSG_CHECKING([if we have usable eventfd support])
+AC_RUN_IFELSE(
+  [AC_LANG_PROGRAM([#include <sys/eventfd.h>],
+      [return eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE) == -1;])],
+  AC_MSG_RESULT(yes)
+  AC_DEFINE([HAVE_EVENTFD], 1, [Define to 1 if your system supports eventfd and the EFD_NONBLOCK and EFD_SEMAPHORE flags.]),
+  AC_MSG_RESULT(no)
+)
+
 AST_GCC_ATTRIBUTE(pure)
 AST_GCC_ATTRIBUTE(malloc)
 AST_GCC_ATTRIBUTE(const)
diff --git a/include/asterisk/alertpipe.h b/include/asterisk/alertpipe.h
new file mode 100644
index 0000000..5ff854c
--- /dev/null
+++ b/include/asterisk/alertpipe.h
@@ -0,0 +1,159 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Sean Bright
+ *
+ * Sean Bright <sean.bright at gmail.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.
+ */
+
+#ifndef ASTERISK_ALERTPIPE_H
+#define ASTERISK_ALERTPIPE_H
+
+#include "asterisk/utils.h"
+
+typedef enum {
+	AST_ALERT_READ_SUCCESS = 0,
+	AST_ALERT_NOT_READABLE,
+	AST_ALERT_READ_FAIL,
+	AST_ALERT_READ_FATAL,
+} ast_alert_status_t;
+
+/*!
+ * \brief Initialize an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array to hold the alert pipe's file descriptors
+ *
+ * \return non-zero if a failure occurred, zero otherwise.
+ */
+int ast_alertpipe_init(int alert_pipe[2]);
+
+/*!
+ * \brief Close an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element containing the alert pipe's file descriptors
+ */
+void ast_alertpipe_close(int alert_pipe[2]);
+
+/*!
+ * \brief Read an event from an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \retval AST_ALERT_READ_SUCCESS on success
+ * \retval AST_ALERT_NOT_READABLE if the alert pipe is not readable
+ * \retval AST_ALERT_READ_FATAL if the alert pipe's file descriptors are in
+ *         blocking mode, or a read error occurs.
+ */
+ast_alert_status_t ast_alertpipe_read(int alert_pipe[2]);
+
+/*!
+ * \brief Write an event to an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return see write(2)
+ */
+ssize_t ast_alertpipe_write(int alert_pipe[2]);
+
+/*!
+ * \brief Consume all alerts written to the alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \retval AST_ALERT_READ_SUCCESS on success
+ * \retval AST_ALERT_NOT_READABLE if the alert pipe is not readable
+ * \retval AST_ALERT_READ_FATAL if the alert pipe's file descriptors are in
+ *         blocking mode, or a read error occurs.
+ */
+ast_alert_status_t ast_alertpipe_flush(int alert_pipe[2]);
+
+/*!
+ * \brief Sets the alert pipe file descriptors to default values
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ */
+AST_INLINE_API(
+void ast_alertpipe_clear(int alert_pipe[2]),
+{
+	alert_pipe[0] = alert_pipe[1] = -1;
+}
+)
+
+/*!
+ * \brief Determine if the alert pipe is readable
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return non-zero if the alert pipe is readable, zero otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_readable(int alert_pipe[2]),
+{
+	return alert_pipe[0] > -1;
+}
+)
+
+/*!
+ * \brief Determine if the alert pipe is writable
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return non-zero if the alert pipe is writable, zero otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_writable(int alert_pipe[2]),
+{
+	return alert_pipe[1] > -1;
+}
+)
+
+/*!
+ * \brief Get the alert pipe's read file descriptor
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return -1 if the file descriptor is not initialized, a non-negative value
+ *            otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_readfd(int alert_pipe[2]),
+{
+	return alert_pipe[0];
+}
+)
+
+/*!
+ * \brief Swap the file descriptors from two alert pipes
+ * \since 13.16.0
+ *
+ * \param p1 a two-element array containing an alert pipe's file descriptors
+ * \param p2 a two-element array containing an alert pipe's file descriptors
+ */
+AST_INLINE_API(
+void ast_alertpipe_swap(int alert_pipe_1[2], int alert_pipe_2[2]),
+{
+	SWAP(alert_pipe_1[0], alert_pipe_2[0]);
+	SWAP(alert_pipe_1[1], alert_pipe_2[1]);
+}
+)
+
+#endif /* ASTERISK_ALERTPIPE_H */
diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index 5f8a9d3..b39386b 100644
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -225,6 +225,10 @@
 /* Define to 1 if you have the `euidaccess' function. */
 #undef HAVE_EUIDACCESS
 
+/* Define to 1 if your system supports eventfd and the EFD_NONBLOCK and
+   EFD_SEMAPHORE flags. */
+#undef HAVE_EVENTFD
+
 /* Define to 1 if you have the `exp' function. */
 #undef HAVE_EXP
 
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 32c9c7f..3e04b5d 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -123,6 +123,7 @@
 #ifndef _ASTERISK_CHANNEL_H
 #define _ASTERISK_CHANNEL_H
 
+#include "asterisk/alertpipe.h"
 #include "asterisk/abstract_jb.h"
 #include "asterisk/astobj2.h"
 #include "asterisk/poll-compat.h"
@@ -4267,12 +4268,6 @@
 void ast_channel_named_pickupgroups_set(struct ast_channel *chan, struct ast_namedgroups *value);
 
 /* Alertpipe accessors--the "internal" functions for channel.c use only */
-typedef enum {
-	AST_ALERT_READ_SUCCESS = 0,
-	AST_ALERT_NOT_READABLE,
-	AST_ALERT_READ_FAIL,
-	AST_ALERT_READ_FATAL,
-} ast_alert_status_t;
 int ast_channel_alert_write(struct ast_channel *chan);
 int ast_channel_alert_writable(struct ast_channel *chan);
 ast_alert_status_t ast_channel_internal_alert_flush(struct ast_channel *chan);
diff --git a/main/alertpipe.c b/main/alertpipe.c
new file mode 100644
index 0000000..fa6ec7b
--- /dev/null
+++ b/main/alertpipe.c
@@ -0,0 +1,166 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Sean Bright
+ *
+ * Sean Bright <sean.bright at gmail.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 Alert Pipe API
+ *
+ * \author Sean Bright
+ */
+
+#include "asterisk.h"
+
+#include <unistd.h>
+#include <fcntl.h>
+
+#ifdef HAVE_EVENTFD
+# include <sys/eventfd.h>
+#endif
+
+#include "asterisk/alertpipe.h"
+#include "asterisk/logger.h"
+
+int ast_alertpipe_init(int alert_pipe[2])
+{
+#ifdef HAVE_EVENTFD
+
+	int fd = eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE);
+	if (fd > -1) {
+		alert_pipe[0] = alert_pipe[1] = fd;
+		return 0;
+	}
+
+	ast_log(LOG_WARNING, "Failed to create alert pipe with eventfd(), falling back to pipe(): %s\n",
+		strerror(errno));
+	ast_alertpipe_clear(alert_pipe);
+
+#endif
+
+	if (pipe(alert_pipe)) {
+		ast_log(LOG_WARNING, "Failed to create alert pipe: %s\n", strerror(errno));
+		return -1;
+	} else {
+		int flags = fcntl(alert_pipe[0], F_GETFL);
+		if (fcntl(alert_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
+			ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",
+				strerror(errno));
+			ast_alertpipe_close(alert_pipe);
+			return -1;
+		}
+		flags = fcntl(alert_pipe[1], F_GETFL);
+		if (fcntl(alert_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
+			ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",
+				strerror(errno));
+			ast_alertpipe_close(alert_pipe);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+void ast_alertpipe_close(int alert_pipe[2])
+{
+#ifdef HAVE_EVENTFD
+
+	if (alert_pipe[0] == alert_pipe[1]) {
+		if (alert_pipe[0] > -1) {
+			close(alert_pipe[0]);
+			ast_alertpipe_clear(alert_pipe);
+		}
+		return;
+	}
+
+#endif
+
+	if (alert_pipe[0] > -1) {
+		close(alert_pipe[0]);
+	}
+	if (alert_pipe[1] > -1) {
+		close(alert_pipe[1]);
+	}
+	ast_alertpipe_clear(alert_pipe);
+}
+
+ast_alert_status_t ast_alertpipe_read(int alert_pipe[2])
+{
+	uint64_t tmp;
+
+	if (!ast_alertpipe_readable(alert_pipe)) {
+		return AST_ALERT_NOT_READABLE;
+	}
+
+	if (read(alert_pipe[0], &tmp, sizeof(tmp)) < 0) {
+		if (errno != EINTR && errno != EAGAIN) {
+			ast_log(LOG_WARNING, "read() failed: %s\n", strerror(errno));
+			return AST_ALERT_READ_FAIL;
+		}
+	}
+
+	return AST_ALERT_READ_SUCCESS;
+}
+
+ssize_t ast_alertpipe_write(int alert_pipe[2])
+{
+	uint64_t tmp = 1;
+
+	if (!ast_alertpipe_writable(alert_pipe)) {
+		errno = EBADF;
+		return 0;
+	}
+
+	/* preset errno in case returned size does not match */
+	errno = EPIPE;
+	return write(alert_pipe[1], &tmp, sizeof(tmp)) != sizeof(tmp);
+}
+
+ast_alert_status_t ast_alertpipe_flush(int alert_pipe[2])
+{
+	int bytes_read;
+	uint64_t tmp[16];
+
+	if (!ast_alertpipe_readable(alert_pipe)) {
+		return AST_ALERT_NOT_READABLE;
+	}
+
+	/* Read the alertpipe until it is exhausted. */
+	for (;;) {
+		bytes_read = read(alert_pipe[0], tmp, sizeof(tmp));
+		if (bytes_read < 0) {
+			if (errno == EINTR) {
+				continue;
+			}
+			if (errno == EAGAIN || errno == EWOULDBLOCK) {
+				/*
+				 * Would block so nothing left to read.
+				 * This is the normal loop exit.
+				 */
+				break;
+			}
+			ast_log(LOG_WARNING, "read() failed flushing alertpipe: %s\n",
+				strerror(errno));
+			return AST_ALERT_READ_FAIL;
+		}
+		if (!bytes_read) {
+			/* Read nothing so we are done */
+			break;
+		}
+	}
+
+	return AST_ALERT_READ_SUCCESS;
+}
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 7f3c8fe..89222d3 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -35,6 +35,7 @@
 #include <signal.h>
 
 #include "asterisk/heap.h"
+#include "asterisk/alertpipe.h"
 #include "asterisk/astobj2.h"
 #include "asterisk/stringfields.h"
 #include "asterisk/app.h"
@@ -954,7 +955,6 @@
 int ast_bridge_channel_queue_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr)
 {
 	struct ast_frame *dup;
-	char nudge = 0;
 
 	if (bridge_channel->suspended
 		/* Also defer DTMF frames. */
@@ -983,7 +983,7 @@
 	}
 
 	AST_LIST_INSERT_TAIL(&bridge_channel->wr_queue, dup, frame_list);
-	if (write(bridge_channel->alert_pipe[1], &nudge, sizeof(nudge)) != sizeof(nudge)) {
+	if (ast_alertpipe_write(bridge_channel->alert_pipe)) {
 		ast_log(LOG_ERROR, "We couldn't write alert pipe for %p(%s)... something is VERY wrong\n",
 			bridge_channel, ast_channel_name(bridge_channel->chan));
 	}
@@ -2257,25 +2257,6 @@
 
 /*!
  * \internal
- * \param bridge_channel Channel to read wr_queue alert pipe.
- *
- * \return Nothing
- */
-static void bridge_channel_read_wr_queue_alert(struct ast_bridge_channel *bridge_channel)
-{
-	char nudge;
-
-	if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) {
-		if (errno != EINTR && errno != EAGAIN) {
-			ast_log(LOG_WARNING, "read() failed for alert pipe on %p(%s): %s\n",
-				bridge_channel, ast_channel_name(bridge_channel->chan),
-				strerror(errno));
-		}
-	}
-}
-
-/*!
- * \internal
  * \brief Handle bridge channel write frame to channel.
  * \since 12.0.0
  *
@@ -2296,7 +2277,7 @@
 		/* No frame, flush the alert pipe of excess alerts. */
 		ast_log(LOG_WARNING, "Weird.  No frame from bridge for %s to process?\n",
 			ast_channel_name(bridge_channel->chan));
-		bridge_channel_read_wr_queue_alert(bridge_channel);
+		ast_alertpipe_read(bridge_channel->alert_pipe);
 		ast_bridge_channel_unlock(bridge_channel);
 		return;
 	}
@@ -2312,7 +2293,7 @@
 				break;
 			}
 		}
-		bridge_channel_read_wr_queue_alert(bridge_channel);
+		ast_alertpipe_read(bridge_channel->alert_pipe);
 		AST_LIST_REMOVE_CURRENT(frame_list);
 		break;
 	}
@@ -2879,62 +2860,6 @@
 		&& AST_LIST_EMPTY(&bridge_channel->wr_queue);
 }
 
-/*!
- * \internal
- * \brief Close a pipe.
- * \since 12.0.0
- *
- * \param my_pipe What to close.
- *
- * \return Nothing
- */
-static void pipe_close(int *my_pipe)
-{
-	if (my_pipe[0] > -1) {
-		close(my_pipe[0]);
-		my_pipe[0] = -1;
-	}
-	if (my_pipe[1] > -1) {
-		close(my_pipe[1]);
-		my_pipe[1] = -1;
-	}
-}
-
-/*!
- * \internal
- * \brief Initialize a pipe as non-blocking.
- * \since 12.0.0
- *
- * \param my_pipe What to initialize.
- *
- * \retval 0 on success.
- * \retval -1 on error.
- */
-static int pipe_init_nonblock(int *my_pipe)
-{
-	int flags;
-
-	my_pipe[0] = -1;
-	my_pipe[1] = -1;
-	if (pipe(my_pipe)) {
-		ast_log(LOG_WARNING, "Can't create pipe! Try increasing max file descriptors with ulimit -n\n");
-		return -1;
-	}
-	flags = fcntl(my_pipe[0], F_GETFL);
-	if (fcntl(my_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
-		ast_log(LOG_WARNING, "Unable to set read pipe nonblocking! (%d: %s)\n",
-			errno, strerror(errno));
-		return -1;
-	}
-	flags = fcntl(my_pipe[1], F_GETFL);
-	if (fcntl(my_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
-		ast_log(LOG_WARNING, "Unable to set write pipe nonblocking! (%d: %s)\n",
-			errno, strerror(errno));
-		return -1;
-	}
-	return 0;
-}
-
 /* Destroy elements of the bridge channel structure and the bridge channel structure itself */
 static void bridge_channel_destroy(void *obj)
 {
@@ -2954,7 +2879,7 @@
 	while ((fr = AST_LIST_REMOVE_HEAD(&bridge_channel->wr_queue, frame_list))) {
 		bridge_frame_free(fr);
 	}
-	pipe_close(bridge_channel->alert_pipe);
+	ast_alertpipe_close(bridge_channel->alert_pipe);
 
 	ast_cond_destroy(&bridge_channel->cond);
 
@@ -2971,7 +2896,7 @@
 		return NULL;
 	}
 	ast_cond_init(&bridge_channel->cond, NULL);
-	if (pipe_init_nonblock(bridge_channel->alert_pipe)) {
+	if (ast_alertpipe_init(bridge_channel->alert_pipe)) {
 		ao2_ref(bridge_channel, -1);
 		return NULL;
 	}
diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index b3c0a48..d838ea8 100644
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -36,6 +36,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 
+#include "asterisk/alertpipe.h"
 #include "asterisk/paths.h"
 #include "asterisk/channel.h"
 #include "asterisk/channel_internal.h"
@@ -1265,152 +1266,52 @@
 /* Alertpipe functions */
 int ast_channel_alert_write(struct ast_channel *chan)
 {
-	char blah = 0x7F;
-
-	if (!ast_channel_alert_writable(chan)) {
-		errno = EBADF;
-		return 0;
-	}
-	/* preset errno in case returned size does not match */
-	errno = EPIPE;
-	return write(chan->alertpipe[1], &blah, sizeof(blah)) != sizeof(blah);
-}
-
-static int channel_internal_alert_check_nonblock(struct ast_channel *chan)
-{
-	int flags;
-
-	flags = fcntl(chan->alertpipe[0], F_GETFL);
-	/* For some odd reason, the alertpipe occasionally loses nonblocking status,
-	 * which immediately causes a deadlock scenario.  Detect and prevent this. */
-	if ((flags & O_NONBLOCK) == 0) {
-		ast_log(LOG_ERROR, "Alertpipe on channel %s lost O_NONBLOCK?!!\n", ast_channel_name(chan));
-		if (fcntl(chan->alertpipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
-			ast_log(LOG_WARNING, "Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
-			return -1;
-		}
-	}
-	return 0;
+	return ast_alertpipe_write(chan->alertpipe);
 }
 
 ast_alert_status_t ast_channel_internal_alert_flush(struct ast_channel *chan)
 {
-	int bytes_read;
-	char blah[100];
-
-	if (!ast_channel_internal_alert_readable(chan)) {
-		return AST_ALERT_NOT_READABLE;
-	}
-	if (channel_internal_alert_check_nonblock(chan)) {
-		return AST_ALERT_READ_FATAL;
-	}
-
-	/* Read the alertpipe until it is exhausted. */
-	for (;;) {
-		bytes_read = read(chan->alertpipe[0], blah, sizeof(blah));
-		if (bytes_read < 0) {
-			if (errno == EINTR) {
-				continue;
-			}
-			if (errno == EAGAIN || errno == EWOULDBLOCK) {
-				/*
-				 * Would block so nothing left to read.
-				 * This is the normal loop exit.
-				 */
-				break;
-			}
-			ast_log(LOG_WARNING, "read() failed flushing alertpipe: %s\n",
-				strerror(errno));
-			return AST_ALERT_READ_FAIL;
-		}
-		if (!bytes_read) {
-			/* Read nothing so we are done */
-			break;
-		}
-	}
-
-	return AST_ALERT_READ_SUCCESS;
+	return ast_alertpipe_flush(chan->alertpipe);
 }
 
 ast_alert_status_t ast_channel_internal_alert_read(struct ast_channel *chan)
 {
-	char blah;
-
-	if (!ast_channel_internal_alert_readable(chan)) {
-		return AST_ALERT_NOT_READABLE;
-	}
-	if (channel_internal_alert_check_nonblock(chan)) {
-		return AST_ALERT_READ_FATAL;
-	}
-
-	if (read(chan->alertpipe[0], &blah, sizeof(blah)) < 0) {
-		if (errno != EINTR && errno != EAGAIN) {
-			ast_log(LOG_WARNING, "read() failed: %s\n", strerror(errno));
-			return AST_ALERT_READ_FAIL;
-		}
-	}
-
-	return AST_ALERT_READ_SUCCESS;
+	return ast_alertpipe_read(chan->alertpipe);
 }
 
 int ast_channel_alert_writable(struct ast_channel *chan)
 {
-	return chan->alertpipe[1] > -1;
+	return ast_alertpipe_writable(chan->alertpipe);
 }
 
 int ast_channel_internal_alert_readable(struct ast_channel *chan)
 {
-	return chan->alertpipe[0] > -1;
+	return ast_alertpipe_readable(chan->alertpipe);
 }
 
 void ast_channel_internal_alertpipe_clear(struct ast_channel *chan)
 {
-	chan->alertpipe[0] = chan->alertpipe[1] = -1;
+	ast_alertpipe_clear(chan->alertpipe);
 }
 
 void ast_channel_internal_alertpipe_close(struct ast_channel *chan)
 {
-	if (ast_channel_internal_alert_readable(chan)) {
-		close(chan->alertpipe[0]);
-		chan->alertpipe[0] = -1;
-	}
-	if (ast_channel_alert_writable(chan)) {
-		close(chan->alertpipe[1]);
-		chan->alertpipe[1] = -1;
-	}
+	ast_alertpipe_close(chan->alertpipe);
 }
 
 int ast_channel_internal_alertpipe_init(struct ast_channel *chan)
 {
-	if (pipe(chan->alertpipe)) {
-		ast_log(LOG_WARNING, "Channel allocation failed: Can't create alert pipe! Try increasing max file descriptors with ulimit -n\n");
-		return -1;
-	} else {
-		int flags = fcntl(chan->alertpipe[0], F_GETFL);
-		if (fcntl(chan->alertpipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
-			ast_log(LOG_WARNING, "Channel allocation failed: Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
-			return -1;
-		}
-		flags = fcntl(chan->alertpipe[1], F_GETFL);
-		if (fcntl(chan->alertpipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
-			ast_log(LOG_WARNING, "Channel allocation failed: Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
-			return -1;
-		}
-	}
-	return 0;
+	return ast_alertpipe_init(chan->alertpipe);
 }
 
 int ast_channel_internal_alert_readfd(struct ast_channel *chan)
 {
-	return chan->alertpipe[0];
+	return ast_alertpipe_readfd(chan->alertpipe);
 }
 
 void ast_channel_internal_alertpipe_swap(struct ast_channel *chan1, struct ast_channel *chan2)
 {
-	int i;
-	for (i = 0; i < ARRAY_LEN(chan1->alertpipe); i++) {
-		SWAP(chan1->alertpipe[i], chan2->alertpipe[i]);
-	}
+	ast_alertpipe_swap(chan1->alertpipe, chan2->alertpipe);
 }
 
 /* file descriptor array accessors */
diff --git a/main/utils.c b/main/utils.c
index 2033664..c254db5 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -67,6 +67,9 @@
 #define AST_API_MODULE
 #include "asterisk/config.h"
 
+#define AST_API_MODULE
+#include "asterisk/alertpipe.h"
+
 static char base64[64];
 static char b2a[256];
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iff0fb1573e7f7a187d5211ddc60aa8f3da3edb1d
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list