Merge dashpay#7042: fix: re-enable cppcheck and bump its version

9bde19e test: more clear names of white & black list for cpp check: ALWAYS_ENABLED_WARNINGS / SUPPRESSED_WARNINGS (Konstantin Akimov)
cefc57a fix: ignore unexpected cppcheck for some files (Konstantin Akimov)
1b2900b test: remove unused exceptions for cppcheck (Konstantin Akimov)
fbac886 fix: re-active cppcheck (Konstantin Akimov)
78e613a ci: bump cppcheck version in a container to 2.17.1 (Konstantin Akimov)
72d1d34 refactor: fix cppcheck warning for cachemultimap.h (Konstantin Akimov)
8e064b3 refactor: fix warnings of cppcheck for netinfo.h (Konstantin Akimov)
ec8a438 test: run cpp-check with --check-level=exhaustive (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  The previous logic had an intersection of black & white list for exception, and only warnings that got in the white list have been shown.

  Everything that has not been matched with whitelist should be filtered by blacklist (exceptions), but not only whitelist matches

  ## What was done?
   - fixed blacklist & whitelist filtering
   - bump cpp-check to 2.17.1 (from 2.13.0)
   - removed outdated exceptions for cppcheck
   - added new exception for cppcheck
   - fixed warnings in `evo/mninfo.h` and `cachemultimap.h`

  ## How Has This Been Tested?
  Run locally

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 9bde19e

Tree-SHA512: b115a17d4b6911d1bbd529122fd979364ef07f2847f87a79caa94ee59a7e1a8f6b3fe8b811aa2bd30954d3b097d5f01952613105a3d6d755ce5102f4fdf7c47a

Author: PastaPastaPasta

contrib/containers/ci/ci-slim.Dockerfile

@@ -1,6 +1,6 @@
 # Builder for cppcheck
 FROM debian:bookworm-slim AS cppcheck-builder
-ARG CPPCHECK_VERSION=2.13.0
+ARG CPPCHECK_VERSION=2.17.1
 RUN set -ex; \
     apt-get update && apt-get install -y --no-install-recommends \
         curl \

src/cachemultimap.h

@@ -150,10 +150,10 @@ class CacheMultiMap
         if(mit == mapIndex.end()) {
             return;
         }
-        it_map_t& mapIt = mit->second;
+        const it_map_t& mapIt = mit->second;
 
-        for(it_map_it it = mapIt.begin(); it != mapIt.end(); ++it) {
-            listItems.erase(it->second);
+        for (const auto& it : mapIt) {
+            listItems.erase(it.second);
         }
 
         mapIndex.erase(mit);

src/evo/netinfo.h

@@ -176,7 +176,7 @@ class DomainPort
     DomainPort::Status Set(const std::string& addr, const uint16_t port);
     DomainPort::Status Validate() const;
     uint16_t GetPort() const { return m_port; }
-    std::string ToStringAddr() const { return m_addr; }
+    const std::string& ToStringAddr() const { return m_addr; }
     std::string ToStringAddrPort() const { return strprintf("%s:%d", m_addr, m_port); }
 };
 
@@ -312,8 +312,6 @@ class MnNetInfo final : public NetInfoInterface
     template <typename Stream>
     MnNetInfo(deserialize_type, Stream& s) { s >> *this; }
 
-    ~MnNetInfo() = default;
-
     template <typename Stream>
     void Serialize(Stream& s) const
     {
@@ -398,8 +396,6 @@ class ExtNetInfo final : public NetInfoInterface
     template <typename Stream>
     ExtNetInfo(deserialize_type, Stream& s) { s >> *this; }
 
-    ~ExtNetInfo() = default;
-
     template <typename Stream>
     void Serialize(Stream& s) const
     {

test/lint/lint-cppcheck-dash.py

@@ -15,7 +15,7 @@
 
 os.environ['LC_ALL'] = 'C'
 
-ENABLED_CHECKS = (
+ALWAYS_ENABLED_WARNINGS = (
     "Class '.*' has a constructor with 1 argument that is not explicit.",
     "Struct '.*' has a constructor with 1 argument that is not explicit.",
     "Function parameter '.*' should be passed by const reference.",
@@ -33,26 +33,13 @@
     # ".*",
 )
 
-IGNORED_WARNINGS = (
-    "src/bls/bls.h:.* Struct 'CBLSIdImplicit' has a constructor with 1 argument that is not explicit.",
-    "src/rpc/masternode.cpp:.*:21: warning: Consider using std::copy algorithm instead of a raw loop.",  # UniValue doesn't support std::copy
-    "src/cachemultimap.h:.*: warning: Variable 'mapIt' can be declared as reference to const",
-    "src/evo/simplifiedmns.cpp:.*:20: warning: Consider using std::copy algorithm instead of a raw loop.",
-    "src/llmq/commitment.cpp.* warning: Consider using std::all_of or std::none_of algorithm instead of a raw loop. [useStlAlgorithm]",
-    "src/rpc/.*cpp:.*: note: Function pointer used here.",
-    "src/masternode/sync.cpp:.*: warning: Variable 'pnode' can be declared as pointer to const [constVariableReference]",
-    "src/wallet/bip39.cpp.*: warning: The scope of the variable 'ssCurrentWord' can be reduced. [variableScope]",
-    "src/.*:.*: warning: Local variable '_' shadows outer function [shadowFunction]",
-
+SUPPRESSED_WARNINGS = (
     "src/stacktraces.cpp:.*: .*: Parameter 'info' can be declared as pointer to const",
     "src/stacktraces.cpp:.*: note: You might need to cast the function pointer here",
 
-    "[note|warning]: Return value 'state.Invalid(.*)' is always false",
-    "note: Calling function 'Invalid' returns 0",
-    "note: Shadow variable",
-
-    # General catchall, for some reason any value named 'hash' is viewed as never used.
-    "Variable 'hash' is assigned a value that is never used.",
+    # current version of cppcheck fails with this error if exhaustive level is used
+    # TODO: remove with a newer version
+    "warning: Internal error: Child process crashed with signal 6",
 
     # The following can be useful to ignore when the catch all is used
     # "Consider performing initialization in initialization list.",
@@ -85,8 +72,8 @@ def main():
     files_output = subprocess.check_output(['git', 'ls-files', '--'] + patterns, universal_newlines=True, encoding="utf8")
     files = [f.strip() for f in files_output.splitlines() if f.strip()]
 
-    enabled_regexp = '|'.join(ENABLED_CHECKS)
-    ignored_regexp = '|'.join(IGNORED_WARNINGS)
+    always_enabled_regexp = '|'.join(ALWAYS_ENABLED_WARNINGS)
+    suppressed_regexp = '|'.join(SUPPRESSED_WARNINGS)
     files_regexp = '|'.join(re.escape(f) for f in files)
 
     script_dir = os.path.dirname(os.path.abspath(__file__))
@@ -107,6 +94,7 @@ def main():
         '--language=c++',
         '--std=c++20',
         '--template=gcc',
+        '--check-level=exhaustive',
         '-D__cplusplus',
         '-DENABLE_WALLET',
         '-DCLIENT_VERSION_BUILD',
@@ -130,14 +118,16 @@ def main():
 
     unique_sorted_lines = sorted(set(dependencies_output.stdout.splitlines()))
     for line in unique_sorted_lines:
-        if re.search(enabled_regexp, line) and not re.search(ignored_regexp, line) and re.search(files_regexp, line):
+        if not re.search(files_regexp, line):
+            continue
+        if re.search(always_enabled_regexp, line) or not re.search(suppressed_regexp, line):
             warnings.append(line)
 
     if warnings:
         print('\n'.join(warnings))
         print()
         print("Advice not applicable in this specific case? Add an exception by updating")
-        print(f"IGNORED_WARNINGS in {__file__}")
+        print(f"SUPPRESSED_WARNINGS in {__file__}")
         # Uncomment to enforce the linter / comment to run locally
         exit_code = 1