>From 8135a97b87ab01af7bc31aabbda16482f5417a50 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 8 Dec 2014 17:39:57 +0100 Subject: [PATCH 5/5] UTIL: Unify the fd_nonblocking implementation The responder and child_common modules each had their own implementation. Unify it instead and add a unit test. --- src/providers/ad/ad_gpo.c | 4 ++-- src/providers/ipa/ipa_selinux.c | 4 ++-- src/providers/krb5/krb5_child_handler.c | 4 ++-- src/providers/ldap/sdap_child_helpers.c | 4 ++-- src/responder/common/responder_common.c | 25 +------------------------ src/tests/cmocka/test_child_common.c | 4 ++-- src/tests/util-tests.c | 21 +++++++++++++++++++++ src/util/child_common.c | 23 ----------------------- src/util/util.c | 24 ++++++++++++++++++++++++ src/util/util.h | 12 ++++++++++++ 10 files changed, 68 insertions(+), 57 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 375ef1d8a7df13911831a55fed5d5a425daaa996..1ab40af0a465c46ef7fdcd63b6c322db2889274f 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -3974,8 +3974,8 @@ gpo_fork_child(struct tevent_req *req) close(pipefd_from_child[1]); state->io->write_to_child_fd = pipefd_to_child[1]; close(pipefd_to_child[0]); - fd_nonblocking(state->io->read_from_child_fd); - fd_nonblocking(state->io->write_to_child_fd); + sss_fd_nonblocking(state->io->read_from_child_fd); + sss_fd_nonblocking(state->io->write_to_child_fd); ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL); if (ret != EOK) { diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c index 133b679b6d518704ebb2bd901c64ac48170c9a0b..4286eb1630f16d4b43e8fd83a0e1dcf401d0a3d1 100644 --- a/src/providers/ipa/ipa_selinux.c +++ b/src/providers/ipa/ipa_selinux.c @@ -1058,8 +1058,8 @@ static errno_t selinux_fork_child(struct selinux_child_state *state) close(pipefd_from_child[1]); state->io->write_to_child_fd = pipefd_to_child[1]; close(pipefd_to_child[0]); - fd_nonblocking(state->io->read_from_child_fd); - fd_nonblocking(state->io->write_to_child_fd); + sss_fd_nonblocking(state->io->read_from_child_fd); + sss_fd_nonblocking(state->io->write_to_child_fd); ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL); if (ret != EOK) { diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c index 633cd917737d3f39526b049cc3d930b67f8b5c66..28ed0cb1c3daf54b629d0b88fb35d50f72aa0f39 100644 --- a/src/providers/krb5/krb5_child_handler.c +++ b/src/providers/krb5/krb5_child_handler.c @@ -320,8 +320,8 @@ static errno_t fork_child(struct tevent_req *req) close(pipefd_from_child[1]); state->io->write_to_child_fd = pipefd_to_child[1]; close(pipefd_to_child[0]); - fd_nonblocking(state->io->read_from_child_fd); - fd_nonblocking(state->io->write_to_child_fd); + sss_fd_nonblocking(state->io->read_from_child_fd); + sss_fd_nonblocking(state->io->write_to_child_fd); ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL); if (ret != EOK) { diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c index 72435cc74d8021c0713a94b4fe4e23762564da2c..afe6351e96b8cdfe6926334f044a140510457e41 100644 --- a/src/providers/ldap/sdap_child_helpers.c +++ b/src/providers/ldap/sdap_child_helpers.c @@ -108,8 +108,8 @@ static errno_t sdap_fork_child(struct tevent_context *ev, close(pipefd_from_child[1]); child->io->write_to_child_fd = pipefd_to_child[1]; close(pipefd_to_child[0]); - fd_nonblocking(child->io->read_from_child_fd); - fd_nonblocking(child->io->write_to_child_fd); + sss_fd_nonblocking(child->io->read_from_child_fd); + sss_fd_nonblocking(child->io->write_to_child_fd); ret = child_handler_setup(ev, pid, NULL, NULL, NULL); if (ret != EOK) { diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index a5a44478759e0d515e8437c3bb9bcd78dbb1e4f5..666abe6101c308d8f05ecea299af9146384042b4 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -22,8 +22,6 @@ #include "config.h" #include -#include -#include #include #include #include @@ -45,27 +43,6 @@ #include "monitor/monitor_interfaces.h" #include "sbus/sbus_client.h" -static errno_t set_nonblocking(int fd) -{ - int v; - int ferr; - errno_t error; - - /* Get the current flags for this file descriptor */ - v = fcntl(fd, F_GETFL, 0); - - errno = 0; - /* Set the non-blocking flag on this fd */ - ferr = fcntl(fd, F_SETFL, v | O_NONBLOCK); - if (ferr < 0) { - error = errno; - DEBUG(SSSDBG_FATAL_FAILURE, "Unable to set fd non-blocking: [%d][%s]\n", - error, strerror(error)); - return error; - } - return EOK; -} - static errno_t set_close_on_exec(int fd) { int v; @@ -601,7 +578,7 @@ int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval) orig_umaskval = umask(umaskval); - ret = set_nonblocking(fd); + ret = sss_fd_nonblocking(fd); if (ret != EOK) { goto done; } diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c index 7c36abdb5ae2cbc8b1813b549df298a603815af6..11e383e52f16bc114b1b82380d7dd638c6269f1a 100644 --- a/src/tests/cmocka/test_child_common.c +++ b/src/tests/cmocka/test_child_common.c @@ -293,8 +293,8 @@ void test_exec_child_echo(void **state) io_fds->write_to_child_fd = child_tctx->pipefd_to_child[1]; close(child_tctx->pipefd_to_child[0]); - fd_nonblocking(io_fds->write_to_child_fd); - fd_nonblocking(io_fds->read_from_child_fd); + sss_fd_nonblocking(io_fds->write_to_child_fd); + sss_fd_nonblocking(io_fds->read_from_child_fd); ret = child_handler_setup(child_tctx->test_ctx->ev, child_pid, NULL, NULL, NULL); diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c index 08e8b8d263c46618a0cdfb2203684305fa6dddc6..94015d8e1e0efb143a4fea998f1b16db1e63365e 100644 --- a/src/tests/util-tests.c +++ b/src/tests/util-tests.c @@ -407,6 +407,26 @@ START_TEST(test_sss_filter_sanitize) } END_TEST +START_TEST(test_fd_nonblocking) +{ + int fd; + int flags; + errno_t ret; + + fd = open("/dev/null", O_RDONLY); + fail_unless(fd > 0); + + flags = fcntl(fd, F_GETFL, 0); + fail_if(flags & O_NONBLOCK); + + ret = sss_fd_nonblocking(fd); + fail_unless(ret == EOK); + flags = fcntl(fd, F_GETFL, 0); + fail_unless(flags & O_NONBLOCK); + close(fd); +} +END_TEST + START_TEST(test_size_t_overflow) { fail_unless(!SIZE_T_OVERFLOW(1, 1), "unexpected overflow"); @@ -1020,6 +1040,7 @@ Suite *util_suite(void) tcase_add_test (tc_util, test_check_ipv6_addr); tcase_add_test (tc_util, test_is_host_in_domain); tcase_add_test (tc_util, test_known_service); + tcase_add_test (tc_util, test_fd_nonblocking); tcase_set_timeout(tc_util, 60); TCase *tc_utf8 = tcase_create("utf8"); diff --git a/src/util/child_common.c b/src/util/child_common.c index 0afd3a6176d6fcf0cefa6c711ac5ff5fc2d415b3..b1af02337e0bbab66e766dde7a33f3730689a20a 100644 --- a/src/util/child_common.c +++ b/src/util/child_common.c @@ -518,29 +518,6 @@ int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, return EOK; } -/* The pipes to communicate with the child must be nonblocking */ -void fd_nonblocking(int fd) -{ - int flags; - int ret; - - flags = fcntl(fd, F_GETFL, 0); - if (flags == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "F_GETFL failed [%d][%s].\n", ret, strerror(ret)); - return; - } - - if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "F_SETFL failed [%d][%s].\n", ret, strerror(ret)); - } - - return; -} - static void child_invoke_callback(struct tevent_context *ev, struct tevent_immediate *imm, void *pvt); diff --git a/src/util/util.c b/src/util/util.c index 2acb8604ac0c2bc7b83ee578c7bbead9a7fd44b3..613c559bb2002686c7833642d0946e46e5a9b5d6 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -880,3 +880,27 @@ done: return ret; } + +/* Set the nonblocking flag to the fd */ +errno_t sss_fd_nonblocking(int fd) +{ + int flags; + int ret; + + flags = fcntl(fd, F_GETFL, 0); + if (flags == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, + "F_GETFL failed [%d][%s].\n", ret, strerror(ret)); + return ret; + } + + if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, + "F_SETFL failed [%d][%s].\n", ret, strerror(ret)); + return ret; + } + + return EOK; +} diff --git a/src/util/util.h b/src/util/util.h index 45efd1aef94c2e058a435933e7c41adaecc676e2..60dbf9381c5fe26e7d42d51c6f9c7df6e3ebc4be 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -444,6 +445,17 @@ errno_t sss_hash_create_ex(TALLOC_CTX *mem_ctx, errno_t add_strings_lists(TALLOC_CTX *mem_ctx, const char **l1, const char **l2, bool copy_strings, char ***_new_list); +/** + * @brief set file descriptor as nonblocking + * + * Set the O_NONBLOCK flag for the input fd + * + * @param[in] fd The file descriptor to set as nonblocking + * + * @return EOK on success, errno code otherwise + */ +errno_t sss_fd_nonblocking(int fd); + /* Copy a NULL-terminated string list * Returns NULL on out of memory error or invalid input */ -- 2.1.0