cryfs-unmount correctly unmounts paths that contain spaces

ChangeLog.txt

@@ -3,6 +3,7 @@ Version 0.11.1 (unreleased)
 Bugfix:
 * Fix building of the range-v3 dependency. The conan remote URL for this dependency changed and we have to use the new URL. See https://github.com/cryfs/cryfs/issues/398
 * Update to CryptoPP 8.6. This fixes a rare bug where CryptoPP 8.5 encrypts data wrongly, see https://github.com/weidai11/cryptopp/issues/1069
+* cryfs-unmount correctly unmounts paths that contain spaces, see https://github.com/cryfs/cryfs/issues/372
 
 Version 0.11.0
 ---------------

src/cpp-utils/process/subprocess.cpp

@@ -3,91 +3,74 @@
 #include <stdexcept>
 #include <cerrno>
 #include <array>
-
+#include <boost/process.hpp>
-#if defined(__APPLE__)
-
-#include <sys/wait.h>
-constexpr const char* openmode = "r";
-
-#elif !defined(_MSC_VER)
-
-#include <sys/wait.h>
-constexpr const char* openmode = "re";
-
-#else
-
-#define popen _popen
-#define pclose _pclose
-#define WEXITSTATUS(a) a
-#define WIFEXITED(a) true
-constexpr const char* openmode = "r";
-
-#endif
 
 using std::string;
+using std::vector;
 
