From: Philipp Gesang Date: Thu, 4 Jan 2018 16:35:48 +0000 (+0100) Subject: redirect unused fds to /dev/null in pipestream X-Git-Tag: v2.10~1^2~8 X-Git-Url: http://developer.intra2net.com/git/?a=commitdiff_plain;h=ff5191e67b68c6ab74d126c6441922f636945eb1;p=libi2ncommon redirect unused fds to /dev/null in pipestream Closing an fd like stdout and stderr will cause EBADF on access which may not be handled in a child process. Thus redirect them to /dev/null if the user requests that their output be ignored. --- diff --git a/src/pipestream.cpp b/src/pipestream.cpp index 66ee39b..b9cdcf7 100644 --- a/src/pipestream.cpp +++ b/src/pipestream.cpp @@ -27,6 +27,7 @@ on this file might be covered by the GNU General Public License. #include #include #include +#include #include #include @@ -203,6 +204,34 @@ mk_argv (const std::vector &command) return boost::shared_array (ret); } +/** @brief Helper for redirecting a file descriptor to \c /dev/null. + * This will only acquire an fd the first time it is called + * or if it is called after unsuccessfully attempting to + * acquire one. + * + * @param fd The open file descriptor to operate on. + * + * @returns \c true on success, \c false otherwise (the call to + * either \c open(2) or \c dup2(2) failed). + */ +static bool +redirect_devnull (const int fd) +{ + static int nullfd = -1; + + errno = 0; + if (nullfd == -1 && (nullfd = open ("/dev/null", O_RDWR)) == -1) { + return false; + } + + errno = 0; + if (dup2 (nullfd, fd) == -1) { + return false; + } + + return true; +} + /** @brief Helper aggregating common code for the shell-free ctors. * * @param argv Argument list prepared for \c execve(2). @@ -242,14 +271,20 @@ inpipebuf::init_without_shell (const char *const *argv, close (pipefd [0]); if (!out) { - close (STDOUT_FILENO); + if (!redirect_devnull (STDOUT_FILENO)) { + fprintf(stderr, "redirect_devnull/stdout: %m\n"); + /* XXX should we bail here? */ + } } else if (dup2 (pipefd[1], STDOUT_FILENO) == -1) { fprintf(stderr, "dup2/stdout: %m\n"); exit(EXIT_FAILURE); } if (!err) { - close (STDERR_FILENO); + if (!redirect_devnull (STDERR_FILENO)) { + fprintf(stderr, "redirect_devnull/stderr: %m\n"); + /* XXX should we bail here? */ + } } else if (dup2 (pipefd[1], STDERR_FILENO) == -1) { fprintf(stderr, "dup2/stderr: %m\n"); exit(EXIT_FAILURE); diff --git a/test/test_pipestream.cpp b/test/test_pipestream.cpp index bf7b068..1b4bf8e 100644 --- a/test/test_pipestream.cpp +++ b/test/test_pipestream.cpp @@ -112,6 +112,7 @@ BOOST_AUTO_TEST_SUITE(pipestream) const std::string result = capture_exec (I2n::join_string (bad_command, " ")); + BOOST_CHECK(!exres.normal_exit); BOOST_CHECK_EQUAL(result.size (), 0); } @@ -122,6 +123,7 @@ BOOST_AUTO_TEST_SUITE(pipestream) ExecResult exres = ExecResult (); const std::string result = capture_exec (bad_command, exres); + BOOST_CHECK(exres.normal_exit); BOOST_CHECK(!exres.terminated_by_signal); BOOST_CHECK_EQUAL(result.size (), 0); } @@ -133,7 +135,9 @@ BOOST_AUTO_TEST_SUITE(pipestream) ExecResult exres = ExecResult (); const std::string result = capture_exec (bad_command, exres, false, true); + BOOST_CHECK(exres.normal_exit); BOOST_CHECK(!exres.terminated_by_signal); + BOOST_CHECK_EQUAL(exres.return_code, EXIT_FAILURE); BOOST_CHECK_EQUAL(result.size (), 0); } @@ -208,6 +212,85 @@ BOOST_AUTO_TEST_SUITE(pipestream) BOOST_CHECK_EQUAL(result.size (), 0); } + const char *const echo_abs = "/bin/echo"; + const char *const echo_rel = "echo"; + + static std::vector + mk_echo_argv (const std::string &text, const bool absolute=true) + { + std::vector ret; + + ret.push_back (absolute ? echo_abs : echo_rel); + ret.push_back ("-n"); + ret.push_back (text); + + return ret; + } + + BOOST_AUTO_TEST_CASE(abspath_echo_noshell_capture_ok) + { + ExecResult exres = ExecResult (); + const std::string text = "The significant owl hoots in the night."; + const std::vector argv = mk_echo_argv (text); + const std::string result = capture_exec (argv, exres, true, false, true); + + BOOST_CHECK(exres.normal_exit); + BOOST_CHECK_EQUAL(exres.return_code, EXIT_SUCCESS); + BOOST_CHECK_EQUAL(result, text); + } + + BOOST_AUTO_TEST_CASE(relpath_echo_noshell_capture_ok) + { + ExecResult exres = ExecResult (); + const std::string text = "Yet many grey lords go sadly to the masterless men."; + const std::vector argv = mk_echo_argv (text, false); + const std::string result = capture_exec (argv, exres, true, false, true); + + BOOST_CHECK(exres.normal_exit); + BOOST_CHECK_EQUAL(exres.return_code, EXIT_SUCCESS); + BOOST_CHECK_EQUAL(result, text); + } + + static std::vector + mk_errcho_argv (const std::string &text) + { + /* + * Hack cause there’s no way to make echo print to stderr without + * redirection. + */ + std::vector ret; + + ret.push_back ("/bin/sh"); + ret.push_back ("-c"); + ret.push_back (std::string ("1>&- 1>&2 echo -n '") + text + "'"); /* brr */ + + return ret; + } + + BOOST_AUTO_TEST_CASE(sh_errcho_capture_ok) + { + ExecResult exres = ExecResult (); + const std::string text = "Hooray, hooray for the spinster’s sister’s daughter."; + const std::vector argv = mk_errcho_argv (text); + const std::string result = capture_exec (argv, exres, false, true, true); + + BOOST_CHECK(exres.normal_exit); + BOOST_CHECK_EQUAL(exres.return_code, EXIT_SUCCESS); + BOOST_CHECK_EQUAL(result, text); + } + + BOOST_AUTO_TEST_CASE(sh_errcho_stdout_empty_ok) + { + ExecResult exres = ExecResult (); + const std::string text = "To the axeman, all supplicants are the same height."; + const std::vector argv = mk_errcho_argv (text); + const std::string result = capture_exec (argv, exres, true, false, true); + + BOOST_CHECK(exres.normal_exit); + BOOST_CHECK_EQUAL(exres.return_code, EXIT_SUCCESS); + BOOST_CHECK_EQUAL(result.size (), 0); + } + BOOST_AUTO_TEST_SUITE_END() /* [pipestream->read] */ BOOST_AUTO_TEST_SUITE_END() /* [pipestream] */