From 69fac3c3d554a285b6171648a77cdba0974cb109 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 13 Sep 2017 15:32:47 +0200 Subject: pass arguments as vector to cmCTest::RunCommand() The only 2 callers took care to construct a properly escaped string, but not using the documented way, and that string was passed only to be immediately split into tokens again. Start with a vector and join it only for logging, avoiding needless quotes during that. --- Source/CTest/cmCTestCoverageHandler.cxx | 45 ++++++++++++++++++++++++++------- Source/cmCTest.cxx | 16 ++++-------- Source/cmCTest.h | 7 +---- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Source/CTest/cmCTestCoverageHandler.cxx b/Source/CTest/cmCTestCoverageHandler.cxx index 5ae4ed5..95d56ec 100644 --- a/Source/CTest/cmCTestCoverageHandler.cxx +++ b/Source/CTest/cmCTestCoverageHandler.cxx @@ -860,6 +860,24 @@ int cmCTestCoverageHandler::HandleDelphiCoverage( return static_cast(cont->TotalCoverage.size()); } +static std::string joinCommandLine(const std::vector& args) +{ + std::string ret; + + for (std::string const& s : args) { + if (s.find(' ') == std::string::npos) { + ret += s + ' '; + } else { + ret += "\"" + s + "\" "; + } + } + + // drop trailing whitespace + ret.erase(ret.size() - 1); + + return ret; +} + int cmCTestCoverageHandler::HandleBlanketJSCoverage( cmCTestCoverageHandlerContainer* cont) { @@ -974,6 +992,11 @@ int cmCTestCoverageHandler::HandleGCovCoverage( cmCTestCoverageHandlerLocale locale_C; static_cast(locale_C); + std::vector basecovargs = + cmSystemTools::ParseArguments(gcovExtraFlags.c_str()); + basecovargs.insert(basecovargs.begin(), gcovCommand); + basecovargs.push_back("-o"); + // files is a list of *.da and *.gcda files with coverage data in them. // These are binary files that you give as input to gcov so that it will // give us text output we can analyze to summarize coverage. @@ -985,8 +1008,10 @@ int cmCTestCoverageHandler::HandleGCovCoverage( // Call gcov to get coverage data for this *.gcda file: // std::string fileDir = cmSystemTools::GetFilenamePath(f); - std::string command = "\"" + gcovCommand + "\" " + gcovExtraFlags + " " + - "-o \"" + fileDir + "\" " + "\"" + f + "\""; + std::vector covargs = basecovargs; + covargs.push_back(fileDir); + covargs.push_back(f); + const std::string command = joinCommandLine(covargs); cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, command << std::endl, this->Quiet); @@ -996,9 +1021,8 @@ int cmCTestCoverageHandler::HandleGCovCoverage( int retVal = 0; *cont->OFS << "* Run coverage for: " << fileDir << std::endl; *cont->OFS << " Command: " << command << std::endl; - int res = - this->CTest->RunCommand(command.c_str(), &output, &errors, &retVal, - tempDir.c_str(), 0 /*this->TimeOut*/); + int res = this->CTest->RunCommand(covargs, &output, &errors, &retVal, + tempDir.c_str(), 0 /*this->TimeOut*/); *cont->OFS << " Output: " << output << std::endl; *cont->OFS << " Errors: " << errors << std::endl; @@ -1337,6 +1361,11 @@ int cmCTestCoverageHandler::HandleLCovCoverage( cmCTestCoverageHandlerLocale locale_C; static_cast(locale_C); + std::vector covargs = + cmSystemTools::ParseArguments(lcovExtraFlags.c_str()); + covargs.insert(covargs.begin(), lcovCommand); + const std::string command = joinCommandLine(covargs); + // In intel compiler we have to call codecov only once in each executable // directory. It collects all *.dyn files to generate .dpi file. for (std::string const& f : files) { @@ -1344,7 +1373,6 @@ int cmCTestCoverageHandler::HandleLCovCoverage( this->Quiet); std::string fileDir = cmSystemTools::GetFilenamePath(f); cmWorkingDirectory workdir(fileDir); - std::string command = "\"" + lcovCommand + "\" " + lcovExtraFlags + " "; cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "Current coverage dir: " << fileDir << std::endl, @@ -1357,9 +1385,8 @@ int cmCTestCoverageHandler::HandleLCovCoverage( int retVal = 0; *cont->OFS << "* Run coverage for: " << fileDir << std::endl; *cont->OFS << " Command: " << command << std::endl; - int res = - this->CTest->RunCommand(command.c_str(), &output, &errors, &retVal, - fileDir.c_str(), 0 /*this->TimeOut*/); + int res = this->CTest->RunCommand(covargs, &output, &errors, &retVal, + fileDir.c_str(), 0 /*this->TimeOut*/); *cont->OFS << " Output: " << output << std::endl; *cont->OFS << " Errors: " << errors << std::endl; diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index 1325fd3..bcc45ce 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -2565,24 +2565,18 @@ bool cmCTest::SetCTestConfigurationFromCMakeVariable( return true; } -bool cmCTest::RunCommand(const char* command, std::string* stdOut, - std::string* stdErr, int* retVal, const char* dir, - double timeout, Encoding encoding) +bool cmCTest::RunCommand(std::vector const& args, + std::string* stdOut, std::string* stdErr, int* retVal, + const char* dir, double timeout, Encoding encoding) { - std::vector args = cmSystemTools::ParseArguments(command); - - if (args.empty()) { - return false; - } - std::vector argv; for (std::string const& a : args) { argv.push_back(a.c_str()); } argv.push_back(nullptr); - *stdOut = ""; - *stdErr = ""; + stdOut->clear(); + stdErr->clear(); cmsysProcess* cp = cmsysProcess_New(); cmsysProcess_SetCommand(cp, &*argv.begin()); diff --git a/Source/cmCTest.h b/Source/cmCTest.h index 66e6a26..dbd67dc 100644 --- a/Source/cmCTest.h +++ b/Source/cmCTest.h @@ -245,13 +245,8 @@ public: * exit code will be stored. If the retVal is not specified and * the program exits with a code other than 0, then the this * function will return false. - * - * If the command has spaces in the path the caller MUST call - * cmSystemTools::ConvertToRunCommandPath on the command before passing - * it into this function or it will not work. The command must be correctly - * escaped for this to with spaces. */ - bool RunCommand(const char* command, std::string* stdOut, + bool RunCommand(std::vector const& args, std::string* stdOut, std::string* stdErr, int* retVal = nullptr, const char* dir = nullptr, double timeout = 0.0, Encoding encoding = cmProcessOutput::Auto); -- cgit v0.12 From 67529aab816f7f6acd942c76d7b57ecf9c6bf2d3 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 13 Sep 2017 15:35:14 +0200 Subject: Doc: document that CoverageExtraFlags will come first --- Help/manual/ctest.1.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Help/manual/ctest.1.rst b/Help/manual/ctest.1.rst index 03466ce..423f1ca 100644 --- a/Help/manual/ctest.1.rst +++ b/Help/manual/ctest.1.rst @@ -886,6 +886,8 @@ Configuration settings include: * `CTest Script`_ variable: :variable:`CTEST_COVERAGE_EXTRA_FLAGS` * :module:`CTest` module variable: ``COVERAGE_EXTRA_FLAGS`` + These options are the first arguments passed to ``CoverageCommand``. + .. _`CTest MemCheck Step`: CTest MemCheck Step -- cgit v0.12