-namespace cpputils {
+namespace bp = boost::process;
-    namespace {
+namespace bf = boost::filesystem;
-	class SubprocessHandle final {
+
-	public:
+namespace cpputils
-		SubprocessHandle(const string &command)
+{
-		: _subprocess(popen(command.c_str(), openmode)) {
+	namespace
-			if (!_subprocess) {
+	{
-				throw std::runtime_error("Error starting subprocess " + command + ". Errno: " + std::to_string(errno));
+		bf::path _find_executable(const char *command)
+		{
+			bf::path executable = bp::search_path(command);
+			if (executable == "")
+			{
+				throw std::runtime_error("Tried to run command " + std::string(command) + " but didn't find it in the PATH");
 			}
+			return executable;
+		}
+	}
+
+	SubprocessResult Subprocess::call(const char *command, const vector<string> &args)
+	{
+		return call(_find_executable(command), args);
+	}
+
+	SubprocessResult Subprocess::check_call(const char *command, const vector<string> &args)
+	{
+		return check_call(_find_executable(command), args);
+	}
+
+	SubprocessResult Subprocess::call(const bf::path &executable, const vector<string> &args)
+	{
+		if (!bf::exists(executable))
+		{
+			throw std::runtime_error("Tried to run executable " + executable.string() + " but didn't find it");
 		}
 
-		~SubprocessHandle() {
+		bp::ipstream child_stdout;
-			if (_subprocess != nullptr) {
+		bp::ipstream child_stderr;
-				close();
+		bp::child child = bp::child(bp::exe = executable.string(), bp::std_out > child_stdout, bp::std_err > child_stderr, bp::args(args));
-			}
+		if (!child.valid())
+		{
+			throw std::runtime_error("Error starting subprocess " + executable.string() + ". Errno: " + std::to_string(errno));
 		}
 
-		string getOutput() {
+		child.join();
-			string output;
+
-			std::array<char, 1024> buffer{};
+		string output_stdout = string(std::istreambuf_iterator<char>(child_stdout), {});
-			while (fgets(buffer.data(), buffer.size(), _subprocess) != nullptr) {
+		string output_stderr = string(std::istreambuf_iterator<char>(child_stderr), {});
-				output += buffer.data();
+
-			}
+		return SubprocessResult{
-			return output;
+			std::move(output_stdout),
+			std::move(output_stderr),
+			child.exit_code(),
+		};
+	}
+
+	SubprocessResult Subprocess::check_call(const bf::path &executable, const vector<string> &args)
+	{
+		auto result = call(executable, args);
+		if (result.exitcode != 0)
+		{
+			throw SubprocessError("Subprocess \"" + executable.string() + "\" exited with code " + std::to_string(result.exitcode));
 		}
-
+		return result;
-		int close() {
+	}
-			auto returncode = pclose(_subprocess);
-			_subprocess = nullptr;
-			if (returncode == -1) {
-				throw std::runtime_error("Error calling pclose. Errno: " + std::to_string(errno));
-			}
-#pragma GCC diagnostic push // WIFEXITSTATUS / WEXITSTATUS use old style casts
-#pragma GCC diagnostic ignored "-Wold-style-cast"
-			if (!WIFEXITED(returncode)) {
-				// WEXITSTATUS is only valid if WIFEXITED is 0.
-				throw std::runtime_error("WIFEXITED returned " + std::to_string(WIFEXITED(returncode)));
-			}
-			return WEXITSTATUS(returncode);
-#pragma GCC diagnostic pop
-		}
-
-	private:
-		FILE *_subprocess;
-	};
-
-    }
-
-    SubprocessResult Subprocess::call(const string &command) {
-		SubprocessHandle subprocess(command);
-        string output = subprocess.getOutput();
-        int exitcode = subprocess.close();
-
-        return SubprocessResult {output, exitcode};
-    }
-
-    SubprocessResult Subprocess::check_call(const string &command) {
-        auto result = call(command);
-        if(result.exitcode != 0) {
-            throw SubprocessError("Subprocess \""+command+"\" exited with code "+std::to_string(result.exitcode));
-        }
-        return result;
-    }
 
 }

src/cpp-utils/process/subprocess.h

@@ -3,26 +3,34 @@
 #define MESSMER_CPPUTILS_PROCESS_SUBPROCESS_H
 
 #include <string>
+#include <vector>
 #include <stdexcept>
+#include <boost/filesystem/path.hpp>
 #include "../macros.h"
 
-namespace cpputils {
+namespace cpputils
-    struct SubprocessResult final {
+{
-        std::string output;
+    struct SubprocessResult final
+    {
+        std::string output_stdout;
+        std::string output_stderr;
         int exitcode;
     };
 
-    struct SubprocessError final : public std::runtime_error {
+    struct SubprocessError final : public std::runtime_error
-        SubprocessError(std::string msg): std::runtime_error(std::move(msg)) {}
+    {
+        SubprocessError(std::string msg) : std::runtime_error(std::move(msg)) {}
     };
 
-    //TODO Test
+    class Subprocess final
-    class Subprocess final {
+    {
     public:
-        static SubprocessResult call(const std::string &command);
+        static SubprocessResult call(const char *command, const std::vector<std::string> &args);
-        static SubprocessResult check_call(const std::string &command);
+        static SubprocessResult call(const boost::filesystem::path &executable, const std::vector<std::string> &args);
-    private:
+        static SubprocessResult check_call(const char *command, const std::vector<std::string> &args);
+        static SubprocessResult check_call(const boost::filesystem::path &executable, const std::vector<std::string> &args);
 
+    private:
         DISALLOW_COPY_AND_ASSIGN(Subprocess);
     };
 }

src/fspp/fuse/Fuse.cpp

@@ -482,16 +482,15 @@ void Fuse::unmount(const bf::path& mountdir, bool force) {
   //TODO Find better way to unmount (i.e. don't use external fusermount). Unmounting by kill(getpid(), SIGINT) worked, but left the mount directory transport endpoint as not connected.
 #if defined(__APPLE__)
   UNUSED(force);
-  int returncode = cpputils::Subprocess::call(std::string("umount ") + mountdir.string()).exitcode;
+  int returncode = cpputils::Subprocess::call("umount", {mountdir.string()}).exitcode;
 #elif defined(_MSC_VER)
   UNUSED(force);
   std::wstring mountdir_ = std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>>().from_bytes(mountdir.string());
   BOOL success = DokanRemoveMountPoint(mountdir_.c_str());
   int returncode = success ? 0 : -1;
 #else
-  std::string command = force ? "fusermount -u" : "fusermount -z -u";  // "-z" takes care that if the filesystem can't be unmounted right now because something is opened, it will be unmounted as soon as it can be.
+  std::vector<std::string> args = force ? std::vector<std::string>({"-u", mountdir.string()}) : std::vector<std::string>({"-u", "-z", mountdir.string()});  // "-z" takes care that if the filesystem can't be unmounted right now because something is opened, it will be unmounted as soon as it can be.
-  int returncode = cpputils::Subprocess::call(
+  int returncode = cpputils::Subprocess::call("fusermount", args).exitcode;
-	  command + " " + mountdir.string()).exitcode;
 #endif
   if (returncode != 0) {
     throw std::runtime_error("Could not unmount filesystem");

test/cpp-utils/assert/backtrace_test.cpp

@@ -9,187 +9,221 @@ using std::string;
 using testing::HasSubstr;
 namespace bf = boost::filesystem;
 
-namespace {
+namespace
-	std::string call_process_exiting_with(const std::string& kind, const std::string& signal = "") {
+{
+	std::string call_process_exiting_with(const std::string &kind, const std::string &signal = "")
+	{
 #if defined(_MSC_VER)
-		auto executable = get_executable().parent_path() / "cpp-utils-test_exit_signal.exe";
+		auto executable = bf::canonical(get_executable().parent_path()) / "cpp-utils-test_exit_signal.exe";
 #else
-		auto executable = get_executable().parent_path() / "cpp-utils-test_exit_signal";
+		auto executable = bf::canonical(get_executable().parent_path()) / "cpp-utils-test_exit_signal";
 #endif
-		if (!bf::exists(executable)) {
+		if (!bf::exists(executable))
+		{
 			throw std::runtime_error(executable.string() + " not found.");
 		}
-		const std::string command = executable.string() + " \"" + kind + "\" \"" + signal + "\"  2>&1";
+		auto result = cpputils::Subprocess::call(executable, {kind, signal});
-		auto result = cpputils::Subprocess::call(command);
+		return result.output_stderr;
-		return result.output;
 	}
 }
 
 #if !(defined(_MSC_VER) && defined(NDEBUG))
 
-TEST(BacktraceTest, ContainsTopLevelLine) {
+TEST(BacktraceTest, ContainsTopLevelLine)
-    string backtrace = cpputils::backtrace();
+{
-    EXPECT_THAT(backtrace, HasSubstr("BacktraceTest"));
+	string backtrace = cpputils::backtrace();
-    EXPECT_THAT(backtrace, HasSubstr("ContainsTopLevelLine"));
+	EXPECT_THAT(backtrace, HasSubstr("BacktraceTest"));
+	EXPECT_THAT(backtrace, HasSubstr("ContainsTopLevelLine"));
 }
 #endif
 
-namespace {
+namespace
-	std::string call_process_exiting_with_nullptr_violation() {
+{
+	std::string call_process_exiting_with_nullptr_violation()
+	{
 		return call_process_exiting_with("nullptr");
 	}
-	std::string call_process_exiting_with_exception(const std::string& message) {
+	std::string call_process_exiting_with_exception(const std::string &message)
+	{
 		return call_process_exiting_with("exception", message);
 	}
 }
 #if defined(_MSC_VER)
 #include <Windows.h>
-namespace {
+namespace
-	std::string call_process_exiting_with_sigsegv() {
+{
+	std::string call_process_exiting_with_sigsegv()
+	{
 		return call_process_exiting_with("signal", std::to_string(EXCEPTION_ACCESS_VIOLATION));
 	}
-	std::string call_process_exiting_with_sigill() {
+	std::string call_process_exiting_with_sigill()
+	{
 		return call_process_exiting_with("signal", std::to_string(EXCEPTION_ILLEGAL_INSTRUCTION));
 	}
-	std::string call_process_exiting_with_code(DWORD code) {
+	std::string call_process_exiting_with_code(DWORD code)
+	{
 		return call_process_exiting_with("signal", std::to_string(code));
 	}
 }
 #else
-namespace {
+namespace
-	std::string call_process_exiting_with_sigsegv() {
+{
+	std::string call_process_exiting_with_sigsegv()
+	{
 		return call_process_exiting_with("signal", std::to_string(SIGSEGV));
 	}
-	std::string call_process_exiting_with_sigabrt() {
+	std::string call_process_exiting_with_sigabrt()
+	{
 		return call_process_exiting_with("signal", std::to_string(SIGABRT));
 	}
-	std::string call_process_exiting_with_sigill() {
+	std::string call_process_exiting_with_sigill()
+	{
 		return call_process_exiting_with("signal", std::to_string(SIGILL));
 	}
 }
 #endif
 
-TEST(BacktraceTest, DoesntCrashOnCaughtException) {
+TEST(BacktraceTest, DoesntCrashOnCaughtException)
+{
 	// This is needed to make sure we don't use some kind of vectored exception handler on Windows
 	// that ignores the call stack and always jumps on when an exception happens.
 	cpputils::showBacktraceOnCrash();
-	try {
+	try
+	{
 		throw std::logic_error("exception");
-	} catch (const std::logic_error& e) {
+	}
+	catch (const std::logic_error &e)
+	{
 		// intentionally empty
 	}
 }
 
 #if !(defined(_MSC_VER) && defined(NDEBUG))
-TEST(BacktraceTest, ContainsBacktrace) {
+TEST(BacktraceTest, ContainsBacktrace)
-    string backtrace = cpputils::backtrace();
+{
+	string backtrace = cpputils::backtrace();
 #if defined(_MSC_VER)
-    EXPECT_THAT(backtrace, HasSubstr("testing::Test::Run"));
+	EXPECT_THAT(backtrace, HasSubstr("testing::Test::Run"));
 #else
-    EXPECT_THAT(backtrace, HasSubstr("BacktraceTest_ContainsBacktrace_Test::TestBody"));
+	EXPECT_THAT(backtrace, HasSubstr("BacktraceTest_ContainsBacktrace_Test::TestBody"));
 #endif
 }
 
-TEST(BacktraceTest, ShowBacktraceOnNullptrAccess) {
+TEST(BacktraceTest, ShowBacktraceOnNullptrAccess)
-    auto output = call_process_exiting_with_nullptr_violation();
+{
+	auto output = call_process_exiting_with_nullptr_violation();
 #if defined(_MSC_VER)
-    EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
+	EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
 #else
-    EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
+	EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
 #endif
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigSegv) {
+TEST(BacktraceTest, ShowBacktraceOnSigSegv)
+{
 	auto output = call_process_exiting_with_sigsegv();
 #if defined(_MSC_VER)
-    EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
+	EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
 #else
-    EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
+	EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
 #endif
 }
 
-TEST(BacktraceTest, ShowBacktraceOnUnhandledException) {
+TEST(BacktraceTest, ShowBacktraceOnUnhandledException)
+{
 	auto output = call_process_exiting_with_exception("my_exception_message");
 #if defined(_MSC_VER)
-    EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
+	EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
 #else
-    EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
+	EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
 #endif
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigIll) {
+TEST(BacktraceTest, ShowBacktraceOnSigIll)
+{
 	auto output = call_process_exiting_with_sigill();
 #if defined(_MSC_VER)
-    EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
+	EXPECT_THAT(output, HasSubstr("handle_exit_signal"));
 #else
-    EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
+	EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
 #endif
 }
 #else
-TEST(BacktraceTest, ContainsBacktrace) {
+TEST(BacktraceTest, ContainsBacktrace)
+{
 	string backtrace = cpputils::backtrace();
 	EXPECT_THAT(backtrace, HasSubstr("#0"));
 }
-TEST(BacktraceTest, ShowBacktraceOnNullptrAccess) {
+TEST(BacktraceTest, ShowBacktraceOnNullptrAccess)
+{
 	auto output = call_process_exiting_with_nullptr_violation();
 	EXPECT_THAT(output, HasSubstr("#1"));
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigSegv) {
+TEST(BacktraceTest, ShowBacktraceOnSigSegv)
+{
 	auto output = call_process_exiting_with_sigsegv();
 	EXPECT_THAT(output, HasSubstr("#1"));
 }
 
-TEST(BacktraceTest, ShowBacktraceOnUnhandledException) {
+TEST(BacktraceTest, ShowBacktraceOnUnhandledException)
+{
 	auto output = call_process_exiting_with_exception("my_exception_message");
 	EXPECT_THAT(output, HasSubstr("#1"));
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigIll) {
+TEST(BacktraceTest, ShowBacktraceOnSigIll)
+{
 	auto output = call_process_exiting_with_sigill();
 	EXPECT_THAT(output, HasSubstr("#1"));
 }
 #endif
 
 #if !defined(_MSC_VER)
-TEST(BacktraceTest, ShowBacktraceOnSigAbrt) {
+TEST(BacktraceTest, ShowBacktraceOnSigAbrt)
-    auto output = call_process_exiting_with_sigabrt();
+{
-    EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
+	auto output = call_process_exiting_with_sigabrt();
+	EXPECT_THAT(output, HasSubstr("cpputils::backtrace"));
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigAbrt_ShowsCorrectSignalName) {
+TEST(BacktraceTest, ShowBacktraceOnSigAbrt_ShowsCorrectSignalName)
+{
 	auto output = call_process_exiting_with_sigabrt();
 	EXPECT_THAT(output, HasSubstr("SIGABRT"));
 }
 #endif
 
 #if !defined(_MSC_VER)
-constexpr const char* sigsegv_message = "SIGSEGV";
+constexpr const char *sigsegv_message = "SIGSEGV";
-constexpr const char* sigill_message = "SIGILL";
+constexpr const char *sigill_message = "SIGILL";
 #else
-constexpr const char* sigsegv_message = "EXCEPTION_ACCESS_VIOLATION";
+constexpr const char *sigsegv_message = "EXCEPTION_ACCESS_VIOLATION";
-constexpr const char* sigill_message = "EXCEPTION_ILLEGAL_INSTRUCTION";
+constexpr const char *sigill_message = "EXCEPTION_ILLEGAL_INSTRUCTION";
 #endif
 
-TEST(BacktraceTest, ShowBacktraceOnSigSegv_ShowsCorrectSignalName) {
+TEST(BacktraceTest, ShowBacktraceOnSigSegv_ShowsCorrectSignalName)
+{
 	auto output = call_process_exiting_with_sigsegv();
 	EXPECT_THAT(output, HasSubstr(sigsegv_message));
 }
 
-TEST(BacktraceTest, ShowBacktraceOnSigIll_ShowsCorrectSignalName) {
+TEST(BacktraceTest, ShowBacktraceOnSigIll_ShowsCorrectSignalName)
+{
 	auto output = call_process_exiting_with_sigill();
 	EXPECT_THAT(output, HasSubstr(sigill_message));
 }
 
 #if !defined(_MSC_VER)
-TEST(BacktraceTest, ShowBacktraceOnUnhandledException_ShowsCorrectExceptionMessage) {
+TEST(BacktraceTest, ShowBacktraceOnUnhandledException_ShowsCorrectExceptionMessage)
+{
 	auto output = call_process_exiting_with_exception("my_exception_message");
 	EXPECT_THAT(output, HasSubstr("my_exception_message"));
 }
 #endif
 
 #if defined(_MSC_VER)
-TEST(BacktraceTest, UnknownCode_ShowsCorrectSignalName) {
+TEST(BacktraceTest, UnknownCode_ShowsCorrectSignalName)
+{
 	auto output = call_process_exiting_with_code(0x1234567);
 	EXPECT_THAT(output, HasSubstr("UNKNOWN_CODE(0x1234567)"));
 }

test/cpp-utils/process/SubprocessTest.cpp

@@ -10,122 +10,159 @@ using cpputils::SubprocessError;
 using std::string;
 namespace bf = boost::filesystem;
 
-namespace {
-std::string exit_with_message_and_status(const char* message, int status) {
 #if defined(_MSC_VER)
-    auto executable = get_executable().parent_path() / "cpp-utils-test_exit_status.exe";
+constexpr const char* NEWLINE = "\r\n";
 #else
-    auto executable = get_executable().parent_path() / "cpp-utils-test_exit_status";
+constexpr const char* NEWLINE = "\n";
 #endif
-    if (!bf::exists(executable)) {
+
-        throw std::runtime_error(executable.string() + " not found.");
+namespace
+{
+    bf::path exit_with_message_and_status()
+    {
+#if defined(_MSC_VER)
+        auto executable = bf::canonical(get_executable().parent_path()) / "cpp-utils-test_exit_status.exe";
+#else
+        auto executable = bf::canonical(get_executable().parent_path()) / "cpp-utils-test_exit_status";
+#endif
+        if (!bf::exists(executable))
+        {
+            throw std::runtime_error(executable.string() + " not found.");
+        }
+        return executable;
     }
-	return executable.string() + " \"" + message + "\" " + std::to_string(status);
-}
 }
 
-TEST(SubprocessTest, CheckCall_success_output) {
+TEST(SubprocessTest, CheckCall_success_output)
-    EXPECT_EQ("hello", Subprocess::check_call(exit_with_message_and_status("hello", 0)).output);
+{
+    EXPECT_EQ(std::string("hello") + NEWLINE, Subprocess::check_call(exit_with_message_and_status(), {"0", "hello"}).output_stdout);
 }
 
-TEST(SubprocessTest, CheckCall_successwithemptyoutput_output) {
+TEST(SubprocessTest, CheckCall_successwithemptyoutput_output)
-    EXPECT_EQ("", Subprocess::check_call(exit_with_message_and_status("", 0)).output);
+{
+    EXPECT_EQ("", Subprocess::check_call(exit_with_message_and_status(), {"0"}).output_stdout);
 }
 
-TEST(SubprocessTest, CheckCall_success_exitcode) {
+TEST(SubprocessTest, CheckCall_success_exitcode)
-    EXPECT_EQ(0, Subprocess::check_call(exit_with_message_and_status("hello", 0)).exitcode);
+{
+    EXPECT_EQ(0, Subprocess::check_call(exit_with_message_and_status(), {"0", "hello"}).exitcode);
 }
 
-TEST(SubprocessTest, CheckCall_successwithemptyoutput_exitcode) {
+TEST(SubprocessTest, CheckCall_successwithemptyoutput_exitcode)
-    EXPECT_EQ(0, Subprocess::check_call(exit_with_message_and_status("", 0)).exitcode);
+{
+    EXPECT_EQ(0, Subprocess::check_call(exit_with_message_and_status(), {"0"}).exitcode);
 }
 
-TEST(SubprocessTest, CheckCall_error) {
+TEST(SubprocessTest, CheckCall_error)
+{
     EXPECT_THROW(
-      Subprocess::check_call(exit_with_message_and_status("", 1)),
+        Subprocess::check_call(exit_with_message_and_status(), {"1"}),
-      SubprocessError
+        SubprocessError);
-    );
 }
 
-TEST(SubprocessTest, CheckCall_error5) {
+TEST(SubprocessTest, CheckCall_error5)
+{
     EXPECT_THROW(
-      Subprocess::check_call(exit_with_message_and_status("", 5)),
+        Subprocess::check_call(exit_with_message_and_status(), {"5"}),
-      SubprocessError
+        SubprocessError);
-    );
 }
 
-TEST(SubprocessTest, CheckCall_errorwithoutput) {
+TEST(SubprocessTest, CheckCall_errorwithoutput)
+{
     EXPECT_THROW(
-      Subprocess::check_call(exit_with_message_and_status("hello", 1)),
+        Subprocess::check_call(exit_with_message_and_status(), {"1", "hello"}),
-      SubprocessError
+        SubprocessError);
-    );
 }
 
-TEST(SubprocessTest, CheckCall_error5withoutput) {
+TEST(SubprocessTest, CheckCall_error5withoutput)
+{
     EXPECT_THROW(
-      Subprocess::check_call(exit_with_message_and_status("hello", 5)),
+        Subprocess::check_call(exit_with_message_and_status(), {"5", "hello"}),
-      SubprocessError
+        SubprocessError);
-    );
 }
 
-TEST(SubprocessTest, Call_success_exitcode) {
+TEST(SubprocessTest, Call_success_exitcode)
-    EXPECT_EQ(0, Subprocess::call(exit_with_message_and_status("hello", 0)).exitcode);
+{
+    EXPECT_EQ(0, Subprocess::call(exit_with_message_and_status(), {"0", "hello"}).exitcode);
 }
 
-TEST(SubprocessTest, Call_success_output) {
+TEST(SubprocessTest, Call_success_output)
-    EXPECT_EQ("hello", Subprocess::call(exit_with_message_and_status("hello", 0)).output);
+{
+    EXPECT_EQ(std::string("hello") + NEWLINE, Subprocess::call(exit_with_message_and_status(), {"0", "hello"}).output_stdout);
 }
 
-TEST(SubprocessTest, Call_error_exitcode) {
+TEST(SubprocessTest, Call_error_exitcode)
-    EXPECT_EQ(1, Subprocess::call(exit_with_message_and_status("", 1)).exitcode);
+{
+    EXPECT_EQ(1, Subprocess::call(exit_with_message_and_status(), {"1"}).exitcode);
 }
 
-TEST(SubprocessTest, Call_error_output) {
+TEST(SubprocessTest, Call_error_output)
-    EXPECT_EQ("", Subprocess::call(exit_with_message_and_status("", 1)).output);
+{
+    EXPECT_EQ("", Subprocess::call(exit_with_message_and_status(), {"1"}).output_stdout);
 }
 
-TEST(SubprocessTest, Call_error5_exitcode) {
+TEST(SubprocessTest, Call_error5_exitcode)
-    EXPECT_EQ(5, Subprocess::call(exit_with_message_and_status("", 5)).exitcode);
+{
+    EXPECT_EQ(5, Subprocess::call(exit_with_message_and_status(), {"5"}).exitcode);
 }
 
-TEST(SubprocessTest, Call_error5_output) {
+TEST(SubprocessTest, Call_error5_output)
-    EXPECT_EQ("", Subprocess::call(exit_with_message_and_status("", 1)).output);
+{
+    EXPECT_EQ("", Subprocess::call(exit_with_message_and_status(), {"1"}).output_stdout);
 }
 
-TEST(SubprocessTest, Call_errorwithoutput_output) {
+TEST(SubprocessTest, Call_errorwithoutput_output)
-    EXPECT_EQ("hello", Subprocess::call(exit_with_message_and_status("hello", 1)).output);
+{
+    EXPECT_EQ(std::string("hello") + NEWLINE, Subprocess::call(exit_with_message_and_status(), {"1", "hello"}).output_stdout);
 }
 
-TEST(SubprocessTest, Call_errorwithoutput_exitcode) {
+TEST(SubprocessTest, Call_errorwithoutput_exitcode)
-    EXPECT_EQ(1, Subprocess::call(exit_with_message_and_status("hello", 1)).exitcode);
+{
+    EXPECT_EQ(1, Subprocess::call(exit_with_message_and_status(), {"1", "hello"}).exitcode);
 }
 
-TEST(SubprocessTest, Call_error5withoutput_output) {
+TEST(SubprocessTest, Call_error5withoutput_output)
-    EXPECT_EQ("hello", Subprocess::call(exit_with_message_and_status("hello", 5)).output);
+{
+    EXPECT_EQ(std::string("hello") + NEWLINE, Subprocess::call(exit_with_message_and_status(), {"5", "hello"}).output_stdout);
 }
 
-TEST(SubprocessTest, Call_error5withoutput_exitcode) {
+TEST(SubprocessTest, Call_error5withoutput_exitcode)
-    EXPECT_EQ(5, Subprocess::call(exit_with_message_and_status("hello", 5)).exitcode);
+{
+    EXPECT_EQ(5, Subprocess::call(exit_with_message_and_status(), {"5", "hello"}).exitcode);
 }
 
 // TODO Move this test to a test suite for ThreadSystem/LoopThread
 #include <cpp-utils/thread/LoopThread.h>
-TEST(SubprocessTest, CallFromThreadSystemThread) {
+TEST(SubprocessTest, CallFromThreadSystemThread)
+{
     cpputils::ConditionBarrier barrier;
 
     cpputils::LoopThread thread(
-        [&barrier] () {
+        [&barrier]()
-            auto result = Subprocess::check_call(exit_with_message_and_status("hello", 0));
+        {
+            auto result = Subprocess::check_call(exit_with_message_and_status(), {"0", "hello"});
             EXPECT_EQ(0, result.exitcode);
-            EXPECT_EQ("hello", result.output);
+            EXPECT_EQ(std::string("hello") + NEWLINE, result.output_stdout);
 
             barrier.release();
 
             return false; // don't run loop again
         },
-        "child_thread"
+        "child_thread");
-    );
     thread.start();
     barrier.wait();
     thread.stop(); // just to make sure it's stopped before the test exits. Returning false above should already stop it, but we don't know when exactly. thread.stop() will block until it's actually stopped.
 }
+
+TEST(SubprocessTest, Call_argumentwithspaces)
+{
+    // Test that arguments can have spaces and are still treated as one argument
+    EXPECT_EQ(std::string("hello world") + NEWLINE, Subprocess::check_call(exit_with_message_and_status(), {"0", "hello world"}).output_stdout);
+    EXPECT_EQ(std::string("hello") + NEWLINE + "world" + NEWLINE, Subprocess::check_call(exit_with_message_and_status(), {"0", "hello", "world"}).output_stdout);
+}
+
+TEST(SubprocessTest, Call_withcommandfrompath)
+{
+    // Test that we can call a system command without specifying the full path
+    EXPECT_EQ("hello\n", Subprocess::check_call("echo", {"hello"}).output_stdout);
+}

test/cpp-utils/process/exit_status.cpp

@@ -1,16 +1,21 @@
-// This is a small executable that prints its first argument and exits with the exit status in its second argument
+// This is a small executable that exits with the exit status in its first argument and before exiting prints all other arguments, each on a separate line.
 
 #include <iostream>
 #include <cstdlib>
 
-int main(int argc, char* argv[]) {
+int main(int argc, char *argv[])
-	if (argc != 3) {
+{
+	if (argc < 2)
+	{
 		std::cerr << "Wrong number of arguments" << std::endl;
 		std::abort();
 	}
 
-	std::cout << argv[1];
+	for (int i = 2; i < argc; ++i)
+	{
+		std::cout << argv[i] << "\n";
+	}
 
-	int exit_status = static_cast<int>(std::strtol(argv[2], nullptr, 10));
+	int exit_status = static_cast<int>(std::strtol(argv[1], nullptr, 10));
 	return exit_status;
 }