>From 5b219f1196ca9500ad2d7b1c3a9a508f8f06f4b0 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 1 Dec 2014 12:06:00 +0100 Subject: [PATCH 2/5] TESTS: Cover child_common.c with unit tests The module wasn't tested properly, which made it harder to patch it. --- Makefile.am | 3 + src/tests/cmocka/dummy_child.c | 29 ++++ src/tests/cmocka/test_child_common.c | 299 +++++++++++++++++++++++++++++++++++ src/util/child_common.c | 19 ++- src/util/child_common.h | 7 - 5 files changed, 344 insertions(+), 13 deletions(-) diff --git a/Makefile.am b/Makefile.am index 85699d11ed85d4af7b9be5905271e6ce2eba62eb..1a87561bf1e6612ba384fff4bcbacd83184a17a6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2170,6 +2170,9 @@ test_child_common_CFLAGS = \ $(AM_CFLAGS) \ -DCHILD_DIR=\"$(builddir)\" \ $(NULL) +test_child_common_LDFLAGS = \ + -Wl,-wrap,child_io_destructor \ + $(NULL) test_child_common_LDADD = \ $(CMOCKA_LIBS) \ $(POPT_LIBS) \ diff --git a/src/tests/cmocka/dummy_child.c b/src/tests/cmocka/dummy_child.c index 057fa6450b51cbac589e64553bb0692cbbfac655..e2c7fd30eff65c7cb17bd5cff429593f95ccc8a8 100644 --- a/src/tests/cmocka/dummy_child.c +++ b/src/tests/cmocka/dummy_child.c @@ -35,6 +35,10 @@ int main(int argc, const char *argv[]) int opt; int debug_fd = -1; poptContext pc; + ssize_t len; + ssize_t written; + errno_t ret; + uint8_t buf[IN_BUF_SIZE]; const char *action = NULL; const char *guitar; const char *drums; @@ -80,6 +84,31 @@ int main(int argc, const char *argv[]) DEBUG(SSSDBG_CRIT_FAILURE, "This band sounds weird\n"); _exit(1); } + } else if (strcasecmp(action, "echo") == 0) { + errno = 0; + len = sss_atomic_read_s(STDIN_FILENO, buf, IN_BUF_SIZE); + if (len == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, "read failed [%d][%s].\n", ret, strerror(ret)); + _exit(1); + } + close(STDIN_FILENO); + + errno = 0; + written = sss_atomic_write_s(STDOUT_FILENO, buf, len); + if (written == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, "write failed [%d][%s].\n", ret, + strerror(ret)); + _exit(1); + } + close(STDOUT_FILENO); + + if (written != len) { + DEBUG(SSSDBG_CRIT_FAILURE, "Expected to write %zu bytes, wrote %zu\n", + len, written); + _exit(1); + } } } diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c index ee6e297f58b0895b5401464bb3f9b5386a3e6411..cf64744269fa0da8f9acfc3ab26de13885d808d1 100644 --- a/src/tests/cmocka/test_child_common.c +++ b/src/tests/cmocka/test_child_common.c @@ -31,6 +31,8 @@ #define TEST_BIN "dummy-child" #define ECHO_STR "Hello child" +static int destructor_called; + struct child_test_ctx { int pipefd_to_child[2]; int pipefd_from_child[2]; @@ -153,6 +155,291 @@ void test_exec_child_extra_args(void **state) } } +struct tevent_req *echo_child_write_send(TALLOC_CTX *mem_ctx, + struct child_test_ctx *child_tctx, + struct child_io_fds *io_fds, + const char *input); +static void echo_child_write_done(struct tevent_req *subreq); +static void echo_child_read_done(struct tevent_req *subreq); + +int __real_child_io_destructor(void *ptr); + +int __wrap_child_io_destructor(void *ptr) +{ + destructor_called = 1; + return __real_child_io_destructor(ptr); +} + +/* Test that writing to the pipes works as expected */ +void test_exec_child_io_destruct(void **state) +{ + struct child_test_ctx *child_tctx = talloc_get_type(*state, + struct child_test_ctx); + struct child_io_fds *io_fds; + + io_fds = talloc(child_tctx, struct child_io_fds); + io_fds->read_from_child_fd = -1; + io_fds->write_to_child_fd = -1; + assert_non_null(io_fds); + talloc_set_destructor((void *) io_fds, child_io_destructor); + + io_fds->read_from_child_fd = child_tctx->pipefd_from_child[0]; + io_fds->write_to_child_fd = child_tctx->pipefd_to_child[1]; + + destructor_called = 0; + talloc_free(io_fds); + assert_int_equal(destructor_called, 1); + + errno = 0; + close(child_tctx->pipefd_from_child[0]); + assert_int_equal(errno, EBADF); + + errno = 0; + close(child_tctx->pipefd_from_child[1]); + assert_int_equal(errno, 0); + + errno = 0; + close(child_tctx->pipefd_to_child[0]); + assert_int_equal(errno, 0); + + errno = 0; + close(child_tctx->pipefd_to_child[1]); + assert_int_equal(errno, EBADF); +} + +void test_child_cb(int child_status, + struct tevent_signal *sige, + void *pvt); + +/* Test that writing to the pipes works as expected */ +void test_exec_child_handler(void **state) +{ + errno_t ret; + pid_t child_pid; + struct child_test_ctx *child_tctx = talloc_get_type(*state, + struct child_test_ctx); + struct sss_child_ctx_old *child_old_ctx; + + ret = unsetenv("TEST_CHILD_ACTION"); + assert_int_equal(ret, 0); + + child_pid = fork(); + assert_int_not_equal(child_pid, -1); + if (child_pid == 0) { + ret = exec_child(child_tctx, + child_tctx->pipefd_to_child, + child_tctx->pipefd_from_child, + CHILD_DIR"/"TEST_BIN, 2, NULL); + assert_int_equal(ret, EOK); + } + + ret = child_handler_setup(child_tctx->test_ctx->ev, child_pid, + test_child_cb, child_tctx, &child_old_ctx); + assert_int_equal(ret, EOK); + + ret = test_ev_loop(child_tctx->test_ctx); + assert_int_equal(ret, EOK); + assert_int_equal(child_tctx->test_ctx->error, 0); +} + +void test_child_cb(int child_status, + struct tevent_signal *sige, + void *pvt) +{ + struct child_test_ctx *child_ctx = talloc_get_type(pvt, struct child_test_ctx); + + child_ctx->test_ctx->error = EIO; + if (WIFEXITED(child_status) && WEXITSTATUS(child_status) == 0) { + child_ctx->test_ctx->error = 0; + } + + child_ctx->test_ctx->done = true; +} + +/* Test that writing to the pipes works as expected */ +void test_exec_child_echo(void **state) +{ + errno_t ret; + pid_t child_pid; + struct child_test_ctx *child_tctx = talloc_get_type(*state, + struct child_test_ctx); + struct tevent_req *req; + struct child_io_fds *io_fds; + + setenv("TEST_CHILD_ACTION", "echo", 1); + + io_fds = talloc(child_tctx, struct child_io_fds); + assert_non_null(io_fds); + io_fds->read_from_child_fd = -1; + io_fds->write_to_child_fd = -1; + talloc_set_destructor((void *) io_fds, child_io_destructor); + + child_pid = fork(); + assert_int_not_equal(child_pid, -1); + if (child_pid == 0) { + ret = exec_child(child_tctx, + child_tctx->pipefd_to_child, + child_tctx->pipefd_from_child, + CHILD_DIR"/"TEST_BIN, 2, NULL); + assert_int_equal(ret, EOK); + } + + DEBUG(SSSDBG_FUNC_DATA, "Forked into %d\n", child_pid); + + io_fds->read_from_child_fd = child_tctx->pipefd_from_child[0]; + close(child_tctx->pipefd_from_child[1]); + 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); + + ret = child_handler_setup(child_tctx->test_ctx->ev, child_pid, + NULL, NULL, NULL); + assert_int_equal(ret, EOK); + + req = echo_child_write_send(child_tctx, child_tctx, io_fds, ECHO_STR); + assert_non_null(req); + + ret = test_ev_loop(child_tctx->test_ctx); + talloc_free(io_fds); + assert_int_equal(ret, EOK); +} + +struct test_exec_echo_state { + struct child_io_fds *io_fds; + struct io_buffer buf; + struct child_test_ctx *child_test_ctx; +}; + +struct tevent_req *echo_child_write_send(TALLOC_CTX *mem_ctx, + struct child_test_ctx *child_tctx, + struct child_io_fds *io_fds, + const char *input) +{ + struct tevent_req *req; + struct tevent_req *subreq; + struct test_exec_echo_state *echo_state; + + req = tevent_req_create(mem_ctx, &echo_state, struct test_exec_echo_state); + assert_non_null(req); + + echo_state->child_test_ctx = child_tctx; + + echo_state->buf.data = (unsigned char *) talloc_strdup(echo_state, input); + assert_non_null(echo_state->buf.data); + echo_state->buf.size = strlen(input) + 1; + echo_state->io_fds = io_fds; + + DEBUG(SSSDBG_TRACE_INTERNAL, "Writing..\n"); + subreq = write_pipe_send(child_tctx, child_tctx->test_ctx->ev, + echo_state->buf.data, echo_state->buf.size, + echo_state->io_fds->write_to_child_fd); + assert_non_null(subreq); + tevent_req_set_callback(subreq, echo_child_write_done, req); + + return req; +} + +static void echo_child_write_done(struct tevent_req *subreq) +{ + struct tevent_req *req; + struct test_exec_echo_state *echo_state; + errno_t ret; + + req = tevent_req_callback_data(subreq, struct tevent_req); + echo_state = tevent_req_data(req, struct test_exec_echo_state); + + ret = write_pipe_recv(subreq); + DEBUG(SSSDBG_TRACE_INTERNAL, "Writing OK\n"); + talloc_zfree(subreq); + assert_int_equal(ret, EOK); + + close(echo_state->io_fds->write_to_child_fd); + echo_state->io_fds->write_to_child_fd = -1; + + DEBUG(SSSDBG_TRACE_INTERNAL, "Reading..\n"); + subreq = read_pipe_send(echo_state, + echo_state->child_test_ctx->test_ctx->ev, + echo_state->io_fds->read_from_child_fd); + assert_non_null(subreq); + tevent_req_set_callback(subreq, echo_child_read_done, req); +} + +static void echo_child_read_done(struct tevent_req *subreq) +{ + struct tevent_req *req; + struct test_exec_echo_state *echo_state; + errno_t ret; + ssize_t len; + uint8_t *buf; + + req = tevent_req_callback_data(subreq, struct tevent_req); + echo_state = tevent_req_data(req, struct test_exec_echo_state); + + ret = read_pipe_recv(subreq, echo_state, &buf, &len); + talloc_zfree(subreq); + DEBUG(SSSDBG_TRACE_INTERNAL, "Reading OK\n"); + assert_int_equal(ret, EOK); + + close(echo_state->io_fds->read_from_child_fd); + echo_state->io_fds->read_from_child_fd = -1; + + assert_string_equal(buf, echo_state->buf.data); + echo_state->child_test_ctx->test_ctx->done = true; +} + +void sss_child_cb(int pid, int wait_status, void *pvt); + +/* Just make sure the exec works. The child does nothing but exits */ +void test_sss_child(void **state) +{ + errno_t ret; + pid_t child_pid; + struct child_test_ctx *child_tctx = talloc_get_type(*state, + struct child_test_ctx); + struct sss_sigchild_ctx *sc_ctx; + struct sss_child_ctx *sss_child; + + ret = unsetenv("TEST_CHILD_ACTION"); + assert_int_equal(ret, 0); + + ret = sss_sigchld_init(child_tctx, child_tctx->test_ctx->ev, &sc_ctx); + assert_int_equal(ret, EOK); + + child_pid = fork(); + assert_int_not_equal(child_pid, -1); + if (child_pid == 0) { + ret = exec_child(child_tctx, + child_tctx->pipefd_to_child, + child_tctx->pipefd_from_child, + CHILD_DIR"/"TEST_BIN, 2, NULL); + assert_int_equal(ret, EOK); + } + + ret = sss_child_register(child_tctx, sc_ctx, + child_pid, + sss_child_cb, + child_tctx, &sss_child); + assert_int_equal(ret, EOK); + + ret = test_ev_loop(child_tctx->test_ctx); + assert_int_equal(ret, EOK); + assert_int_equal(child_tctx->test_ctx->error, 0); +} + +void sss_child_cb(int pid, int wait_status, void *pvt) +{ + struct child_test_ctx *child_ctx = talloc_get_type(pvt, struct child_test_ctx); + + child_ctx->test_ctx->error = EIO; + if (WIFEXITED(wait_status) && WEXITSTATUS(wait_status) == 0) { + child_ctx->test_ctx->error = 0; + } + + child_ctx->test_ctx->done = true; +} + int main(int argc, const char *argv[]) { int rv; @@ -171,6 +458,18 @@ int main(int argc, const char *argv[]) unit_test_setup_teardown(test_exec_child_extra_args, child_test_setup, child_test_teardown), + unit_test_setup_teardown(test_exec_child_io_destruct, + child_test_setup, + child_test_teardown), + unit_test_setup_teardown(test_exec_child_handler, + child_test_setup, + child_test_teardown), + unit_test_setup_teardown(test_exec_child_echo, + child_test_setup, + child_test_teardown), + unit_test_setup_teardown(test_sss_child, + child_test_setup, + child_test_teardown), }; /* Set debug level to invalid value so we can deside if -d 0 was used. */ diff --git a/src/util/child_common.c b/src/util/child_common.c index 3b159f9d9a6144d348369ee39a5cca93fcd9c46e..ba63c48f73fa9b1153b5a1c729e1524b548dd4c4 100644 --- a/src/util/child_common.c +++ b/src/util/child_common.c @@ -46,6 +46,13 @@ struct sss_child_ctx { struct sss_sigchild_ctx *sigchld_ctx; }; +static void sss_child_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data); + errno_t sss_sigchld_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct sss_sigchild_ctx **child_ctx) @@ -177,12 +184,12 @@ static void sss_child_invoke_cb(struct tevent_context *ev, talloc_free(imm); } -void sss_child_handler(struct tevent_context *ev, - struct tevent_signal *se, - int signum, - int count, - void *siginfo, - void *private_data) +static void sss_child_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) { struct sss_sigchild_ctx *sigchld_ctx; struct tevent_immediate *imm; diff --git a/src/util/child_common.h b/src/util/child_common.h index 9124bc3fd602a9d8aa38cb31bf1feffa5989ff94..10753a1bbc882339fa9aec0f271c784a9cb0343c 100644 --- a/src/util/child_common.h +++ b/src/util/child_common.h @@ -68,13 +68,6 @@ errno_t sss_child_register(TALLOC_CTX *mem_ctx, void *pvt, struct sss_child_ctx **child_ctx); -void sss_child_handler(struct tevent_context *ev, - struct tevent_signal *se, - int signum, - int count, - void *siginfo, - void *private_data); - /* Callback to be invoked when a sigchld handler is called. * The tevent_signal * associated with the handler will be * freed automatically when this function returns. -- 1.9.3