From 548753aa697c27d7f9a52bbd352c52d45b80f371 Mon Sep 17 00:00:00 2001 From: Andrey Prygunkov Date: Tue, 23 Feb 2016 22:07:30 +0100 Subject: [PATCH] #172: using vector of strings in SplitCommandLine instead of dynamic array of c-style strings. --- daemon/extension/SchedulerScript.cpp | 7 ++++--- daemon/extension/SchedulerScript.h | 7 ++++--- daemon/main/Options.cpp | 2 +- daemon/main/nzbget.h | 1 + daemon/postprocess/DupeMatcher.cpp | 12 +++--------- daemon/postprocess/Unpack.cpp | 10 +++------- daemon/util/Util.cpp | 22 +++++----------------- daemon/util/Util.h | 11 ++--------- 8 files changed, 23 insertions(+), 49 deletions(-) diff --git a/daemon/extension/SchedulerScript.cpp b/daemon/extension/SchedulerScript.cpp index 524c4c1a..490bbe80 100644 --- a/daemon/extension/SchedulerScript.cpp +++ b/daemon/extension/SchedulerScript.cpp @@ -32,8 +32,8 @@ void SchedulerScriptController::StartScript(const char* param, bool externalProcess, int taskId) { - char** argv = nullptr; - if (externalProcess && !Util::SplitCommandLine(param, &argv)) + std::vector argv; + if (externalProcess && (argv = Util::SplitCommandLine(param)).empty()) { error("Could not execute scheduled process-script, failed to parse command line: %s", param); return; @@ -48,7 +48,8 @@ void SchedulerScriptController::StartScript(const char* param, bool externalProc if (externalProcess) { scriptController->SetScript(argv[0]); - scriptController->SetArgs((const char**)argv, true); + scriptController->m_args = std::move(argv); + scriptController->SetArgs((const char**)scriptController->m_args.data(), false); } scriptController->SetAutoDestroy(true); diff --git a/daemon/extension/SchedulerScript.h b/daemon/extension/SchedulerScript.h index ed3ebc6b..4206e57d 100644 --- a/daemon/extension/SchedulerScript.h +++ b/daemon/extension/SchedulerScript.h @@ -32,9 +32,10 @@ class SchedulerScriptController : public Thread, public NzbScriptController { private: - CString m_script; - bool m_externalProcess; - int m_taskId; + CString m_script; + bool m_externalProcess; + int m_taskId; + std::vector m_args; void PrepareParams(const char* scriptName); void ExecuteExternalProcess(); diff --git a/daemon/main/Options.cpp b/daemon/main/Options.cpp index 14296fdc..c59a370f 100644 --- a/daemon/main/Options.cpp +++ b/daemon/main/Options.cpp @@ -1242,7 +1242,7 @@ void Options::InitScheduler() BString<100>("Task%i.Command", n), CommandCount, CommandNames, CommandValues); if (param && strlen(param) > 0 && taskCommand == scProcess && - !Util::SplitCommandLine(param, nullptr)) + Util::SplitCommandLine(param).empty()) { ConfigError("Invalid value for option \"Task%i.Param\"", n); continue; diff --git a/daemon/main/nzbget.h b/daemon/main/nzbget.h index d7b61c9a..1cba318f 100644 --- a/daemon/main/nzbget.h +++ b/daemon/main/nzbget.h @@ -196,6 +196,7 @@ using namespace MSXML; #include #include #include +#include #include #include #include diff --git a/daemon/postprocess/DupeMatcher.cpp b/daemon/postprocess/DupeMatcher.cpp index bf339bf0..329b8d17 100644 --- a/daemon/postprocess/DupeMatcher.cpp +++ b/daemon/postprocess/DupeMatcher.cpp @@ -67,12 +67,12 @@ bool RarLister::FindLargestFile(DupeMatcher* owner, const char* directory, unrar.m_filenameBuf = filenameBuf; unrar.m_filenameBufLen = filenameBufLen; - char** cmdArgs = nullptr; - if (!Util::SplitCommandLine(g_Options->GetUnrarCmd(), &cmdArgs)) + std::vector cmdArgs = Util::SplitCommandLine(g_Options->GetUnrarCmd()); + if (cmdArgs.empty()) { return false; } - const char* unrarPath = *cmdArgs; + const char* unrarPath = cmdArgs[0]; unrar.SetScript(unrarPath); const char* args[4]; @@ -106,12 +106,6 @@ bool RarLister::FindLargestFile(DupeMatcher* owner, const char* directory, usleep(200 * 1000); } - for (char** argPtr = cmdArgs; *argPtr; argPtr++) - { - free(*argPtr); - } - free(cmdArgs); - *maxSize = unrar.m_maxSize; *compressed = unrar.m_compressed; diff --git a/daemon/postprocess/Unpack.cpp b/daemon/postprocess/Unpack.cpp index e5bf1835..f94888ea 100644 --- a/daemon/postprocess/Unpack.cpp +++ b/daemon/postprocess/Unpack.cpp @@ -361,8 +361,8 @@ bool UnpackController::PrepareCmdParams(const char* command, ParamList* params, return true; } - char** cmdArgs = nullptr; - if (!Util::SplitCommandLine(command, &cmdArgs)) + std::vector cmdArgs = Util::SplitCommandLine(command); + if (cmdArgs.empty()) { PrintMessage(Message::mkError, "Could not start %s, failed to parse command line: %s", infoName, command); m_unpackOk = false; @@ -370,11 +370,7 @@ bool UnpackController::PrepareCmdParams(const char* command, ParamList* params, return false; } - for (char** argPtr = cmdArgs; *argPtr; argPtr++) - { - params->emplace_back(*argPtr); - } - free(cmdArgs); + std::move(cmdArgs.begin(), cmdArgs.end(), std::back_inserter(*params)); return true; } diff --git a/daemon/util/Util.cpp b/daemon/util/Util.cpp index 12fc75a3..9a7b5c98 100644 --- a/daemon/util/Util.cpp +++ b/daemon/util/Util.cpp @@ -306,11 +306,10 @@ bool Util::MatchFileExt(const char* filename, const char* extensionList, const c return false; } -bool Util::SplitCommandLine(const char* commandLine, char*** argv) +std::vector Util::SplitCommandLine(const char* commandLine) { - int argCount = 0; + std::vector result; char buf[1024]; - char* pszArgList[100]; uint32 len = 0; bool escaping = false; bool space = true; @@ -358,15 +357,11 @@ bool Util::SplitCommandLine(const char* commandLine, char*** argv) } } - if ((space || !*p) && len > 0 && argCount < 100) + if ((space || !*p) && len > 0) { //add token buf[len] = '\0'; - if (argv) - { - pszArgList[argCount] = strdup(buf); - } - (argCount)++; + result.emplace_back(buf); len = 0; } @@ -376,14 +371,7 @@ bool Util::SplitCommandLine(const char* commandLine, char*** argv) } } - if (argv) - { - pszArgList[argCount] = nullptr; - *argv = (char**)malloc((argCount + 1) * sizeof(char*)); - memcpy(*argv, pszArgList, sizeof(char*) * (argCount + 1)); - } - - return argCount > 0; + return result; } void Util::TrimRight(char* str) diff --git a/daemon/util/Util.h b/daemon/util/Util.h index 62d65b84..30794c9e 100644 --- a/daemon/util/Util.h +++ b/daemon/util/Util.h @@ -43,16 +43,9 @@ public: /* * Split command line into arguments. * Uses spaces and single quotation marks as separators. - * Returns bool if sucessful or false if bad escaping was detected. - * Parameter "argv" may be nullptr if only a syntax check is needed. - * Parsed parameters returned in Array "argv", which contains at least one element. - * The last element in array is nullptr. - * Restrictions: the number of arguments is limited to 100 and each argument must - * be maximum 1024 chars long. - * If these restrictions are exceeded, only first 100 arguments and only first 1024 - * for each argument are returned (the function still returns "true"). + * May return empty list if bad escaping was detected. */ - static bool SplitCommandLine(const char* commandLine, char*** argv); + static std::vector SplitCommandLine(const char* commandLine); static int64 JoinInt64(uint32 Hi, uint32 Lo); static void SplitInt64(int64 Int64, uint32* Hi, uint32* Lo);