diff --git a/common/threading/SubProcess.cpp b/common/threading/SubProcess.cpp
index 58aea7c565dc816bcc4180ede050ef070461353c..68ea251fbfae685eda2362ac057795e15ebd9177 100644
--- a/common/threading/SubProcess.cpp
+++ b/common/threading/SubProcess.cpp
@@ -23,6 +23,7 @@
#include "SubProcess.hpp"
#include "common/exception/Errnum.hpp"
+#include <memory>
#include <string.h>
#include <iostream>
#include <sys/signal.h>
@@ -98,14 +99,20 @@ SubProcess::SubProcess(const std::string & executable, const std::list<std::stri
ScopedPosixSpawnAttr attr;
cta::exception::Errnum::throwOnReturnedErrno(posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK),
"In Subprocess::Subprocess(): failed to posix_spawnattr_setflags()");
- char ** cargv = new char*[argv.size()+1];
- int index = 0;
- for (auto a=argv.cbegin(); a!=argv.cend(); a++) {
- cargv[index++] = ::strdup(a->c_str());
+ {
+ std::unique_ptr<char *[]> cargv (new char*[argv.size()+1]);
+ size_t index = 0;
+ std::list<std::unique_ptr<char, void (*)(char *)>> cargvStrings;
+ for (auto a=argv.cbegin(); a!=argv.cend(); a++) {
+ cargv[index] = ::strdup(a->c_str());
+ std::unique_ptr<char, void (*)(char *)> upStr (cargv[index], [](char *s){::free(s);});
+ cargvStrings.emplace_back(std::move(upStr));
+ index++;
+ }
+ cargv[argv.size()] = NULL;
+ int spawnRc=::posix_spawnp(&m_child, executable.c_str(), fileActions, attr, cargv.get(), ::environ);
+ cta::exception::Errnum::throwOnReturnedErrno(spawnRc, "In Subprocess::Subprocess failed to posix_spawn()");
}
- cargv[argv.size()] = NULL;
- int spawnRc=::posix_spawnp(&m_child, executable.c_str(), fileActions, attr, cargv, ::environ);
- cta::exception::Errnum::throwOnReturnedErrno(spawnRc, "In Subprocess::Subprocess failed to posix_spawn()");
// We are the parent process. Close the write sides of pipes.
::close(stdoutPipe[writeSide]);
::close(stderrPipe[writeSide]);
diff --git a/tapeserver/CMakeLists.txt b/tapeserver/CMakeLists.txt
index dab2e913625e5fb2c4a8243634e6972fe79488b6..d7aad618c080419b7245655ee20df7982d410cb6 100644
--- a/tapeserver/CMakeLists.txt
+++ b/tapeserver/CMakeLists.txt
@@ -19,8 +19,7 @@ add_library(cta-tapedSystemTests SHARED
cta-tapedSystemtests.cpp)
target_link_libraries(cta-tapedSystemTests
- systemTestHelper
unitTestHelper
ctacommon)
-install(TARGETS cta-tapedSystemTests DESTINATION usr/${CMAKE_INSTALL_LIBDIR})
\ No newline at end of file
+install(TARGETS cta-tapedSystemTests DESTINATION usr/${CMAKE_INSTALL_LIBDIR})
diff --git a/tapeserver/cta-tapedSystemtests.cpp b/tapeserver/cta-tapedSystemtests.cpp
index f868d5d06773e065707db0f7e2058244f91a4e32..df844e0d83d6f1f848e0dfa93d542b6aa7d57e42 100644
--- a/tapeserver/cta-tapedSystemtests.cpp
+++ b/tapeserver/cta-tapedSystemtests.cpp
@@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include "tests/Subprocess.hpp"
+#include "common/threading/SubProcess.hpp"
#include "tests/TempFile.hpp"
#include <gtest/gtest.h>
@@ -25,12 +25,12 @@ namespace systemTests {
TEST(cta_taped, InvocationTests) {
{
// Do we get help with -h or --help?
- Subprocess spHelpShort("cta-taped", std::list<std::string>({"cta-taped", "-h"}));
+ cta::threading::SubProcess spHelpShort("cta-taped", std::list<std::string>({"cta-taped", "-h"}));
spHelpShort.wait();
ASSERT_NE(std::string::npos, spHelpShort.stdout().find("Usage"));
ASSERT_TRUE(spHelpShort.stderr().empty());
ASSERT_EQ(EXIT_SUCCESS, spHelpShort.exitValue());
- Subprocess spHelpLong("cta-taped", std::list<std::string>({"cta-taped", "--help"}));
+ cta::threading::SubProcess spHelpLong("cta-taped", std::list<std::string>({"cta-taped", "--help"}));
spHelpLong.wait();
ASSERT_NE(std::string::npos, spHelpLong.stdout().find("Usage: cta-taped [options]"));
ASSERT_TRUE(spHelpLong.stderr().empty());
@@ -39,7 +39,7 @@ TEST(cta_taped, InvocationTests) {
{
// Do we get proper complaint when the configuration file is not there?
- Subprocess spNoConfigFile("cta-taped", std::list<std::string>({"cta-taped", "-f", "-s", "-c", "/no/such/file"}));
+ cta::threading::SubProcess spNoConfigFile("cta-taped", std::list<std::string>({"cta-taped", "-f", "-s", "-c", "/no/such/file"}));
spNoConfigFile.wait();
ASSERT_NE(std::string::npos, spNoConfigFile.stdout().find("Failed to open configuration file"));
ASSERT_TRUE(spNoConfigFile.stderr().empty());
@@ -58,11 +58,11 @@ TEST(cta_taped, InvocationTests) {
"taped BufferCount 1\n"
"taped TpConfigPath ");
ctaConf.stringAppend(tpConfig.path());
- Subprocess spNoDrive("cta-taped", std::list<std::string>({"cta-taped", "-f", "-s", "-c", ctaConf.path()}));
+ cta::threading::SubProcess spNoDrive("cta-taped", std::list<std::string>({"cta-taped", "-f", "-s", "-c", ctaConf.path()}));
spNoDrive.wait();
ASSERT_NE(std::string::npos, spNoDrive.stdout().find("MSG=\"Aborting\" Message=\"No drive found in configuration\""));
ASSERT_TRUE(spNoDrive.stderr().empty());
ASSERT_EQ(EXIT_FAILURE, spNoDrive.exitValue());
}
}
-}
\ No newline at end of file
+}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 33a81bee83343da0bdf6e50461cd9221a0c07dfb..2b810fd03dc9766031706e3f5c5abb6f5de006ba 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -75,14 +75,10 @@ target_link_libraries(cta-unitTests-multiProcess
add_library(unitTestHelper
TempFile.cpp)
-add_library(systemTestHelper
- Subprocess.cpp)
-
add_library(systemTestHelperTests SHARED
SubprocessSystemTests.cpp)
target_link_libraries(systemTestHelperTests
- systemTestHelper
ctacommon)
add_executable(cta-systemTests
@@ -90,7 +86,6 @@ add_executable(cta-systemTests
${GMOCK_SRC})
target_link_libraries(cta-systemTests
- systemTestHelper
systemTestHelperTests
cta-tapedSystemTests
gtest
diff --git a/tests/Subprocess.cpp b/tests/Subprocess.cpp
deleted file mode 100644
index 00b7ce0024f53919ac23eb98143f9018ce19bfa8..0000000000000000000000000000000000000000
--- a/tests/Subprocess.cpp
+++ /dev/null
@@ -1,157 +0,0 @@
-/*
- * The CERN Tape Archive (CTA) project
- * Copyright (C) 2015 CERN
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "Subprocess.hpp"
-#include "common/exception/Errnum.hpp"
-#include <string.h>
-#include <iostream>
-#include <sys/signal.h>
-#include <sys/wait.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <spawn.h>
-#include <memory>
-
-namespace {
-class ScopedPosixSpawnFileActions{
-public:
- ScopedPosixSpawnFileActions() {
- ::posix_spawn_file_actions_init(&m_action);
- }
- ~ScopedPosixSpawnFileActions() {
- ::posix_spawn_file_actions_destroy(&m_action);
- }
- operator ::posix_spawn_file_actions_t * () { return &m_action; }
-private:
- ::posix_spawn_file_actions_t m_action;
-};
-}
-
-namespace {
-class ScopedPosixSpawnAttr{
-public:
- ScopedPosixSpawnAttr() {
- ::posix_spawnattr_init(&m_attr);
- }
- ~ScopedPosixSpawnAttr() {
- ::posix_spawnattr_destroy(&m_attr);
- }
- operator ::posix_spawnattr_t * () { return &m_attr; }
-private:
- ::posix_spawnattr_t m_attr;
-};
-}
-
-namespace systemTests {
-Subprocess::Subprocess(const std::string & executable, const std::list<std::string>& argv) {
- // Sanity checks
- if (argv.size() < 1)
- throw cta::exception::Exception(
- "In Subprocess::Subprocess: not enough elements in argv");
- // Prepare the pipes for the child's stdout and stderr (stdin will be closed)
- const size_t readSide=0;
- const size_t writeSide=1;
- int stdoutPipe[2];
- int stderrPipe[2];
- cta::exception::Errnum::throwOnNonZero(::pipe2(stdoutPipe, O_NONBLOCK),
- "In Subprocess::Subprocess failed to create the stdout pipe");
- cta::exception::Errnum::throwOnNonZero(::pipe2(stderrPipe, O_NONBLOCK),
- "In Subprocess::Subprocess failed to create the stderr pipe");
- // Prepare the actions to be taken on file descriptors
- ScopedPosixSpawnFileActions fileActions;
- // We will be the child process. Close the read sides of the pipes.
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_addclose(fileActions, stdoutPipe[readSide]),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_addclose() (1)");
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_addclose(fileActions, stderrPipe[readSide]),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_addclose() (2)");
- // Close stdin and rewire the stdout and stderr to the pipes.
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_adddup2(fileActions, stdoutPipe[writeSide], STDOUT_FILENO),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_adddup2() (1)");
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_adddup2(fileActions, stderrPipe[writeSide], STDERR_FILENO),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_adddup2() (2)");
- // Close the now duplicated pipe file descriptors
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_addclose(fileActions, stdoutPipe[writeSide]),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_addclose() (3)");
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawn_file_actions_addclose(fileActions, stderrPipe[writeSide]),
- "In Subprocess::Subprocess(): failed to posix_spawn_file_actions_addclose() (4)");
- // And finally spawn the subprocess
- // Prepare the spawn attributes (we need vfork)
- ScopedPosixSpawnAttr attr;
- cta::exception::Errnum::throwOnReturnedErrno(posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK),
- "In Subprocess::Subprocess(): failed to posix_spawnattr_setflags()");
- char ** cargv = new char*[argv.size()+1];
- int index = 0;
- for (auto a=argv.cbegin(); a!=argv.cend(); a++) {
- cargv[index++] = ::strdup(a->c_str());
- }
- cargv[argv.size()] = NULL;
- int spawnRc=::posix_spawnp(&m_child, executable.c_str(), fileActions, attr, cargv, ::environ);
- cta::exception::Errnum::throwOnReturnedErrno(spawnRc, "In Subprocess::Subprocess failed to posix_spawn()");
- // We are the parent process. Close the write sides of pipes.
- ::close(stdoutPipe[writeSide]);
- ::close(stderrPipe[writeSide]);
- m_stdoutFd = stdoutPipe[readSide];
- m_stderrFd = stderrPipe[readSide];
-}
-
-void Subprocess::kill(int signal) {
- ::kill(m_child, signal);
-}
-
-int Subprocess::exitValue() {
- if(!m_child)
- throw cta::exception::Exception("In Subprocess::exitValue: child process not waited for");
- return WEXITSTATUS(m_childStatus);
-}
-
-void Subprocess::wait() {
- ::waitpid(m_child, &m_childStatus, 0);
- char buff[1000];
- int rc;
- while (0<(rc=::read(m_stdoutFd, buff, sizeof(buff)))) {
- m_stdout.append(buff, rc);
- }
- ::close(m_stdoutFd);
- while (0<(rc=::read(m_stderrFd, buff, sizeof(buff)))) {
- m_stderr.append(buff, rc);
- }
- ::close(m_stderrFd);
- m_childComplete = true;
-}
-
-std::string Subprocess::stdout() {
- if(!m_child)
- throw cta::exception::Exception("In Subprocess::stdout: child process not waited for");
- return m_stdout;
-}
-
-std::string Subprocess::stderr() {
- if(!m_child)
- throw cta::exception::Exception("In Subprocess::stderr: child process not waited for");
- return m_stderr;
-}
-
-Subprocess::~Subprocess() {
- if(!m_childComplete) {
- this->kill(SIGKILL);
- this->wait();
- }
-}
-
-
-} // namespace systemTests
\ No newline at end of file
diff --git a/tests/Subprocess.hpp b/tests/Subprocess.hpp
deleted file mode 100644
index 7e8005168388bba4bade4f3c004fd9edac22bde4..0000000000000000000000000000000000000000
--- a/tests/Subprocess.hpp
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * The CERN Tape Archive (CTA) project
- * Copyright (C) 2015 CERN
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#pragma once
-
-#include <list>
-#include <string>
-
-namespace systemTests {
-class Subprocess {
-public:
- Subprocess(const std::string & program, const std::list<std::string> &argv);
- ~Subprocess();
- void wait(void);
- std::string stdout();
- std::string stderr();
- void kill(int signal);
- int exitValue();
-private:
- int m_stdoutFd;
- int m_stderrFd;
- pid_t m_child;
- bool m_childComplete;
- int m_childStatus;
- std::string m_stdout;
- std::string m_stderr;
-};
-}
\ No newline at end of file
diff --git a/tests/SubprocessSystemTests.cpp b/tests/SubprocessSystemTests.cpp
index 6b0b2a595b05c5fc92c363b64dee412ad0e62b99..9a04603ef162a429d37fc3b39d26ce7cadb94882 100644
--- a/tests/SubprocessSystemTests.cpp
+++ b/tests/SubprocessSystemTests.cpp
@@ -16,26 +16,26 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include "Subprocess.hpp"
+#include "common/threading/SubProcess.hpp"
#include <gtest/gtest.h>
namespace systemTests {
-TEST(SuprocessHelper, basicTests) {
- Subprocess sp("echo", std::list<std::string>({"echo", "Hello,", "world."}));
+TEST(SubProcessHelper, basicTests) {
+ cta::threading::SubProcess sp("echo", std::list<std::string>({"echo", "Hello,", "world."}));
sp.wait();
ASSERT_EQ("Hello, world.\n", sp.stdout());
ASSERT_EQ("", sp.stderr());
ASSERT_EQ(0, sp.exitValue());
- Subprocess sp2("cat", std::list<std::string>({"cat", "/no/such/file"}));
+ cta::threading::SubProcess sp2("cat", std::list<std::string>({"cat", "/no/such/file"}));
sp2.wait();
ASSERT_EQ("", sp2.stdout());
ASSERT_NE(std::string::npos, sp2.stderr().find("/no/such/file"));
ASSERT_EQ(1, sp2.exitValue());
- Subprocess sp3("/no/such/file", std::list<std::string>({"/no/such/file"}));
+ cta::threading::SubProcess sp3("/no/such/file", std::list<std::string>({"/no/such/file"}));
sp3.wait();
ASSERT_EQ("", sp3.stdout());
ASSERT_EQ(127, sp3.exitValue());
ASSERT_EQ("", sp3.stderr());
}
-}
\ No newline at end of file
+}