From 2a23d2e204a983690234f6b70283508ccf57c21a Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Thu, 19 Oct 2023 22:57:55 -0700 Subject: [PATCH] Fix CircleCI build, more coverity fixes. --- .circleci/config.yml | 1 - .clang-tidy | 14 +++++--------- CMakeLists.txt | 1 + include/scsl/Flags.h | 2 +- src/sl/Buffer.cc | 8 +++++++- src/sl/Flags.cc | 18 +++++++++++------- src/sl/StringUtil.cc | 4 ++-- src/test/SimpleSuite.cc | 4 ++-- test/stringutil.cc | 14 +++++++++++++- 9 files changed, 42 insertions(+), 24 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6c58b11..cceed2e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -33,4 +33,3 @@ workflows: ctest: jobs: - ctest - - static_analysis diff --git a/.clang-tidy b/.clang-tidy index 1b80d39..f287a48 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -12,6 +12,7 @@ Checks: >- -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-avoid-non-const-global-variables, -cppcoreguidelines-pro-bounds-array-to-pointer-decay, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, -cppcoreguidelines-pro-type-vararg, -google-readability-braces-around-statements, -google-readability-function-size, @@ -20,14 +21,9 @@ Checks: >- -modernize-use-nodiscard, -modernize-use-trailing-return-type, -performance-unnecessary-value-param, - -readability-magic-numbers, + -readability-magic-numbers CheckOptions: - - key: readability-function-cognitive-complexity.Threshold - value: 100 - - key: readability-function-cognitive-complexity.IgnoreMacros - value: true - # Set naming conventions for your style below (there are dozens of naming settings possible): - # See https://clang.llvm.org/extra/clang-tidy/checks/readability/identifier-naming.html - - key: readability-identifier-naming.ClassCase - value: CamelCase + readability-function-cognitive-complexity.Threshold: 100 + readability-function-cognitive-complexity.IgnoreMacros: true + readability-identifier-naming.ClassCase: CamelCase diff --git a/CMakeLists.txt b/CMakeLists.txt index ca4b7cb..4eaf121 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,6 +6,7 @@ project(scsl LANGUAGES CXX set(CMAKE_CXX_STANDARD 14) set(CMAKE_VERBOSE_MAKEFILES TRUE) set(VERBOSE YES) +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # compile options: # -Wall Default to all errors. diff --git a/include/scsl/Flags.h b/include/scsl/Flags.h index afe5250..f8817f0 100644 --- a/include/scsl/Flags.h +++ b/include/scsl/Flags.h @@ -329,7 +329,7 @@ public: private: ParseStatus parseArg(int argc, char **argv, int &index); - Flag *checkGetArg(std::string fName, FlagType eType); + Flag *checkGetArg(std::string& fName, FlagType eType); std::string name; std::string description; diff --git a/src/sl/Buffer.cc b/src/sl/Buffer.cc index fd907ef..8775c38 100644 --- a/src/sl/Buffer.cc +++ b/src/sl/Buffer.cc @@ -207,7 +207,13 @@ Buffer::Resize(size_t newCapacity) auto newContents = new uint8_t[newCapacity]; memset(newContents, 0, newCapacity); - if (this->length > 0) { + + // Defensive coding check. + if ((this->length > 0) && (this->contents == nullptr)) { + abort(); + } + + if (this->length > 0 && this->contents != nullptr) { memcpy(newContents, this->contents, this->length); } diff --git a/src/sl/Flags.cc b/src/sl/Flags.cc index 8f879c5..ec0f8cc 100644 --- a/src/sl/Flags.cc +++ b/src/sl/Flags.cc @@ -21,11 +21,12 @@ /// +#include #include #include #include -#include +#include #include #include @@ -76,14 +77,14 @@ Flags::Flags(std::string fName) Flags::Flags(std::string fName, std::string fDescription) - : name(fName), description(fDescription) + : name(std::move(fName)), description(std::move(fDescription)) { } Flags::~Flags() { - for (auto flag : this->flags) { + for (auto &flag : this->flags) { if (flag.second->Type == FlagType::String) { delete flag.second->Value.s; } @@ -103,7 +104,8 @@ Flags::Register(std::string fName, FlagType fType, std::string fDescription) return false; } - auto flag = NewFlag(fName, fType, fDescription); + auto flag = NewFlag(fName, fType, std::move(fDescription)); + assert(flag != nullptr); this->flags[fName] = flag; return true; } @@ -124,7 +126,7 @@ Flags::Register(std::string fName, bool defaultValue, std::string fDescription) bool Flags::Register(std::string fName, int defaultValue, std::string fDescription) { - if (!this->Register(fName, FlagType::Integer, fDescription)) { + if (!this->Register(fName, FlagType::Integer, std::move(fDescription))) { return false; } @@ -139,6 +141,8 @@ Flags::Register(std::string fName, unsigned int defaultValue, std::string fDescr if (!this->Register(fName, FlagType::UnsignedInteger, std::move(fDescription))) { return false; } + assert(this->flags.count(fName) != 0); + assert(this->flags[fName] != nullptr); this->flags[fName]->Value.u = defaultValue; return true; @@ -164,7 +168,7 @@ Flags::Register(std::string fName, std::string defaultValue, std::string fDescri return false; } - this->flags[fName]->Value.s = new std::string(defaultValue); + this->flags[fName]->Value.s = new std::string(std::move(defaultValue)); return true; } @@ -368,7 +372,7 @@ Flags::Arg(size_t i) Flag * -Flags::checkGetArg(std::string fName, FlagType eType) +Flags::checkGetArg(std::string& fName, FlagType eType) { if (this->flags[fName] == 0) { return nullptr; diff --git a/src/sl/StringUtil.cc b/src/sl/StringUtil.cc index 3666532..6199d54 100644 --- a/src/sl/StringUtil.cc +++ b/src/sl/StringUtil.cc @@ -62,7 +62,7 @@ SplitKeyValuePair(std::string line, char delimiter) std::string sDelim; sDelim.push_back(std::move(delimiter)); - return SplitKeyValuePair(line, sDelim); + return SplitKeyValuePair(std::move(line), sDelim); } @@ -154,7 +154,7 @@ WrapText(std::string& line, size_t lineLength) } std::string wLine; - for (auto word: parts) { + for (auto &word: parts) { if (word.size() == 0) { continue; } diff --git a/src/test/SimpleSuite.cc b/src/test/SimpleSuite.cc index 8495783..4d21e88 100755 --- a/src/test/SimpleSuite.cc +++ b/src/test/SimpleSuite.cc @@ -53,7 +53,7 @@ SimpleSuite::Silence() void SimpleSuite::AddTest(std::string name, std::function test) { - UnitTest const test_case = {name, test}; + const UnitTest test_case = {std::move(name), test}; tests.push_back(test_case); } @@ -62,7 +62,7 @@ void SimpleSuite::AddFailingTest(std::string name, std::function test) { // auto ntest = [&test]() { return !test(); }; - UnitTest test_case = {name, [&test]() { return !test(); }}; + const UnitTest test_case = {std::move(name), [&test]() { return !test(); }}; tests.push_back(test_case); } diff --git a/test/stringutil.cc b/test/stringutil.cc index 9612342..d278efb 100644 --- a/test/stringutil.cc +++ b/test/stringutil.cc @@ -80,6 +80,17 @@ TestSplit(std::string line, std::string delim, size_t maxCount, std::vector{"hello", "world"}; + const auto *inputLine = "hello=world\n"; + auto actual = U::S::SplitKeyValuePair(inputLine, '='); + + return actual == expected; +} + + bool TestWrapping() { @@ -115,7 +126,7 @@ TestWrapping() return false; } - U::S::WriteTabIndented(std::cout, wrapped, 4, true); +// U::S::WriteTabIndented(std::cout, wrapped, 4, true); return true; } @@ -142,6 +153,7 @@ main() suite.AddTest("SplitN(0) with empty element", TestSplit("abc::def:ghi", ":", 0, std::vector{"abc", "", "def", "ghi"})); + suite.AddTest("TestSplitKV(char)", TestSplitChar); suite.AddTest("TextWrapping", TestWrapping); auto result = suite.Run(); std::cout << suite.GetReport() << "\n";