<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7473">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">utils: Add convenience function for setting fd flags<br><br>There are many places in the code base where we ignore the return value<br>of fcntl() when getting/setting file descriptior flags. This patch<br>introduces a convenience function that allows setting or clearing file<br>descriptor flags and will also log an error on failure for later<br>analysis.<br><br>Change-Id: I8b81901e1b1bd537ca632567cdb408931c6eded7<br>---<br>M apps/app_ices.c<br>M channels/chan_phone.c<br>M channels/vgrabbers.c<br>M codecs/codec_dahdi.c<br>M include/asterisk/utils.h<br>M main/alertpipe.c<br>M main/asterisk.c<br>M main/iostream.c<br>M main/tcptls.c<br>M main/udptl.c<br>M main/utils.c<br>M res/res_agi.c<br>M res/res_http_websocket.c<br>M res/res_musiconhold.c<br>M res/res_pktccops.c<br>M res/res_rtp_asterisk.c<br>M res/res_timing_pthread.c<br>17 files changed, 100 insertions(+), 78 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/apps/app_ices.c b/apps/app_ices.c<br>index 4ca4b67..1194384 100644<br>--- a/apps/app_ices.c<br>+++ b/apps/app_ices.c<br>@@ -113,7 +113,6 @@<br> int fds[2];<br> int ms = -1;<br> int pid = -1;<br>- int flags;<br> struct ast_format *oreadformat;<br> struct ast_frame *f;<br> char filename[256]="";<br>@@ -128,8 +127,7 @@<br> ast_log(LOG_WARNING, "Unable to create pipe\n");<br> return -1;<br> }<br>- flags = fcntl(fds[1], F_GETFL);<br>- fcntl(fds[1], F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(fds[1], O_NONBLOCK);<br> <br> ast_stopstream(chan);<br> <br>diff --git a/channels/chan_phone.c b/channels/chan_phone.c<br>index 76891ac..832f28b 100644<br>--- a/channels/chan_phone.c<br>+++ b/channels/chan_phone.c<br>@@ -1191,7 +1191,6 @@<br> {<br> /* Make a phone_pvt structure for this interface */<br> struct phone_pvt *tmp;<br>- int flags; <br> <br> tmp = ast_calloc(1, sizeof(*tmp));<br> if (tmp) {<br>@@ -1224,8 +1223,7 @@<br> ioctl(tmp->fd, PHONE_VAD, tmp->silencesupression);<br> #endif<br> tmp->mode = mode;<br>- flags = fcntl(tmp->fd, F_GETFL);<br>- fcntl(tmp->fd, F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(tmp->fd, O_NONBLOCK);<br> tmp->owner = NULL;<br> ao2_cleanup(tmp->lastformat);<br> tmp->lastformat = NULL;<br>diff --git a/channels/vgrabbers.c b/channels/vgrabbers.c<br>index 2581740..169e59c 100644<br>--- a/channels/vgrabbers.c<br>+++ b/channels/vgrabbers.c<br>@@ -226,12 +226,8 @@<br> v->b = *geom;<br> b = &v->b; /* shorthand */<br> <br>- i = fcntl(fd, F_GETFL);<br>- if (-1 == fcntl(fd, F_SETFL, i | O_NONBLOCK)) {<br>- /* non fatal, just emit a warning */<br>- ast_log(LOG_WARNING, "error F_SETFL for %s [%s]\n",<br>- dev, strerror(errno));<br>- }<br>+ ast_fd_set_flags(fd, O_NONBLOCK);<br>+<br> /* set format for the camera.<br> * In principle we could retry with a different format if the<br> * one we are asking for is not supported.<br>diff --git a/codecs/codec_dahdi.c b/codecs/codec_dahdi.c<br>index efb0168..941bb1f 100644<br>--- a/codecs/codec_dahdi.c<br>+++ b/codecs/codec_dahdi.c<br>@@ -613,7 +613,6 @@<br> /* Request translation through zap if possible */<br> int fd;<br> struct codec_dahdi_pvt *dahdip = pvt->pvt;<br>- int flags;<br> int tried_once = 0;<br> const char *dev_filename = "/dev/dahdi/transcode";<br> <br>@@ -659,11 +658,7 @@<br> return -1;<br> }<br> <br>- flags = fcntl(fd, F_GETFL);<br>- if (flags > - 1) {<br>- if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))<br>- ast_log(LOG_WARNING, "Could not set non-block mode!\n");<br>- }<br>+ ast_fd_set_flags(fd, O_NONBLOCK);<br> <br> dahdip->fd = fd;<br> <br>diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h<br>index 0a12b1d..c6c3407 100644<br>--- a/include/asterisk/utils.h<br>+++ b/include/asterisk/utils.h<br>@@ -1141,4 +1141,44 @@<br> */<br> int ast_check_ipv6(void);<br> <br>+enum ast_fd_flag_operation {<br>+ AST_FD_FLAG_SET,<br>+ AST_FD_FLAG_CLEAR,<br>+};<br>+<br>+/*<br>+ * \brief Set flags on the given file descriptor<br>+ * \since 13.19<br>+ *<br>+ * If getting or setting flags of the given file descriptor fails, logs an<br>+ * error message.<br>+ *<br>+ * \param fd File descriptor to set flags on<br>+ * \param flags The flag(s) to set<br>+ *<br>+ * \return -1 on error<br>+ * \return 0 if successful<br>+ */<br>+#define ast_fd_set_flags(fd, flags) \<br>+ __ast_fd_set_flags((fd), (flags), AST_FD_FLAG_SET, __FILE__, __LINE__, __PRETTY_FUNCTION__)<br>+<br>+/*<br>+ * \brief Clear flags on the given file descriptor<br>+ * \since 13.19<br>+ *<br>+ * If getting or setting flags of the given file descriptor fails, logs an<br>+ * error message.<br>+ *<br>+ * \param fd File descriptor to clear flags on<br>+ * \param flags The flag(s) to clear<br>+ *<br>+ * \return -1 on error<br>+ * \return 0 if successful<br>+ */<br>+#define ast_fd_clear_flags(fd, flags) \<br>+ __ast_fd_set_flags((fd), (flags), AST_FD_FLAG_CLEAR, __FILE__, __LINE__, __PRETTY_FUNCTION__)<br>+<br>+int __ast_fd_set_flags(int fd, int flags, enum ast_fd_flag_operation op,<br>+ const char *file, int lineno, const char *function);<br>+<br> #endif /* _ASTERISK_UTILS_H */<br>diff --git a/main/alertpipe.c b/main/alertpipe.c<br>index fa6ec7b..7932a73 100644<br>--- a/main/alertpipe.c<br>+++ b/main/alertpipe.c<br>@@ -55,17 +55,8 @@<br> ast_log(LOG_WARNING, "Failed to create alert pipe: %s\n", strerror(errno));<br> return -1;<br> } else {<br>- int flags = fcntl(alert_pipe[0], F_GETFL);<br>- if (fcntl(alert_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {<br>- ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",<br>- strerror(errno));<br>- ast_alertpipe_close(alert_pipe);<br>- return -1;<br>- }<br>- flags = fcntl(alert_pipe[1], F_GETFL);<br>- if (fcntl(alert_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {<br>- ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",<br>- strerror(errno));<br>+ if (ast_fd_set_flags(alert_pipe[0], O_NONBLOCK)<br>+ || ast_fd_set_flags(alert_pipe[1], O_NONBLOCK)) {<br> ast_alertpipe_close(alert_pipe);<br> return -1;<br> }<br>diff --git a/main/asterisk.c b/main/asterisk.c<br>index bc7ac8b..5c658d0 100644<br>--- a/main/asterisk.c<br>+++ b/main/asterisk.c<br>@@ -1569,7 +1569,6 @@<br> int s;<br> socklen_t len;<br> int x;<br>- int flags;<br> struct pollfd fds[1];<br> for (;;) {<br> if (ast_socket < 0)<br>@@ -1607,8 +1606,7 @@<br> close(s);<br> break;<br> }<br>- flags = fcntl(consoles[x].p[1], F_GETFL);<br>- fcntl(consoles[x].p[1], F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(consoles[x].p[1], O_NONBLOCK);<br> consoles[x].mute = 1; /* Default is muted, we will un-mute if necessary */<br> /* Default uid and gid to -2, so then in cli.c/cli_has_permissions() we will be able<br> to know if the user didn't send the credentials. */<br>diff --git a/main/iostream.c b/main/iostream.c<br>index d918633..aaa74fa 100644<br>--- a/main/iostream.c<br>+++ b/main/iostream.c<br>@@ -77,7 +77,7 @@<br> <br> void ast_iostream_nonblock(struct ast_iostream *stream)<br> {<br>- fcntl(stream->fd, F_SETFL, fcntl(stream->fd, F_GETFL) | O_NONBLOCK);<br>+ ast_fd_set_flags(stream->fd, O_NONBLOCK);<br> }<br> <br> SSL *ast_iostream_get_ssl(struct ast_iostream *stream)<br>diff --git a/main/tcptls.c b/main/tcptls.c<br>index a6d0538..02a2af5 100644<br>--- a/main/tcptls.c<br>+++ b/main/tcptls.c<br>@@ -223,7 +223,7 @@<br> pthread_t launched;<br> <br> for (;;) {<br>- int i, flags;<br>+ int i;<br> <br> if (desc->periodic_fn) {<br> desc->periodic_fn(desc);<br>@@ -261,8 +261,7 @@<br> close(fd);<br> continue;<br> }<br>- flags = fcntl(fd, F_GETFL);<br>- fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);<br>+ ast_fd_clear_flags(fd, O_NONBLOCK);<br> <br> tcptls_session->stream = ast_iostream_from_fd(&fd);<br> if (!tcptls_session->stream) {<br>@@ -514,7 +513,6 @@<br> struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session)<br> {<br> struct ast_tcptls_session_args *desc;<br>- int flags;<br> <br> if (!(desc = tcptls_session->parent)) {<br> goto client_start_error;<br>@@ -528,8 +526,7 @@<br> goto client_start_error;<br> }<br> <br>- flags = fcntl(desc->accept_fd, F_GETFL);<br>- fcntl(desc->accept_fd, F_SETFL, flags & ~O_NONBLOCK);<br>+ ast_fd_clear_flags(desc->accept_fd, O_NONBLOCK);<br> <br> if (desc->tls_cfg) {<br> desc->tls_cfg->enabled = 1;<br>@@ -621,7 +618,6 @@<br> <br> void ast_tcptls_server_start(struct ast_tcptls_session_args *desc)<br> {<br>- int flags;<br> int x = 1;<br> int tls_changed = 0;<br> int sd_socket;<br>@@ -740,8 +736,7 @@<br> }<br> <br> systemd_socket_activation:<br>- flags = fcntl(desc->accept_fd, F_GETFL);<br>- fcntl(desc->accept_fd, F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(desc->accept_fd, O_NONBLOCK);<br> if (ast_pthread_create_background(&desc->master, NULL, desc->accept_fn, desc)) {<br> ast_log(LOG_ERROR, "Unable to launch thread for %s on %s: %s\n",<br> desc->name,<br>diff --git a/main/udptl.c b/main/udptl.c<br>index 853e43c..d982f6b 100644<br>--- a/main/udptl.c<br>+++ b/main/udptl.c<br>@@ -1012,7 +1012,6 @@<br> int x;<br> int startplace;<br> int i;<br>- long int flags;<br> RAII_VAR(struct udptl_config *, cfg, ao2_global_obj_ref(globals), ao2_cleanup);<br> <br> if (!cfg || !cfg->general) {<br>@@ -1043,8 +1042,7 @@<br> ast_log(LOG_WARNING, "Unable to allocate socket: %s\n", strerror(errno));<br> return NULL;<br> }<br>- flags = fcntl(udptl->fd, F_GETFL);<br>- fcntl(udptl->fd, F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(udptl->fd, O_NONBLOCK);<br> <br> #ifdef SO_NO_CHECK<br> if (cfg->general->nochecksums)<br>diff --git a/main/utils.c b/main/utils.c<br>index c553207..931a1ae 100644<br>--- a/main/utils.c<br>+++ b/main/utils.c<br>@@ -2749,3 +2749,37 @@<br> }<br> return extra[0] - extra[1];<br> }<br>+<br>+int __ast_fd_set_flags(int fd, int flags, enum ast_fd_flag_operation op,<br>+ const char *file, int lineno, const char *function)<br>+{<br>+ int f;<br>+<br>+ f = fcntl(fd, F_GETFL);<br>+ if (f == -1) {<br>+ ast_log(__LOG_ERROR, file, lineno, function,<br>+ "Failed to get fcntl() flags for file descriptor: %s\n", strerror(errno));<br>+ return -1;<br>+ }<br>+<br>+ switch (op) {<br>+ case AST_FD_FLAG_SET:<br>+ f |= flags;<br>+ break;<br>+ case AST_FD_FLAG_CLEAR:<br>+ f &= ~flags;<br>+ break;<br>+ default:<br>+ ast_assert(0);<br>+ break;<br>+ }<br>+<br>+ f = fcntl(fd, F_SETFL, f);<br>+ if (f == -1) {<br>+ ast_log(__LOG_ERROR, file, lineno, function,<br>+ "Failed to set fcntl() flags for file descriptor: %s\n", strerror(errno));<br>+ return -1;<br>+ }<br>+<br>+ return 0;<br>+}<br>diff --git a/res/res_agi.c b/res/res_agi.c<br>index 13af48f..85914c0 100644<br>--- a/res/res_agi.c<br>+++ b/res/res_agi.c<br>@@ -2046,7 +2046,7 @@<br> FastAGI defaults to port 4573 */<br> static enum agi_result launch_netscript(char *agiurl, char *argv[], int *fds)<br> {<br>- int s = 0, flags;<br>+ int s = 0;<br> char *host, *script;<br> int num_addrs = 0, i = 0;<br> struct ast_sockaddr *addrs;<br>@@ -2076,14 +2076,7 @@<br> continue;<br> }<br> <br>- if ((flags = fcntl(s, F_GETFL)) < 0) {<br>- ast_log(LOG_WARNING, "fcntl(F_GETFL) failed: %s\n", strerror(errno));<br>- close(s);<br>- continue;<br>- }<br>-<br>- if (fcntl(s, F_SETFL, flags | O_NONBLOCK) < 0) {<br>- ast_log(LOG_WARNING, "fnctl(F_SETFL) failed: %s\n", strerror(errno));<br>+ if (ast_fd_set_flags(s, O_NONBLOCK)) {<br> close(s);<br> continue;<br> }<br>@@ -2249,9 +2242,8 @@<br> close(toast[1]);<br> return AGI_RESULT_FAILURE;<br> }<br>- res = fcntl(audio[1], F_GETFL);<br>- if (res > -1)<br>- res = fcntl(audio[1], F_SETFL, res | O_NONBLOCK);<br>+<br>+ res = ast_fd_set_flags(audio[1], O_NONBLOCK);<br> if (res < 0) {<br> ast_log(LOG_WARNING, "unable to set audio pipe parameters: %s\n", strerror(errno));<br> close(fromast[0]);<br>diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c<br>index c1f9a29..baaa40f 100644<br>--- a/res/res_http_websocket.c<br>+++ b/res/res_http_websocket.c<br>@@ -951,17 +951,11 @@<br> /*! \brief Simple echo implementation which echoes received text and binary frames */<br> static void websocket_echo_callback(struct ast_websocket *session, struct ast_variable *parameters, struct ast_variable *headers)<br> {<br>- int flags, res;<br>+ int res;<br> <br> ast_debug(1, "Entering WebSocket echo loop\n");<br> <br>- if ((flags = fcntl(ast_websocket_fd(session), F_GETFL)) == -1) {<br>- goto end;<br>- }<br>-<br>- flags |= O_NONBLOCK;<br>-<br>- if (fcntl(ast_websocket_fd(session), F_SETFL, flags) == -1) {<br>+ if (ast_fd_set_flags(ast_websocket_fd(session), O_NONBLOCK)) {<br> goto end;<br> }<br> <br>diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c<br>index e4bb7a2..ef1b81c 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -922,7 +922,6 @@<br> static struct mohdata *mohalloc(struct mohclass *cl)<br> {<br> struct mohdata *moh;<br>- long flags;<br> <br> if (!(moh = ast_calloc(1, sizeof(*moh))))<br> return NULL;<br>@@ -934,10 +933,8 @@<br> }<br> <br> /* Make entirely non-blocking */<br>- flags = fcntl(moh->pipe[0], F_GETFL);<br>- fcntl(moh->pipe[0], F_SETFL, flags | O_NONBLOCK);<br>- flags = fcntl(moh->pipe[1], F_GETFL);<br>- fcntl(moh->pipe[1], F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(moh->pipe[0], O_NONBLOCK);<br>+ ast_fd_set_flags(moh->pipe[1], O_NONBLOCK);<br> <br> moh->f.frametype = AST_FRAME_VOICE;<br> moh->f.subclass.format = cl->format;<br>diff --git a/res/res_pktccops.c b/res/res_pktccops.c<br>index e8d266c..156c49d 100644<br>--- a/res/res_pktccops.c<br>+++ b/res/res_pktccops.c<br>@@ -648,7 +648,7 @@<br> <br> static int cops_connect(char *host, char *port)<br> {<br>- int s, sfd = -1, flags;<br>+ int s, sfd = -1;<br> struct addrinfo hints;<br> struct addrinfo *rp;<br> struct addrinfo *result;<br>@@ -674,8 +674,7 @@<br> if (sfd == -1) {<br> ast_log(LOG_WARNING, "Failed socket\n");<br> }<br>- flags = fcntl(sfd, F_GETFL);<br>- fcntl(sfd, F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(sfd, O_NONBLOCK);<br> #ifdef HAVE_SO_NOSIGPIPE<br> setsockopt(sfd, SOL_SOCKET, SO_NOSIGPIPE, &trueval, sizeof(trueval));<br> #endif<br>diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c<br>index 5aeb791..16eb7dd 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -3048,8 +3048,7 @@<br> }<br> ast_log(LOG_WARNING, "Unable to allocate %s socket: %s\n", type, strerror(errno));<br> } else {<br>- long flags = fcntl(sock, F_GETFL);<br>- fcntl(sock, F_SETFL, flags | O_NONBLOCK);<br>+ ast_fd_set_flags(sock, O_NONBLOCK);<br> #ifdef SO_NO_CHECK<br> if (nochecksums) {<br> setsockopt(sock, SOL_SOCKET, SO_NO_CHECK, &nochecksums, sizeof(nochecksums));<br>diff --git a/res/res_timing_pthread.c b/res/res_timing_pthread.c<br>index 09952f9..f520796 100644<br>--- a/res/res_timing_pthread.c<br>+++ b/res/res_timing_pthread.c<br>@@ -130,9 +130,7 @@<br> }<br> <br> for (i = 0; i < ARRAY_LEN(timer->pipe); ++i) {<br>- int flags = fcntl(timer->pipe[i], F_GETFL);<br>- flags |= O_NONBLOCK;<br>- fcntl(timer->pipe[i], F_SETFL, flags);<br>+ ast_fd_set_flags(timer->pipe[i], O_NONBLOCK);<br> }<br> <br> ao2_lock(pthread_timers);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7473">change 7473</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7473"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I8b81901e1b1bd537ca632567cdb408931c6eded7 </div>
<div style="display:none"> Gerrit-Change-Number: 7473 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>