<p>Sean Bright has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/7471">View Change</a></p><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/chan_sip.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/http.c<br>M main/manager.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>19 files changed, 83 insertions(+), 103 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/71/7471/1</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 d0fdf5c..a043d3d 100644<br>--- a/apps/app_ices.c<br>+++ b/apps/app_ices.c<br>@@ -115,7 +115,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>@@ -130,8 +129,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 b418b28..67b1e12 100644<br>--- a/channels/chan_phone.c<br>+++ b/channels/chan_phone.c<br>@@ -1193,7 +1193,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>@@ -1226,8 +1225,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/chan_sip.c b/channels/chan_sip.c<br>index 559e5c0..b8cc7bf 100644<br>--- a/channels/chan_sip.c<br>+++ b/channels/chan_sip.c<br>@@ -2948,14 +2948,7 @@<br> goto cleanup;<br> }<br> <br>- if ((flags = fcntl(tcptls_session->fd, F_GETFL)) == -1) {<br>- ast_log(LOG_ERROR, "error setting socket to non blocking mode, fcntl() failed: %s\n", strerror(errno));<br>- goto cleanup;<br>- }<br>-<br>- flags |= O_NONBLOCK;<br>- if (fcntl(tcptls_session->fd, F_SETFL, flags) == -1) {<br>- ast_log(LOG_ERROR, "error setting socket to non blocking mode, fcntl() failed: %s\n", strerror(errno));<br>+ if (ast_fd_set_flags(tcptls_session->fd, O_NONBLOCK)) {<br> goto cleanup;<br> }<br> <br>diff --git a/channels/vgrabbers.c b/channels/vgrabbers.c<br>index 45dced4..3deb32f 100644<br>--- a/channels/vgrabbers.c<br>+++ b/channels/vgrabbers.c<br>@@ -227,12 +227,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 f207b64..edf5038 100644<br>--- a/codecs/codec_dahdi.c<br>+++ b/codecs/codec_dahdi.c<br>@@ -615,7 +615,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>@@ -661,11 +660,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 03705b3..7d386b3 100644<br>--- a/include/asterisk/utils.h<br>+++ b/include/asterisk/utils.h<br>@@ -1142,4 +1142,38 @@<br> */<br> int ast_check_ipv6(void);<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), 1, __FILE__, __LINE__)<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), 0, __FILE__, __LINE__)<br>+<br>+int _ast_fd_set_flags(int fd, int flags, int set, const char *file, int lineno);<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 0abb360..297915d 100644<br>--- a/main/asterisk.c<br>+++ b/main/asterisk.c<br>@@ -1691,7 +1691,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>@@ -1729,8 +1728,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/http.c b/main/http.c<br>index d1a443a..e8d395b 100644<br>--- a/main/http.c<br>+++ b/main/http.c<br>@@ -1955,9 +1955,7 @@<br> }<br> <br> /* make sure socket is non-blocking */<br>- flags = fcntl(ser->fd, F_GETFL);<br>- flags |= O_NONBLOCK;<br>- fcntl(ser->fd, F_SETFL, flags);<br>+ ast_fd_set_flags(ser->fd, O_NONBLOCK);<br> <br> /* Setup HTTP worker private data to keep track of request body reading. */<br> ao2_cleanup(ser->private_data);<br>diff --git a/main/manager.c b/main/manager.c<br>index 5bc87d5..8105265 100644<br>--- a/main/manager.c<br>+++ b/main/manager.c<br>@@ -6684,9 +6684,7 @@<br> }<br> <br> /* make sure socket is non-blocking */<br>- flags = fcntl(ser->fd, F_GETFL);<br>- flags |= O_NONBLOCK;<br>- fcntl(ser->fd, F_SETFL, flags);<br>+ ast_fd_set_flags(ser->fd, O_NONBLOCK);<br> <br> ao2_lock(session);<br> /* Hook to the tail of the event queue */<br>diff --git a/main/tcptls.c b/main/tcptls.c<br>index ef22094..3c2d678 100644<br>--- a/main/tcptls.c<br>+++ b/main/tcptls.c<br>@@ -805,7 +805,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>@@ -843,8 +843,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> tcptls_session->fd = fd;<br> tcptls_session->parent = desc;<br> ast_sockaddr_copy(&tcptls_session->remote_address, &addr);<br>@@ -1061,7 +1060,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>@@ -1075,8 +1073,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>@@ -1164,7 +1161,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> <br>@@ -1267,8 +1263,7 @@<br> ast_log(LOG_ERROR, "Unable to listen for %s!\n", desc->name);<br> goto error;<br> }<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 a568cd1..44e9eb9 100644<br>--- a/main/udptl.c<br>+++ b/main/udptl.c<br>@@ -1014,7 +1014,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>@@ -1045,8 +1044,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 0342030..705b965 100644<br>--- a/main/utils.c<br>+++ b/main/utils.c<br>@@ -2798,3 +2798,24 @@<br> }<br> return extra[0] - extra[1];<br> }<br>+<br>+int _ast_fd_set_flags(int fd, int flags, int set, const char *file, int lineno)<br>+{<br>+ int f;<br>+<br>+ f = fcntl(fd, F_GETFL);<br>+ if (f == -1) {<br>+ ast_log(LOG_ERROR, "%s:%d: Failed to get flags for file descriptor: %s\n",<br>+ file, lineno, strerror(errno));<br>+ return -1;<br>+ }<br>+<br>+ f = fcntl(fd, F_SETFL, set ? (f | flags) : (f & ~flags));<br>+ if (f == -1) {<br>+ ast_log(LOG_ERROR, "%s:%d: Failed to set flags for file descriptor: %s\n",<br>+ file, lineno, 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 4caa13b..f19303f 100644<br>--- a/res/res_agi.c<br>+++ b/res/res_agi.c<br>@@ -2048,7 +2048,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>@@ -2078,14 +2078,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>@@ -2251,9 +2244,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 75a6eba..a65fc8a 100644<br>--- a/res/res_http_websocket.c<br>+++ b/res/res_http_websocket.c<br>@@ -445,19 +445,7 @@<br> <br> int AST_OPTIONAL_API_NAME(ast_websocket_set_nonblock)(struct ast_websocket *session)<br> {<br>- int flags;<br>-<br>- if ((flags = fcntl(session->fd, F_GETFL)) == -1) {<br>- return -1;<br>- }<br>-<br>- flags |= O_NONBLOCK;<br>-<br>- if ((flags = fcntl(session->fd, F_SETFL, flags)) == -1) {<br>- return -1;<br>- }<br>-<br>- return 0;<br>+ return ast_fd_set_flags(session->fd, O_NONBLOCK);<br> }<br> <br> int AST_OPTIONAL_API_NAME(ast_websocket_set_timeout)(struct ast_websocket *session, int timeout)<br>@@ -944,17 +932,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 d791516..dee9c2d 100644<br>--- a/res/res_musiconhold.c<br>+++ b/res/res_musiconhold.c<br>@@ -924,7 +924,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>@@ -936,10 +935,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 037e533..6a3ab78 100644<br>--- a/res/res_pktccops.c<br>+++ b/res/res_pktccops.c<br>@@ -676,8 +676,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 5271d1b..c921f36 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -2765,8 +2765,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 1e76720..0654059 100644<br>--- a/res/res_timing_pthread.c<br>+++ b/res/res_timing_pthread.c<br>@@ -132,9 +132,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/7471">change 7471</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/7471"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I8b81901e1b1bd537ca632567cdb408931c6eded7 </div>
<div style="display:none"> Gerrit-Change-Number: 7471 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>