From f3d91181d4ee9da3f8bbf4ddf8782951c0ae46c1 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 8 Dec 2014 17:39:57 +0100 Subject: UTIL: Unify the fd_nonblocking implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The responder and child_common modules each had their own implementation. Unify it instead and add a unit test. Reviewed-by: Pavel Březina --- 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(-) (limited to 'src') diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 375ef1d8a..1ab40af0a 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 133b679b6..4286eb163 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 633cd9177..28ed0cb1c 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 72435cc74..afe6351e9 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 a5a444787..666abe610 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 7c36abdb5..11e383e52 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 08e8b8d26..94015d8e1 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 0afd3a617..b1af02337 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 2acb8604a..613c559bb 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 45efd1aef..60dbf9381 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 */ -- cgit