From f5e51eace71b103f6470d8393509e0abdb3c99ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Sat, 8 Apr 2023 22:29:09 +0200 Subject: [PATCH] do not use string-to-integer conversions without error handling (#4906) --- .clang-tidy | 2 +- .selfcheck_suppressions | 1 + cli/cmdlineparser.cpp | 90 +++++++--------- cli/cmdlineparser.h | 20 ++++ lib/checkbufferoverrun.cpp | 3 +- lib/checkclass.cpp | 6 +- lib/checkio.cpp | 6 +- lib/checkunusedfunctions.cpp | 2 +- lib/clangimport.cpp | 13 +-- lib/cppcheck.cpp | 8 +- lib/errorlogger.cpp | 33 +++--- lib/importproject.cpp | 7 +- lib/library.cpp | 24 ++--- lib/mathlib.cpp | 3 +- lib/mathlib.h | 3 + lib/suppressions.cpp | 4 +- lib/utils.h | 82 ++++++++++++++ releasenotes.txt | 1 + test/fixture.h | 1 + test/testcmdlineparser.cpp | 203 +++++++++++++++++++++++++++++++++-- test/testerrorlogger.cpp | 8 +- test/testutils.cpp | 149 +++++++++++++++++++++++++ 22 files changed, 549 insertions(+), 120 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 6dccb846e..8542591b8 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace' +Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace,cert-err34-c' WarningsAsErrors: '*' HeaderFilterRegex: '(cli|gui|lib|oss-fuzz|test|triage)\/[a-z]+\.h' CheckOptions: diff --git a/.selfcheck_suppressions b/.selfcheck_suppressions index c61108cdc..b6fd4de66 100644 --- a/.selfcheck_suppressions +++ b/.selfcheck_suppressions @@ -5,6 +5,7 @@ bitwiseOnBoolean # temporary suppressions - fix the warnings! simplifyUsing:lib/valueptr.h varid0:gui/projectfile.cpp +templateInstantiation # warnings in Qt generated code we cannot fix symbolDatabaseWarning:gui/temp/moc_*.cpp diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 1366f1d06..b6233aa48 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -260,7 +260,8 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--checks-max-time=", 18) == 0) { - mSettings.checksMaxTime = std::atoi(argv[i] + 18); + if (!parseNumberArg(argv[i], 18, mSettings.checksMaxTime)) + return false; } else if (std::strcmp(argv[i], "--clang") == 0) { @@ -286,6 +287,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--cppcheck-build-dir=", 21) == 0) { + // TODO: bail out when the folder does not exist? will silently do nothing mSettings.buildDir = Path::fromNativeSeparators(argv[i] + 21); if (endsWith(mSettings.buildDir, '/')) mSettings.buildDir.pop_back(); @@ -365,12 +367,8 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // --error-exitcode=1 else if (std::strncmp(argv[i], "--error-exitcode=", 17) == 0) { - const std::string temp = argv[i]+17; - std::istringstream iss(temp); - if (!(iss >> mSettings.exitCode)) { - printError("argument must be an integer. Try something like '--error-exitcode=1'."); + if (!parseNumberArg(argv[i], 17, mSettings.exitCode)) return false; - } } // Exception handling inside cppcheck client @@ -505,18 +503,19 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else numberString = argv[i]+2; - std::istringstream iss(numberString); - if (!(iss >> mSettings.jobs)) { - printError("argument to '-j' is not a number."); + unsigned int tmp; + std::string err; + if (!strToInt(numberString, tmp, &err)) { + printError("argument to '-j' is not valid - " + err + "."); return false; } - - if (mSettings.jobs > 10000) { + if (tmp > 10000) { // This limit is here just to catch typos. If someone has // need for more jobs, this value should be increased. printError("argument for '-j' is allowed to be 10000 at max."); return false; } + mSettings.jobs = tmp; } #ifdef THREADING_MODEL_FORK @@ -538,11 +537,13 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else numberString = argv[i]+2; - std::istringstream iss(numberString); - if (!(iss >> mSettings.loadAverage)) { - printError("argument to '-l' is not a number."); + int tmp; + std::string err; + if (!strToInt(numberString, tmp, &err)) { + printError("argument to '-l' is not valid - " + err + "."); return false; } + mSettings.loadAverage = tmp; } #endif @@ -577,25 +578,24 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Set maximum number of #ifdef configurations to check else if (std::strncmp(argv[i], "--max-configs=", 14) == 0) { - mSettings.force = false; - - std::istringstream iss(14+argv[i]); - if (!(iss >> mSettings.maxConfigs)) { - printError("argument to '--max-configs=' is not a number."); + int tmp; + if (!parseNumberArg(argv[i], 14, tmp)) return false; - } - - if (mSettings.maxConfigs < 1) { + if (tmp < 1) { printError("argument to '--max-configs=' must be greater than 0."); return false; } + mSettings.maxConfigs = tmp; + mSettings.force = false; maxconfigs = true; } // max ctu depth - else if (std::strncmp(argv[i], "--max-ctu-depth=", 16) == 0) - mSettings.maxCtuDepth = std::atoi(argv[i] + 16); + else if (std::strncmp(argv[i], "--max-ctu-depth=", 16) == 0) { + if (!parseNumberArg(argv[i], 16, mSettings.maxCtuDepth)) + return false; + } // Write results in file else if (std::strncmp(argv[i], "--output-file=", 14) == 0) @@ -603,11 +603,15 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis // is always executed. - else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) - mSettings.performanceValueFlowMaxTime = std::atoi(argv[i] + 33); + else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) { + if (!parseNumberArg(argv[i], 33, mSettings.performanceValueFlowMaxTime, true)) + return false; + } - else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) - mSettings.performanceValueFlowMaxIfCount = std::atoi(argv[i] + 37); + else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) { + if (!parseNumberArg(argv[i], 37, mSettings.performanceValueFlowMaxIfCount, true)) + return false; + } // Specify platform else if (std::strncmp(argv[i], "--platform=", 11) == 0) { @@ -941,26 +945,18 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--template-max-time=", 20) == 0) { - mSettings.templateMaxTime = std::atoi(argv[i] + 20); + if (!parseNumberArg(argv[i], 20, mSettings.templateMaxTime)) + return false; } else if (std::strncmp(argv[i], "--typedef-max-time=", 19) == 0) { - mSettings.typedefMaxTime = std::atoi(argv[i] + 19); + if (!parseNumberArg(argv[i], 19, mSettings.typedefMaxTime)) + return false; } else if (std::strncmp(argv[i], "--valueflow-max-iterations=", 27) == 0) { - long tmp; - try { - tmp = std::stol(argv[i] + 27); - } catch (const std::invalid_argument &) { - printError("argument to '--valueflow-max-iteration' is invalid."); + if (!parseNumberArg(argv[i], 27, mSettings.valueFlowMaxIterations)) return false; - } - if (tmp < 0) { - printError("argument to '--valueflow-max-iteration' needs to be at least 0."); - return false; - } - mSettings.valueFlowMaxIterations = static_cast(tmp); } else if (std::strcmp(argv[i], "-v") == 0 || std::strcmp(argv[i], "--verbose") == 0) @@ -979,20 +975,16 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Define the XML file version (and enable XML output) else if (std::strncmp(argv[i], "--xml-version=", 14) == 0) { - const std::string numberString(argv[i]+14); - - std::istringstream iss(numberString); - if (!(iss >> mSettings.xml_version)) { - printError("argument to '--xml-version' is not a number."); + int tmp; + if (!parseNumberArg(argv[i], 14, tmp)) return false; - } - - if (mSettings.xml_version != 2) { + if (tmp != 2) { // We only have xml version 2 printError("'--xml-version' can only be 2."); return false; } + mSettings.xml_version = tmp; // Enable also XML if version is set mSettings.xml = true; } diff --git a/cli/cmdlineparser.h b/cli/cmdlineparser.h index d7be0f7be..a5f3e4adb 100644 --- a/cli/cmdlineparser.h +++ b/cli/cmdlineparser.h @@ -22,6 +22,8 @@ #include #include +#include "utils.h" + class Settings; /// @addtogroup CLI @@ -118,6 +120,24 @@ protected: private: bool isCppcheckPremium() const; + // TODO: get rid of is_signed + template + static bool parseNumberArg(const char* const arg, std::size_t offset, T& num, bool is_signed = false) + { + T tmp; + std::string err; + if (!strToInt(arg + offset, tmp, &err)) { + printError("argument to '" + std::string(arg, offset) + "' is not valid - " + err + "."); + return false; + } + if (is_signed && tmp < 0) { + printError("argument to '" + std::string(arg, offset) + "' needs to be a positive integer."); + return false; + } + num = tmp; + return true; + } + std::vector mPathNames; std::vector mIgnoredPaths; Settings &mSettings; diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 0755926bb..7b1a040ae 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -143,13 +143,14 @@ static int getMinFormatStringOutputLength(const std::vector ¶m outputStringSize++; if (handleNextParameter) { + // NOLINTNEXTLINE(cert-err34-c) - intentional use int tempDigits = std::abs(std::atoi(digits_string.c_str())); if (i_d_x_f_found) tempDigits = std::max(tempDigits, 1); if (digits_string.find('.') != std::string::npos) { const std::string endStr = digits_string.substr(digits_string.find('.') + 1); - const int maxLen = std::max(std::abs(std::atoi(endStr.c_str())), 1); + const int maxLen = std::max(std::abs(strToInt(endStr)), 1); if (formatString[i] == 's') { // For strings, the length after the dot "%.2s" will limit diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 4dc0ac554..fdf0bcdf3 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3224,9 +3224,9 @@ Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLElement *xm MyFileInfo::NameLoc nameLoc; nameLoc.className = name; nameLoc.fileName = file; - nameLoc.lineNumber = std::atoi(line); - nameLoc.column = std::atoi(col); - nameLoc.hash = MathLib::toULongNumber(hash); + nameLoc.lineNumber = strToInt(line); + nameLoc.column = strToInt(col); + nameLoc.hash = strToInt(hash); fileInfo->classDefinitions.push_back(std::move(nameLoc)); } } diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 2573d9246..a01ff0c30 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -643,7 +643,7 @@ void CheckIO::checkFormatString(const Token * const tok, } else if (std::isdigit(*i)) { width += *i; } else if (*i == '$') { - parameterPosition = std::atoi(width.c_str()); + parameterPosition = strToInt(width); hasParameterPosition = true; width.clear(); } @@ -695,7 +695,7 @@ void CheckIO::checkFormatString(const Token * const tok, specifier += (*i == 's' || bracketBeg == formatString.end()) ? std::string{ 's' } : std::string{ bracketBeg, i + 1 }; if (argInfo.variableInfo && argInfo.isKnownType() && argInfo.variableInfo->isArray() && (argInfo.variableInfo->dimensions().size() == 1) && argInfo.variableInfo->dimensions()[0].known) { if (!width.empty()) { - const int numWidth = std::atoi(width.c_str()); + const int numWidth = strToInt(width); if (numWidth != (argInfo.variableInfo->dimension(0) - 1)) invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, specifier); } @@ -718,7 +718,7 @@ void CheckIO::checkFormatString(const Token * const tok, case 'c': if (argInfo.variableInfo && argInfo.isKnownType() && argInfo.variableInfo->isArray() && (argInfo.variableInfo->dimensions().size() == 1) && argInfo.variableInfo->dimensions()[0].known) { if (!width.empty()) { - const int numWidth = std::atoi(width.c_str()); + const int numWidth = strToInt(width); if (numWidth > argInfo.variableInfo->dimension(0)) invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, std::string(1, *i)); } diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index 1805b2bb2..35e8344de 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -442,7 +442,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo } else if (std::strcmp(e2->Name(),"functiondecl") == 0) { const char* lineNumber = e2->Attribute("lineNumber"); if (lineNumber) - decls[functionName] = Location(sourcefile, std::atoi(lineNumber)); + decls[functionName] = Location(sourcefile, strToInt(lineNumber)); } } } diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 4855fd50a..1872d3cab 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -501,11 +501,12 @@ void clangimport::AstNode::setLocations(TokenList *tokenList, int file, int line { for (const std::string &ext: mExtTokens) { if (ext.compare(0, 5, "(ext.substr(5, ext.find_first_of(",>", 5) - 5)); else if (ext.compare(0, 6, "(ext.substr(6, ext.find_first_of(":,>", 6) - 6)); + const auto pos = ext.find(", col:"); + if (pos != std::string::npos) + col = strToInt(ext.substr(pos+6, ext.find_first_of(":,>", pos+6) - (pos+6))); } else if (ext[0] == '<') { const std::string::size_type colon = ext.find(':'); if (colon != std::string::npos) { @@ -513,7 +514,7 @@ void clangimport::AstNode::setLocations(TokenList *tokenList, int file, int line const std::string::size_type sep1 = windowsPath ? ext.find(':', 4) : colon; const std::string::size_type sep2 = ext.find(':', sep1 + 1); file = tokenList->appendFileIfNew(ext.substr(1, sep1 - 1)); - line = MathLib::toLongNumber(ext.substr(sep1 + 1, sep2 - sep1 - 1)); + line = strToInt(ext.substr(sep1 + 1, sep2 - sep1 - 1)); } } } @@ -1551,7 +1552,7 @@ static void setValues(Tokenizer *tokenizer, SymbolDatabase *symbolDatabase) for (const Token *arrtok = tok->linkAt(1)->previous(); arrtok; arrtok = arrtok->previous()) { const std::string &a = arrtok->str(); if (a.size() > 2 && a[0] == '[' && a.back() == ']') - mul *= std::atoi(a.substr(1).c_str()); + mul *= strToInt(a.substr(1)); else break; } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index ec44c4a1d..8c760ac35 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -452,8 +452,8 @@ static bool reportClangErrors(std::istream &is, const std::function(linenr); + loc.column = strToInt(colnr); ErrorMessage errmsg({std::move(loc)}, locFile, Severity::error, @@ -1727,8 +1727,8 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings) const std::string errorString = line.substr(endErrorPos, line.length()); std::string fixedpath = Path::simplifyPath(line.substr(0, endNamePos)); - const int64_t lineNumber = std::atol(lineNumString.c_str()); - const int64_t column = std::atol(columnNumString.c_str()); + const int64_t lineNumber = strToInt(lineNumString); + const int64_t column = strToInt(columnNumString); fixedpath = Path::toNativeSeparators(fixedpath); ErrorMessage errmsg; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 7662d2fac..d3173b252 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -151,7 +151,7 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) severity = attr ? Severity::fromString(attr) : Severity::none; attr = errmsg->Attribute("cwe"); - std::istringstream(attr ? attr : "0") >> cwe.id; + cwe.id = attr ? strToInt(attr) : 0; attr = errmsg->Attribute("inconclusive"); certainty = (attr && (std::strcmp(attr, "true") == 0)) ? Certainty::inconclusive : Certainty::normal; @@ -163,7 +163,7 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) mVerboseMessage = attr ? attr : ""; attr = errmsg->Attribute("hash"); - std::istringstream(attr ? attr : "0") >> hash; + hash = attr ? strToInt(attr) : 0; for (const tinyxml2::XMLElement *e = errmsg->FirstChildElement(); e; e = e->NextSiblingElement()) { if (std::strcmp(e->Name(),"location")==0) { @@ -174,8 +174,8 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) const char *file = strfile ? strfile : unknown; const char *info = strinfo ? strinfo : ""; - const int line = strline ? std::atoi(strline) : 0; - const int column = strcolumn ? std::atoi(strcolumn) : 0; + const int line = strline ? strToInt(strline) : 0; + const int column = strcolumn ? strToInt(strcolumn) : 0; callStack.emplace_front(file, info, line, column); } else if (std::strcmp(e->Name(),"symbol")==0) { mSymbolNames += e->GetText(); @@ -301,26 +301,17 @@ void ErrorMessage::deserialize(const std::string &data) id = std::move(results[0]); severity = Severity::fromString(results[1]); - unsigned long long tmp = 0; + cwe.id = 0; if (!results[2].empty()) { - try { - tmp = MathLib::toULongNumber(results[2]); - } - catch (const InternalError&) { - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid CWE ID"); - } - if (tmp > std::numeric_limits::max()) - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - CWE ID is out of range"); + std::string err; + if (!strToInt(results[2], cwe.id, &err)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid CWE ID - " + err); } - cwe.id = static_cast(tmp); hash = 0; if (!results[3].empty()) { - try { - hash = MathLib::toULongNumber(results[3]); - } - catch (const InternalError&) { - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash"); - } + std::string err; + if (!strToInt(results[3], hash, &err)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash - " + err); } file0 = std::move(results[4]); mShortMessage = std::move(results[5]); @@ -373,7 +364,7 @@ void ErrorMessage::deserialize(const std::string &data) // (*loc).line << '\t' << (*loc).column << '\t' << (*loc).getfile(false) << '\t' << loc->getOrigFile(false) << '\t' << loc->getinfo(); - ErrorMessage::FileLocation loc(substrings[3], MathLib::toLongNumber(substrings[0]), MathLib::toLongNumber(substrings[1])); + ErrorMessage::FileLocation loc(substrings[3], strToInt(substrings[0]), strToInt(substrings[1])); loc.setfile(std::move(substrings[2])); if (substrings.size() == 5) loc.setinfo(substrings[4]); diff --git a/lib/importproject.cpp b/lib/importproject.cpp index 714b7a5dc..9e5475358 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -1147,6 +1147,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti guiProject.analyzeAllVsConfigs.clear(); + // TODO: this should support all available command-line options for (const tinyxml2::XMLElement *node = rootnode->FirstChildElement(); node; node = node->NextSiblingElement()) { if (strcmp(node->Name(), CppcheckXml::RootPathName) == 0) { if (node->Attribute(CppcheckXml::RootPathNameAttrib)) { @@ -1190,7 +1191,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti s.fileName = joinRelativePath(path, s.fileName); s.lineNumber = child->IntAttribute("lineNumber", Suppressions::Suppression::NO_LINE); s.symbolName = readSafe(child->Attribute("symbolName"), ""); - std::istringstream(readSafe(child->Attribute("hash"), "0")) >> s.hash; + s.hash = strToInt(readSafe(child->Attribute("hash"), "0")); suppressions.push_back(std::move(s)); } } else if (strcmp(node->Name(), CppcheckXml::VSConfigurationElementName) == 0) @@ -1218,9 +1219,9 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti else if (strcmp(node->Name(), CppcheckXml::CheckUnusedTemplatesElementName) == 0) temp.checkUnusedTemplates = (strcmp(readSafe(node->GetText(), ""), "true") == 0); else if (strcmp(node->Name(), CppcheckXml::MaxCtuDepthElementName) == 0) - temp.maxCtuDepth = std::atoi(readSafe(node->GetText(), "")); // TODO: get rid of atoi() + temp.maxCtuDepth = strToInt(readSafe(node->GetText(), "2")); // TODO: bail out when missing? else if (strcmp(node->Name(), CppcheckXml::MaxTemplateRecursionElementName) == 0) - temp.maxTemplateRecursion = std::atoi(readSafe(node->GetText(), "")); // TODO: get rid of atoi() + temp.maxTemplateRecursion = strToInt(readSafe(node->GetText(), "100")); // TODO: bail out when missing? else if (strcmp(node->Name(), CppcheckXml::CheckUnknownFunctionReturn) == 0) ; // TODO else if (strcmp(node->Name(), Settings::SafeChecks::XmlRootName) == 0) { diff --git a/lib/library.cpp b/lib/library.cpp index c33cdc97b..48cdec773 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -327,7 +327,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) if (!argString) return Error(ErrorCode::MISSING_ATTRIBUTE, "arg"); - mReflection[reflectionnode->GetText()] = atoi(argString); + mReflection[reflectionnode->GetText()] = strToInt(argString); } } @@ -402,7 +402,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) mExecutableBlocks[extension].setEnd(end); const char * offset = blocknode->Attribute("offset"); if (offset) - mExecutableBlocks[extension].setOffset(atoi(offset)); + mExecutableBlocks[extension].setOffset(strToInt(offset)); } else @@ -490,7 +490,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) if (containerNodeName == "size") { const char* const templateArg = containerNode->Attribute("templateParameter"); if (templateArg) - container.size_templateArgNo = atoi(templateArg); + container.size_templateArgNo = strToInt(templateArg); } else if (containerNodeName == "access") { const char* const indexArg = containerNode->Attribute("indexOperator"); if (indexArg) @@ -499,7 +499,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) } else if (containerNodeName == "type") { const char* const templateArg = containerNode->Attribute("templateParameter"); if (templateArg) - container.type_templateArgNo = atoi(templateArg); + container.type_templateArgNo = strToInt(templateArg); const char* const string = containerNode->Attribute("string"); if (string) @@ -521,7 +521,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) const char *memberTemplateParameter = memberNode->Attribute("templateParameter"); struct Container::RangeItemRecordTypeItem member; member.name = memberName ? memberName : ""; - member.templateParameter = memberTemplateParameter ? std::atoi(memberTemplateParameter) : -1; + member.templateParameter = memberTemplateParameter ? strToInt(memberTemplateParameter) : -1; container.rangeItemRecordType.emplace_back(std::move(member)); } } else @@ -584,7 +584,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) } const char * const size = node->Attribute("size"); if (size) - podType.size = atoi(size); + podType.size = strToInt(size); const char * const sign = node->Attribute("sign"); if (sign) podType.sign = *sign; @@ -710,7 +710,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co if (const char *type = functionnode->Attribute("type")) mReturnValueType[name] = type; if (const char *container = functionnode->Attribute("container")) - mReturnValueContainer[name] = std::atoi(container); + mReturnValueContainer[name] = strToInt(container); if (const char *unknownReturnValues = functionnode->Attribute("unknownValues")) { if (std::strcmp(unknownReturnValues, "all") == 0) { std::vector values{LLONG_MIN, LLONG_MAX}; @@ -723,7 +723,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co return Error(ErrorCode::MISSING_ATTRIBUTE, "nr"); const bool bAnyArg = strcmp(argNrString, "any") == 0; const bool bVariadicArg = strcmp(argNrString, "variadic") == 0; - const int nr = (bAnyArg || bVariadicArg) ? -1 : std::atoi(argNrString); + const int nr = (bAnyArg || bVariadicArg) ? -1 : strToInt(argNrString); ArgumentChecks &ac = func.argumentChecks[nr]; ac.optional = functionnode->Attribute("default") != nullptr; ac.variadic = bVariadicArg; @@ -743,7 +743,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co int indirect = 0; const char * const indirectStr = argnode->Attribute("indirect"); if (indirectStr) - indirect = atoi(indirectStr); + indirect = strToInt(indirectStr); if (argnodename == "not-bool") ac.notbool = true; else if (argnodename == "not-null") @@ -787,8 +787,8 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co return Error(ErrorCode::MISSING_ATTRIBUTE, "value"); long long minsizevalue = 0; try { - minsizevalue = MathLib::toLongNumber(valueattr); - } catch (const InternalError&) { + minsizevalue = strToInt(valueattr); + } catch (const std::runtime_error&) { return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, valueattr); } if (minsizevalue <= 0) @@ -1699,7 +1699,7 @@ std::shared_ptr createTokenFromExpression(const std::string& returnValue, for (Token* tok2 = tokenList->front(); tok2; tok2 = tok2->next()) { if (tok2->str().compare(0, 3, "arg") != 0) continue; - nonneg int const id = std::atoi(tok2->str().c_str() + 3); + nonneg int const id = strToInt(tok2->str().c_str() + 3); tok2->varId(id); if (lookupVarId) (*lookupVarId)[id] = tok2; diff --git a/lib/mathlib.cpp b/lib/mathlib.cpp index 446b7dc46..e332d2b8d 100644 --- a/lib/mathlib.cpp +++ b/lib/mathlib.cpp @@ -286,7 +286,7 @@ MathLib::value MathLib::value::shiftRight(const MathLib::value &v) const return ret; } - +// TODO: remove handling of non-literal stuff MathLib::biguint MathLib::toULongNumber(const std::string & str) { // hexadecimal numbers: @@ -365,6 +365,7 @@ unsigned int MathLib::encodeMultiChar(const std::string& str) }); } +// TODO: remove handling of non-literal stuff MathLib::bigint MathLib::toLongNumber(const std::string & str) { // hexadecimal numbers: diff --git a/lib/mathlib.h b/lib/mathlib.h index 8e0e790f4..1ed77f06c 100644 --- a/lib/mathlib.h +++ b/lib/mathlib.h @@ -69,12 +69,15 @@ public: using biguint = unsigned long long; static const int bigint_bits; + /** @brief for conversion of numeric literals - for atoi-like conversions please use strToInt() */ static bigint toLongNumber(const std::string & str); + /** @brief for conversion of numeric literals - for atoi-like conversions please use strToInt() */ static biguint toULongNumber(const std::string & str); template static std::string toString(T value) { return std::to_string(value); } + /** @brief for conversion of numeric literals */ static double toDoubleNumber(const std::string & str); static bool isInt(const std::string & str); diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index fcde8f5e7..37920e029 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -101,11 +101,11 @@ std::string Suppressions::parseXmlFile(const char *filename) else if (std::strcmp(e2->Name(), "fileName") == 0) s.fileName = text; else if (std::strcmp(e2->Name(), "lineNumber") == 0) - s.lineNumber = std::atoi(text); + s.lineNumber = strToInt(text); else if (std::strcmp(e2->Name(), "symbolName") == 0) s.symbolName = text; else if (*text && std::strcmp(e2->Name(), "hash") == 0) - std::istringstream(text) >> s.hash; + s.hash = strToInt(text); else return "Unknown suppression element \"" + std::string(e2->Name()) + "\", expected id/fileName/lineNumber/symbolName/hash"; } diff --git a/lib/utils.h b/lib/utils.h index 5cd2a823c..e942c82f1 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include @@ -177,6 +179,86 @@ CPPCHECKLIB bool matchglobs(const std::vector &patterns, const std: CPPCHECKLIB void strTolower(std::string& str); +template::value, bool>::type=true> +bool strToInt(const std::string& str, T &num, std::string* err = nullptr) +{ + long long tmp; + try { + std::size_t idx = 0; + tmp = std::stoll(str, &idx); + if (idx != str.size()) { + if (err) + *err = "not an integer"; + return false; + } + } catch (const std::out_of_range&) { + if (err) + *err = "out of range (stoll)"; + return false; + } catch (const std::invalid_argument &) { + if (err) + *err = "not an integer"; + return false; + } + if (str.front() == '-' && std::numeric_limits::min() == 0) { + if (err) + *err = "needs to be positive"; + return false; + } + if (tmp < std::numeric_limits::min() || tmp > std::numeric_limits::max()) { + if (err) + *err = "out of range (limits)"; + return false; + } + num = static_cast(tmp); + return true; +} + +template::value, bool>::type=true> +bool strToInt(const std::string& str, T &num, std::string* err = nullptr) +{ + unsigned long long tmp; + try { + std::size_t idx = 0; + tmp = std::stoull(str, &idx); + if (idx != str.size()) { + if (err) + *err = "not an integer"; + return false; + } + } catch (const std::out_of_range&) { + if (err) + *err = "out of range (stoull)"; + return false; + } catch (const std::invalid_argument &) { + if (err) + *err = "not an integer"; + return false; + } + if (str.front() == '-') { + if (err) + *err = "needs to be positive"; + return false; + } + if (tmp > std::numeric_limits::max()) { + if (err) + *err = "out of range (limits)"; + return false; + } + num = tmp; + return true; +} + +template +T strToInt(const std::string& str) +{ + T tmp = 0; + std::string err; + if (!strToInt(str, tmp, &err)) + throw std::runtime_error("converting '" + str + "' to integer failed - " + err); + return tmp; +} + /** * Simple helper function: * \return size of array diff --git a/releasenotes.txt b/releasenotes.txt index a9800e107..5f98590a3 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -13,3 +13,4 @@ release notes for cppcheck-2.11 - `constVariableReference` - `constVariablePointer` - Limit valueflow analysis in function if the "if" count is high. By default max limit is 100. Limit can be tweaked with --performance-valueflow-max-if-count +- More command-line parameters will now check if the given integer argument is actually valid. Several other internal string-to-integer conversions will not be error checked. diff --git a/test/fixture.h b/test/fixture.h index 47fc5e20e..d37a949d4 100644 --- a/test/fixture.h +++ b/test/fixture.h @@ -157,6 +157,7 @@ extern std::ostringstream output; #define ASSERT_EQUALS_MSG( EXPECTED, ACTUAL, MSG ) assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL, MSG) #define ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&) {} catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) #define ASSERT_THROW_EQUALS( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) +#define ASSERT_THROW_EQUALS_2( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.what()); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) #define ASSERT_NO_THROW( CMD ) do { try { CMD; } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } while (false) #define TODO_ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; } catch (const EXCEPTION&) {} catch (...) { assertThrow(__FILE__, __LINE__); } } while (false) #define TODO_ASSERT( CONDITION ) do { const bool condition=(CONDITION); todoAssertEquals(__FILE__, __LINE__, true, false, condition); } while (false) diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index d3510a300..f0756dea1 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -125,8 +125,10 @@ private: TEST_CASE(fileListInvalid); TEST_CASE(inlineSuppr); TEST_CASE(jobs); + TEST_CASE(jobs2); TEST_CASE(jobsMissingCount); TEST_CASE(jobsInvalid); + TEST_CASE(jobsTooBig); TEST_CASE(maxConfigs); TEST_CASE(maxConfigsMissingCount); TEST_CASE(maxConfigsInvalid); @@ -198,6 +200,28 @@ private: TEST_CASE(valueFlowMaxIterationsInvalid); TEST_CASE(valueFlowMaxIterationsInvalid2); TEST_CASE(valueFlowMaxIterationsInvalid3); + TEST_CASE(checksMaxTime); + TEST_CASE(checksMaxTime2); + TEST_CASE(checksMaxTimeInvalid); +#ifdef THREADING_MODEL_FORK + TEST_CASE(loadAverage); + TEST_CASE(loadAverage2); + TEST_CASE(loadAverageInvalid); +#endif + TEST_CASE(maxCtuDepth); + TEST_CASE(maxCtuDepthInvalid); + TEST_CASE(performanceValueflowMaxTime); + TEST_CASE(performanceValueflowMaxTimeInvalid); + TEST_CASE(performanceValueFlowMaxIfCount); + TEST_CASE(performanceValueFlowMaxIfCountInvalid); + TEST_CASE(templateMaxTime); + TEST_CASE(templateMaxTimeInvalid); + TEST_CASE(templateMaxTimeInvalid2); + TEST_CASE(typedefMaxTime); + TEST_CASE(typedefMaxTimeInvalid); + TEST_CASE(typedefMaxTimeInvalid2); + TEST_CASE(templateMaxTime); + TEST_CASE(templateMaxTime); TEST_CASE(ignorepaths1); TEST_CASE(ignorepaths2); @@ -850,7 +874,7 @@ private: const char * const argv[] = {"cppcheck", "--error-exitcode=", "file.cpp"}; // Fails since exit code not given ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument must be an integer. Try something like '--error-exitcode=1'.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--error-exitcode=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void errorExitcodeStr() { @@ -858,7 +882,7 @@ private: const char * const argv[] = {"cppcheck", "--error-exitcode=foo", "file.cpp"}; // Fails since invalid exit code ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument must be an integer. Try something like '--error-exitcode=1'.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--error-exitcode=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void exitcodeSuppressionsOld() { @@ -934,12 +958,21 @@ private: ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); } + void jobs2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j3", "file.cpp"}; + settings.jobs = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(3, settings.jobs); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + void jobsMissingCount() { REDIRECT; const char * const argv[] = {"cppcheck", "-j", "file.cpp"}; // Fails since -j is missing thread count ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '-j' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '-j' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void jobsInvalid() { @@ -947,7 +980,14 @@ private: const char * const argv[] = {"cppcheck", "-j", "e", "file.cpp"}; // Fails since invalid count given for -j ASSERT_EQUALS(false, defParser.parseFromArgs(4, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '-j' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '-j' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void jobsTooBig() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j10001", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument for '-j' is allowed to be 10000 at max.\n", GET_REDIRECT_OUTPUT); } void maxConfigs() { @@ -966,7 +1006,7 @@ private: const char * const argv[] = {"cppcheck", "--max-configs=", "file.cpp"}; // Fails since --max-configs= is missing limit ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void maxConfigsInvalid() { @@ -974,7 +1014,7 @@ private: const char * const argv[] = {"cppcheck", "--max-configs=e", "file.cpp"}; // Fails since invalid count given for --max-configs= ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void maxConfigsTooSmall() { @@ -1472,7 +1512,7 @@ private: const char * const argv[] = {"cppcheck", "--xml", "--xml-version=a", "file.cpp"}; // FAils since unknown XML format version ASSERT_EQUALS(false, defParser.parseFromArgs(4, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--xml-version' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--xml-version=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void doc() { @@ -1632,15 +1672,160 @@ private: REDIRECT; const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=seven"}; ASSERT(!defParser.parseFromArgs(2, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iteration' is invalid.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iterations=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void valueFlowMaxIterationsInvalid3() { REDIRECT; const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=-1"}; ASSERT(!defParser.parseFromArgs(2, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iteration' needs to be at least 0.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iterations=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); } + + void checksMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=12", "file.cpp"}; + settings.checksMaxTime = SIZE_MAX; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.checksMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void checksMaxTime2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--checks-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + + void checksMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--checks-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + +#ifdef THREADING_MODEL_FORK + void loadAverage() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l", "12", "file.cpp"}; + settings.loadAverage = 0; + ASSERT(defParser.parseFromArgs(4, argv)); + ASSERT_EQUALS(12, settings.loadAverage); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void loadAverage2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l12", "file.cpp"}; + settings.loadAverage = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.loadAverage); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void loadAverageInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l", "one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(4, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '-l' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } +#endif + + void maxCtuDepth() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--max-ctu-depth=12", "file.cpp"}; + settings.maxCtuDepth = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.maxCtuDepth); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void maxCtuDepthInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--max-ctu-depth=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--max-ctu-depth=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void performanceValueflowMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=12", "file.cpp"}; + settings.performanceValueFlowMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.performanceValueFlowMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void performanceValueflowMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--performance-valueflow-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void performanceValueFlowMaxIfCount() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=12", "file.cpp"}; + settings.performanceValueFlowMaxIfCount = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.performanceValueFlowMaxIfCount); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void performanceValueFlowMaxIfCountInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--performance-valueflow-max-if-count=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void templateMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=12", "file.cpp"}; + settings.templateMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.templateMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void templateMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--template-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void templateMaxTimeInvalid2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--template-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=12", "file.cpp"}; + settings.typedefMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.typedefMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--typedef-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTimeInvalid2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--typedef-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + void ignorepaths1() { REDIRECT; const char * const argv[] = {"cppcheck", "-isrc", "file.cpp"}; diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 0490c4c20..abe9890ce 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -336,27 +336,27 @@ private: // invalid CWE ID const char str[] = "7 errorId" "5 error" - "7 invalid" + "7 invalid" // cwe "1 0" "8 test.cpp" "17 Programming error" "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid CWE ID"); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid CWE ID - not an integer"); } { // invalid hash const char str[] = "7 errorId" "5 error" "1 0" - "7 invalid" + "7 invalid" // hash "8 test.cpp" "17 Programming error" "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid hash"); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid hash - not an integer"); } { // out-of-range CWE ID diff --git a/test/testutils.cpp b/test/testutils.cpp index 6ef95fa9e..612b92da4 100644 --- a/test/testutils.cpp +++ b/test/testutils.cpp @@ -33,6 +33,7 @@ private: TEST_CASE(matchglob); TEST_CASE(isStringLiteral); TEST_CASE(isCharLiteral); + TEST_CASE(strToInt); } void isValidGlobPattern() const { @@ -179,6 +180,154 @@ private: ASSERT_EQUALS(true, ::isCharLiteral("U'test'")); ASSERT_EQUALS(true, ::isCharLiteral("L'test'")); } + + void strToInt() { + ASSERT_EQUALS(1, ::strToInt("1")); + ASSERT_EQUALS(-1, ::strToInt("-1")); + ASSERT_EQUALS(1, ::strToInt("1")); + ASSERT_THROW_EQUALS_2(::strToInt(""), std::runtime_error, "converting '' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(""), std::runtime_error, "converting '' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(" "), std::runtime_error, "converting ' ' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(" "), std::runtime_error, "converting ' ' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("-1"), std::runtime_error, "converting '-1' to integer failed - needs to be positive"); + ASSERT_THROW_EQUALS_2(::strToInt("1ms"), std::runtime_error, "converting '1ms' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1.0"), std::runtime_error, "converting '1.0' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("one"), std::runtime_error, "converting 'one' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1ms"), std::runtime_error, "converting '1ms' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1.0"), std::runtime_error, "converting '1.0' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("one"), std::runtime_error, "converting 'one' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '2147483648' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1)), std::runtime_error, "converting '-2147483649' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '128' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1)), std::runtime_error, "converting '-129' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '4294967296' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt("9223372036854775808"), std::runtime_error, "converting '9223372036854775808' to integer failed - out of range (stoll)"); // LLONG_MAX + 1 + ASSERT_THROW_EQUALS_2(::strToInt("18446744073709551616"), std::runtime_error, "converting '18446744073709551616' to integer failed - out of range (stoull)"); // ULLONG_MAX + 1 + + { + long tmp; + ASSERT(::strToInt("1", tmp)); + ASSERT_EQUALS(1, tmp); + } + { + long tmp; + ASSERT(::strToInt("-1", tmp)); + ASSERT_EQUALS(-1, tmp); + } + { + std::size_t tmp; + ASSERT(::strToInt("1", tmp)); + } + { + std::size_t tmp; + ASSERT(!::strToInt("-1", tmp)); + } + { + long tmp; + ASSERT(!::strToInt("1ms", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt("1ms", tmp)); + } + { + long tmp; + ASSERT(!::strToInt("", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt("", tmp)); + } + { + long tmp; + ASSERT(!::strToInt(" ", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt(" ", tmp)); + } + + { + long tmp; + std::string err; + ASSERT(::strToInt("1", tmp, &err)); + ASSERT_EQUALS(1, tmp); + ASSERT_EQUALS("", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(::strToInt("1", tmp, &err)); + ASSERT_EQUALS(1, tmp); + ASSERT_EQUALS("", err); + } + { + unsigned long tmp; + std::string err; + ASSERT(!::strToInt("-1", tmp, &err)); + ASSERT_EQUALS("needs to be positive", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("1ms", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("1.0", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("one", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(!::strToInt("1ms", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("9223372036854775808", tmp, &err)); // LLONG_MAX + 1 + ASSERT_EQUALS("out of range (stoll)", err); + } + { + int tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int8_t tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int8_t tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(!::strToInt("18446744073709551616", tmp, &err)); // ULLONG_MAX + 1 + ASSERT_EQUALS("out of range (stoull)", err); + } + } }; REGISTER_TEST(TestUtils)