From a1ba232e2a092ddb0a793fec3585163fb2ed2202 Mon Sep 17 00:00:00 2001 From: Goncalo Mao-Cheia Date: Sat, 25 Oct 2025 16:29:22 +0100 Subject: [PATCH 1/6] Add check for npos, when checkInternal receives file with missing dot --- lib/cppcheck.cpp | 33 +++++++++++++++++++++++---------- lib/cppcheck.h | 2 ++ test/testcppcheck.cpp | 28 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 2048d6f8098..a623cbc5f25 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -884,6 +884,27 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string return checkInternal(file, cfgname, fileIndex, f); } +bool CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector& files) +{ + if (!mSettings.plistOutput.empty()) { + const bool slashFound = file.spath().find('/') != std::string::npos; + std::string filename = slashFound ? file.spath().substr(file.spath().rfind('/') + 1) : file.spath(); + const std::size_t dotPosition = filename.find('.'); + + if(dotPosition == std::string::npos) { + ErrorMessage::FileLocation loc(filename, 0, 0); + ErrorMessage errmsg({std::move(loc)}, "", Severity::error, "filename does not contain dot", "filenameError", Certainty::normal); + mErrorLogger.reportErr(errmsg); + return false; + } + + const std::size_t fileNameHash = std::hash {}(file.spath()); + filename = mSettings.plistOutput + filename.substr(0, filename.find('.')) + "_" + std::to_string(fileNameHash) + ".plist"; + mLogger->openPlist(filename, files); + } + return true; +} + unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::string &cfgname, int fileIndex, const CreateTokenListFn& createTokenList) { // TODO: move to constructor when CppCheck no longer owns the settings @@ -989,16 +1010,8 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str if (!preprocessor.loadFiles(tokens1, files)) return mLogger->exitcode(); - if (!mSettings.plistOutput.empty()) { - std::string filename2; - if (file.spath().find('/') != std::string::npos) - filename2 = file.spath().substr(file.spath().rfind('/') + 1); - else - filename2 = file.spath(); - const std::size_t fileNameHash = std::hash {}(file.spath()); - filename2 = mSettings.plistOutput + filename2.substr(0, filename2.find('.')) + "_" + std::to_string(fileNameHash) + ".plist"; - mLogger->openPlist(filename2, files); - } + if (!checkPlistOutput(file, files)) + return mLogger->exitcode(); std::string dumpProlog; if (mSettings.dump || !mSettings.addons.empty()) { diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 9108ac28430..1d6d21d0cc9 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -185,6 +185,8 @@ class CPPCHECKLIB CppCheck { */ unsigned int checkFile(const FileWithDetails& file, const std::string &cfgname, int fileIndex); + bool checkPlistOutput(const FileWithDetails& file, const std::vector& files); + /** * @brief Check a file using buffer * @param file the file diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp index 643355ad2c3..8f902761069 100644 --- a/test/testcppcheck.cpp +++ b/test/testcppcheck.cpp @@ -79,6 +79,7 @@ class TestCppcheck : public TestFixture { TEST_CASE(isPremiumCodingStandardId); TEST_CASE(getDumpFileContentsRawTokens); TEST_CASE(getDumpFileContentsLibrary); + TEST_CASE(checkPlistOutput); TEST_CASE(premiumResultsCache); TEST_CASE(toomanyconfigs); TEST_CASE(purgedConfiguration); @@ -522,6 +523,33 @@ class TestCppcheck : public TestFixture { } } + void checkPlistOutput() const { + Suppressions supprs; + ErrorLogger2 errorLogger; + std::vector files = {"file.txt"}; + + { + const auto s = dinit(Settings, $.templateFormat = templateFormat, $.plistOutput = "output"); + ScopedFile file("file", ""); + CppCheck cppcheck(s, supprs, errorLogger, false, {}); + ASSERT_EQUALS(false, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + } + + { + const auto s = dinit(Settings, $.plistOutput = "output"); + ScopedFile file("file.c", ""); + CppCheck cppcheck(s, supprs, errorLogger, false, {}); + ASSERT_EQUALS(true, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + } + + { + Settings s; + ScopedFile file("file.c", ""); + CppCheck cppcheck(s, supprs, errorLogger, false, {}); + ASSERT_EQUALS(true, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + } + } + void premiumResultsCache() const { // Trac #13889 - cached misra results are shown after removing --premium=misra-c-2012 option From 5ae2a1ca8164b2331c9761a94210532a132a56d9 Mon Sep 17 00:00:00 2001 From: Goncalo Mao-Cheia Date: Mon, 27 Oct 2025 00:14:54 +0000 Subject: [PATCH 2/6] Fix formatting error --- lib/cppcheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index a623cbc5f25..29e0568f589 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -891,7 +891,7 @@ bool CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector Date: Mon, 27 Oct 2025 00:30:34 +0000 Subject: [PATCH 3/6] Add filename to error message --- lib/cppcheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 29e0568f589..577cfec1a6a 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -893,7 +893,7 @@ bool CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector Date: Mon, 27 Oct 2025 23:27:10 +0000 Subject: [PATCH 4/6] Removed logic to reportErr in checkPlistOutput. When file does not have suffix, the full name is used. --- lib/cppcheck.cpp | 19 ++++++------------- lib/cppcheck.h | 2 +- test/testcppcheck.cpp | 14 ++++++++++---- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 577cfec1a6a..94943b5eee9 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -884,25 +884,19 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string return checkInternal(file, cfgname, fileIndex, f); } -bool CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector& files) +void CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector& files) { if (!mSettings.plistOutput.empty()) { const bool slashFound = file.spath().find('/') != std::string::npos; std::string filename = slashFound ? file.spath().substr(file.spath().rfind('/') + 1) : file.spath(); - const std::size_t dotPosition = filename.find('.'); - - if (dotPosition == std::string::npos) { - ErrorMessage::FileLocation loc(filename, 0, 0); - ErrorMessage errmsg({std::move(loc)}, "", Severity::error, "Filename '" + filename + "' does not contain dot", "filenameError", Certainty::normal); - mErrorLogger.reportErr(errmsg); - return false; - } + // Removes suffix from filename when it exists + const std::string noSuffixFilename = filename.substr(0, filename.find('.')); + // the hash is added to handle when files in different folders have the same name const std::size_t fileNameHash = std::hash {}(file.spath()); - filename = mSettings.plistOutput + filename.substr(0, filename.find('.')) + "_" + std::to_string(fileNameHash) + ".plist"; + filename = mSettings.plistOutput + noSuffixFilename + "_" + std::to_string(fileNameHash) + ".plist"; mLogger->openPlist(filename, files); } - return true; } unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::string &cfgname, int fileIndex, const CreateTokenListFn& createTokenList) @@ -1010,8 +1004,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str if (!preprocessor.loadFiles(tokens1, files)) return mLogger->exitcode(); - if (!checkPlistOutput(file, files)) - return mLogger->exitcode(); + checkPlistOutput(file, files); std::string dumpProlog; if (mSettings.dump || !mSettings.addons.empty()) { diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 1d6d21d0cc9..54ed1d73227 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -185,7 +185,7 @@ class CPPCHECKLIB CppCheck { */ unsigned int checkFile(const FileWithDetails& file, const std::string &cfgname, int fileIndex); - bool checkPlistOutput(const FileWithDetails& file, const std::vector& files); + void checkPlistOutput(const FileWithDetails& file, const std::vector& files); /** * @brief Check a file using buffer diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp index 8f902761069..b5b95e6f77e 100644 --- a/test/testcppcheck.cpp +++ b/test/testcppcheck.cpp @@ -526,27 +526,33 @@ class TestCppcheck : public TestFixture { void checkPlistOutput() const { Suppressions supprs; ErrorLogger2 errorLogger; - std::vector files = {"file.txt"}; + std::vector files = {"textfile.txt"}; { const auto s = dinit(Settings, $.templateFormat = templateFormat, $.plistOutput = "output"); ScopedFile file("file", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); - ASSERT_EQUALS(false, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); + const std::string outputFile {"outputfile_4017852018407011111.plist"}; + ASSERT(Path::exists(outputFile)); + std::remove(outputFile.c_str()); } { const auto s = dinit(Settings, $.plistOutput = "output"); ScopedFile file("file.c", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); - ASSERT_EQUALS(true, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); + const std::string outputFile {"outputfile_18314070737314589129.plist"}; + ASSERT(Path::exists(outputFile)); + std::remove(outputFile.c_str()); } { Settings s; ScopedFile file("file.c", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); - ASSERT_EQUALS(true, cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files)); + cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); } } From 273da5cb046a05fb6dc9617ee81f3a8055f194ad Mon Sep 17 00:00:00 2001 From: Goncalo Mao-Cheia Date: Mon, 27 Oct 2025 23:34:13 +0000 Subject: [PATCH 5/6] Fix minor typo --- lib/cppcheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 94943b5eee9..51fafddacd7 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -889,7 +889,7 @@ void CppCheck::checkPlistOutput(const FileWithDetails& file, const std::vector Date: Tue, 28 Oct 2025 18:50:30 +0000 Subject: [PATCH 6/6] Add hash calculation to UT due to hashing functions differing between platforms --- test/testcppcheck.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp index b5b95e6f77e..0b6829a29f4 100644 --- a/test/testcppcheck.cpp +++ b/test/testcppcheck.cpp @@ -530,27 +530,31 @@ class TestCppcheck : public TestFixture { { const auto s = dinit(Settings, $.templateFormat = templateFormat, $.plistOutput = "output"); - ScopedFile file("file", ""); + const ScopedFile file("file", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); - cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); - const std::string outputFile {"outputfile_4017852018407011111.plist"}; + const FileWithDetails fileWithDetails {file.path(), Path::identify(file.path(), false), 0}; + + cppcheck.checkPlistOutput(fileWithDetails, files); + const std::string outputFile {"outputfile_" + std::to_string(std::hash {}(fileWithDetails.spath())) + ".plist"}; ASSERT(Path::exists(outputFile)); std::remove(outputFile.c_str()); } { const auto s = dinit(Settings, $.plistOutput = "output"); - ScopedFile file("file.c", ""); + const ScopedFile file("file.c", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); - cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); - const std::string outputFile {"outputfile_18314070737314589129.plist"}; + const FileWithDetails fileWithDetails {file.path(), Path::identify(file.path(), false), 0}; + + cppcheck.checkPlistOutput(fileWithDetails, files); + const std::string outputFile {"outputfile_" + std::to_string(std::hash {}(fileWithDetails.spath())) + ".plist"}; ASSERT(Path::exists(outputFile)); std::remove(outputFile.c_str()); } { Settings s; - ScopedFile file("file.c", ""); + const ScopedFile file("file.c", ""); CppCheck cppcheck(s, supprs, errorLogger, false, {}); cppcheck.checkPlistOutput(FileWithDetails(file.path(), Path::identify(file.path(), false), 0), files); }