utils: Add convenience function for setting fd flags

There are many places in the code base where we ignore the return value
of fcntl() when getting/setting file descriptior flags. This patch
introduces a convenience function that allows setting or clearing file
descriptor flags and will also log an error on failure for later
analysis.

Change-Id: I8b81901e1b1bd537ca632567cdb408931c6eded7

6 files changed, 43 insertions, 27 deletions

diff --git a/main/alertpipe.c b/main/alertpipe.c
index fa6ec7bcc..7932a7346 100644
--- a/main/alertpipe.c
+++ b/main/alertpipe.c
@@ -55,17 +55,8 @@ int ast_alertpipe_init(int alert_pipe[2])
 		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));
+		if (ast_fd_set_flags(alert_pipe[0], O_NONBLOCK)
+		   || ast_fd_set_flags(alert_pipe[1], O_NONBLOCK)) {
 			ast_alertpipe_close(alert_pipe);
 			return -1;
 		}
diff --git a/main/asterisk.c b/main/asterisk.c
index faac569c0..187a8ad6b 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1569,7 +1569,6 @@ static void *listener(void *unused)
 	int s;
 	socklen_t len;
 	int x;
-	int flags;
 	struct pollfd fds[1];
 	for (;;) {
 		if (ast_socket < 0)
@@ -1607,8 +1606,7 @@ static void *listener(void *unused)
 						close(s);
 						break;
 					}
-					flags = fcntl(consoles[x].p[1], F_GETFL);
-					fcntl(consoles[x].p[1], F_SETFL, flags | O_NONBLOCK);
+					ast_fd_set_flags(consoles[x].p[1], O_NONBLOCK);
 					consoles[x].mute = 1; /* Default is muted, we will un-mute if necessary */
 					/* Default uid and gid to -2, so then in cli.c/cli_has_permissions() we will be able
 					   to know if the user didn't send the credentials. */
diff --git a/main/iostream.c b/main/iostream.c
index d91863319..aaa74fae1 100644
--- a/main/iostream.c
+++ b/main/iostream.c
@@ -77,7 +77,7 @@ int ast_iostream_get_fd(struct ast_iostream *stream)
 
 void ast_iostream_nonblock(struct ast_iostream *stream)
 {
-	fcntl(stream->fd, F_SETFL, fcntl(stream->fd, F_GETFL) | O_NONBLOCK);
+	ast_fd_set_flags(stream->fd, O_NONBLOCK);
 }
 
 SSL *ast_iostream_get_ssl(struct ast_iostream *stream)
diff --git a/main/tcptls.c b/main/tcptls.c
index a6d0538af..02a2af5c6 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -223,7 +223,7 @@ void *ast_tcptls_server_root(void *data)
 	pthread_t launched;
 
 	for (;;) {
-		int i, flags;
+		int i;
 
 		if (desc->periodic_fn) {
 			desc->periodic_fn(desc);
@@ -261,8 +261,7 @@ void *ast_tcptls_server_root(void *data)
 			close(fd);
 			continue;
 		}
-		flags = fcntl(fd, F_GETFL);
-		fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
+		ast_fd_clear_flags(fd, O_NONBLOCK);
 
 		tcptls_session->stream = ast_iostream_from_fd(&fd);
 		if (!tcptls_session->stream) {
@@ -514,7 +513,6 @@ void ast_ssl_teardown(struct ast_tls_config *cfg)
 struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session)
 {
 	struct ast_tcptls_session_args *desc;
-	int flags;
 
 	if (!(desc = tcptls_session->parent)) {
 		goto client_start_error;
@@ -528,8 +526,7 @@ struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_se
 		goto client_start_error;
 	}
 
-	flags = fcntl(desc->accept_fd, F_GETFL);
-	fcntl(desc->accept_fd, F_SETFL, flags & ~O_NONBLOCK);
+	ast_fd_clear_flags(desc->accept_fd, O_NONBLOCK);
 
 	if (desc->tls_cfg) {
 		desc->tls_cfg->enabled = 1;
@@ -621,7 +618,6 @@ error:
 
 void ast_tcptls_server_start(struct ast_tcptls_session_args *desc)
 {
-	int flags;
 	int x = 1;
 	int tls_changed = 0;
 	int sd_socket;
@@ -740,8 +736,7 @@ void ast_tcptls_server_start(struct ast_tcptls_session_args *desc)
 	}
 
 systemd_socket_activation:
-	flags = fcntl(desc->accept_fd, F_GETFL);
-	fcntl(desc->accept_fd, F_SETFL, flags | O_NONBLOCK);
+	ast_fd_set_flags(desc->accept_fd, O_NONBLOCK);
 	if (ast_pthread_create_background(&desc->master, NULL, desc->accept_fn, desc)) {
 		ast_log(LOG_ERROR, "Unable to launch thread for %s on %s: %s\n",
 			desc->name,
diff --git a/main/udptl.c b/main/udptl.c
index 853e43c44..d982f6bcb 100644
--- a/main/udptl.c
+++ b/main/udptl.c
@@ -1012,7 +1012,6 @@ struct ast_udptl *ast_udptl_new_with_bindaddr(struct ast_sched_context *sched, s
 	int x;
 	int startplace;
 	int i;
-	long int flags;
 	RAII_VAR(struct udptl_config *, cfg, ao2_global_obj_ref(globals), ao2_cleanup);
 
 	if (!cfg || !cfg->general) {
@@ -1043,8 +1042,7 @@ struct ast_udptl *ast_udptl_new_with_bindaddr(struct ast_sched_context *sched, s
 		ast_log(LOG_WARNING, "Unable to allocate socket: %s\n", strerror(errno));
 		return NULL;
 	}
-	flags = fcntl(udptl->fd, F_GETFL);
-	fcntl(udptl->fd, F_SETFL, flags | O_NONBLOCK);
+	ast_fd_set_flags(udptl->fd, O_NONBLOCK);
 
 #ifdef SO_NO_CHECK
 	if (cfg->general->nochecksums)
diff --git a/main/utils.c b/main/utils.c
index dd7176295..a070da49f 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -2758,3 +2758,37 @@ int ast_compare_versions(const char *version1, const char *version2)
 	}
 	return extra[0] - extra[1];
 }
+
+int __ast_fd_set_flags(int fd, int flags, enum ast_fd_flag_operation op,
+	const char *file, int lineno, const char *function)
+{
+	int f;
+
+	f = fcntl(fd, F_GETFL);
+	if (f == -1) {
+		ast_log(__LOG_ERROR, file, lineno, function,
+			"Failed to get fcntl() flags for file descriptor: %s\n", strerror(errno));
+		return -1;
+	}
+
+	switch (op) {
+	case AST_FD_FLAG_SET:
+		f |= flags;
+		break;
+	case AST_FD_FLAG_CLEAR:
+		f &= ~flags;
+		break;
+	default:
+		ast_assert(0);
+		break;
+	}
+
+	f = fcntl(fd, F_SETFL, f);
+	if (f == -1) {
+		ast_log(__LOG_ERROR, file, lineno, function,
+			"Failed to set fcntl() flags for file descriptor: %s\n", strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}