From dc03a504146986b0f53324e353e9e2d74170d97e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Sat, 24 Sep 2022 11:59:13 +0200 Subject: [PATCH] some small cleanups and refactorings (#4488) * moved `plistFile` from `ErrorLogger` to `CppCheck` * got rid of global CWE objects * lib/CMakeLists.txt: suppress some `-Wfloat-equal` clang warning in matchcompiled builds as well * lib/CMakeLists.txt: moved a loop into proper block * test/CMakeLists.txt: simplified `add_fixture` * test/CMakeLists.txt: moved `fixture_cost` * fixed `naming-privateMemberVariable` selfcheck warning --- cli/cmdlineparser.cpp | 1 + lib/CMakeLists.txt | 20 +++++++++++++------- lib/checknullpointer.cpp | 4 ++++ lib/checkuninitvar.cpp | 4 ++++ lib/checkunusedfunctions.cpp | 1 + lib/cppcheck.cpp | 20 +++++++++++++------- lib/cppcheck.h | 3 +++ lib/errorlogger.h | 22 +--------------------- test/CMakeLists.txt | 24 +++++++----------------- test/testutils.h | 1 + 10 files changed, 48 insertions(+), 52 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index a18c29bce..ecbe15f03 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -37,6 +37,7 @@ #include #include // EXIT_FAILURE #include +#include #include #include #include diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 640c526eb..4fe6a8f06 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -22,11 +22,11 @@ function(build_src output filename) set_source_files_properties(${outfile} PROPERTIES GENERATED TRUE) endfunction() -foreach(file ${srcs}) - build_src(srcs_build ${file}) -endforeach() - if (NOT USE_MATCHCOMPILER_OPT STREQUAL "Off") + foreach(file ${srcs}) + build_src(srcs_build ${file}) + endforeach() + set(srcs_lib ${srcs_build}) else() set(srcs_lib ${srcs}) @@ -53,7 +53,13 @@ endif() if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # -Wfloat-equal is generated by picojson.h - set_source_files_properties(cppcheck.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) - set_source_files_properties(importproject.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) - set_source_files_properties(settings.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + if (NOT USE_MATCHCOMPILER_OPT STREQUAL "Off") + set_source_files_properties(mc_cppcheck.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + set_source_files_properties(mc_importproject.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + set_source_files_properties(mc_settings.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + else() + set_source_files_properties(cppcheck.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + set_source_files_properties(importproject.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + set_source_files_properties(settings.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal) + endif() endif() diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index dc8878ca6..9ac25874f 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -39,6 +39,10 @@ //--------------------------------------------------------------------------- +// CWE ids used: +static const struct CWE CWE_NULL_POINTER_DEREFERENCE(476U); +static const struct CWE CWE_INCORRECT_CALCULATION(682U); + // Register this check class (by creating a static instance of it) namespace { CheckNullPointer instance; diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index cbefb5d52..bcfebcfa0 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -49,6 +49,10 @@ namespace tinyxml2 { //--------------------------------------------------------------------------- +// CWE ids used: +static const struct CWE CWE_USE_OF_UNINITIALIZED_VARIABLE(457U); +static const struct CWE CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION(676U); + // Register this check class (by creating a static instance of it) namespace { CheckUninitVar instance; diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index fd156175b..5d688d593 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -34,6 +34,7 @@ #include #include #include +#include // IWYU pragma: keep #include #include // IWYU pragma: keep #include diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index b325f0a38..861f7d870 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include // <- TEMPORARY #include #include @@ -365,6 +366,11 @@ CppCheck::~CppCheck() mFileInfo.pop_back(); } s_timerResults.showResults(mSettings.showtime); + + if (mPlistFile.is_open()) { + mPlistFile << ErrorLogger::plistFooter(); + mPlistFile.close(); + } } const char * CppCheck::version() @@ -606,9 +612,9 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } } - if (plistFile.is_open()) { - plistFile << ErrorLogger::plistFooter(); - plistFile.close(); + if (mPlistFile.is_open()) { + mPlistFile << ErrorLogger::plistFooter(); + mPlistFile.close(); } CheckUnusedFunctions checkUnusedFunctions(nullptr, nullptr, nullptr); @@ -669,8 +675,8 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string filename2 = filename; std::size_t fileNameHash = std::hash {}(filename); filename2 = mSettings.plistOutput + filename2.substr(0, filename2.find('.')) + "_" + std::to_string(fileNameHash) + ".plist"; - plistFile.open(filename2); - plistFile << ErrorLogger::plistHeader(version(), files); + mPlistFile.open(filename2); + mPlistFile << ErrorLogger::plistHeader(version(), files); } std::ostringstream dumpProlog; @@ -1551,9 +1557,9 @@ void CppCheck::reportErr(const ErrorMessage &msg) mErrorLogger.reportErr(msg); // check if plistOutput should be populated and the current output file is open and the error is not suppressed - if (!mSettings.plistOutput.empty() && plistFile.is_open() && !mSettings.nomsg.isSuppressed(errorMessage)) { + if (!mSettings.plistOutput.empty() && mPlistFile.is_open() && !mSettings.nomsg.isSuppressed(errorMessage)) { // add error to plist output file - plistFile << ErrorLogger::plistData(msg); + mPlistFile << ErrorLogger::plistData(msg); } } diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 77f9feaf3..560823203 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -30,6 +30,7 @@ #include "settings.h" #include +#include // IWYU pragma: keep #include #include #include @@ -238,6 +239,8 @@ private: /** Callback for executing a shell command (exe, args, output) */ std::function,std::string,std::string*)> mExecuteCommand; + + std::ofstream mPlistFile; }; /// @} diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 0b7147cbb..1982c409c 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -27,24 +27,11 @@ #include "color.h" #include -#include #include #include #include #include -/** - * CWE id (Common Weakness Enumeration) - * See https://cwe.mitre.org/ for further reference. - * */ -// CWE list: https://cwe.mitre.org/data/published/cwe_v3.4.1.pdf -static const struct CWE CWE_USE_OF_UNINITIALIZED_VARIABLE(457U); -static const struct CWE CWE_NULL_POINTER_DEREFERENCE(476U); -static const struct CWE CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION(676U); -static const struct CWE CWE_INCORRECT_CALCULATION(682U); -static const struct CWE CWE_EXPIRED_POINTER_DEREFERENCE(825U); - - class Token; class TokenList; @@ -231,16 +218,9 @@ private: * should implement. */ class CPPCHECKLIB ErrorLogger { -protected: - std::ofstream plistFile; public: ErrorLogger() {} - virtual ~ErrorLogger() { - if (plistFile.is_open()) { - plistFile << ErrorLogger::plistFooter(); - plistFile.close(); - } - } + virtual ~ErrorLogger() {} /** * Information about progress is directed here. diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 894fb7d0b..8690a8071 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -63,26 +63,10 @@ if (BUILD_TESTS) set(SKIP_TESTS "" CACHE STRING "A list of tests to skip") function(add_fixture NAME) - set(options) - set(oneValueArgs WORKING_DIRECTORY) - set(multiValueArgs) - - cmake_parse_arguments(PARSE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) - if (${NAME} IN_LIST SKIP_TESTS) elseif(TEST ${NAME}) else() - set(WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}) - if (PARSE_WORKING_DIRECTORY) - set(WORKING_DIRECTORY ${PARSE_WORKING_DIRECTORY}) - endif() - add_test(NAME ${NAME} COMMAND testrunner ${NAME} WORKING_DIRECTORY ${WORKING_DIRECTORY}) - endif() - endfunction() - - function(fixture_cost NAME COST) - if(TEST ${NAME}) - set_tests_properties(${NAME} PROPERTIES COST ${COST}) + add_test(NAME ${NAME} COMMAND testrunner ${NAME} WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}) endif() endfunction() @@ -162,6 +146,12 @@ if (BUILD_TESTS) add_cfg(windows.cpp INCONCLUSIVE NAME windows64 PLATFORM win64) add_cfg(wxwidgets.cpp INCONCLUSIVE) + function(fixture_cost NAME COST) + if(TEST ${NAME}) + set_tests_properties(${NAME} PROPERTIES COST ${COST}) + endif() + endfunction() + # Set cost of the more expensive tests to help improve parallel scheduling # of tests fixture_cost(TestIO 20) diff --git a/test/testutils.h b/test/testutils.h index 5ece4a58f..dc1dbee95 100644 --- a/test/testutils.h +++ b/test/testutils.h @@ -27,6 +27,7 @@ #include "tokenlist.h" #include +#include #include #include // IWYU pragma: keep #include