Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/evo/netinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ class NetInfoEntry

public:
NetInfoEntry() = default;
NetInfoEntry(const DomainPort& domain)
explicit NetInfoEntry(const DomainPort& domain)
{
if (!domain.IsValid()) return;
m_type = NetInfoType::Domain;
m_data = domain;
}
NetInfoEntry(const CService& service)
explicit NetInfoEntry(const CService& service)
{
if (!service.IsValid()) return;
m_type = NetInfoType::Service;
Expand Down
7 changes: 3 additions & 4 deletions src/wallet/bip39.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,11 @@ bool CMnemonic::Check(const SecureString& mnemonic)

SecureString ssCurrentWord;
SecureVector bits(32 + 1);

uint32_t ki, nBitsCount{};
uint32_t nBitsCount{};

for (size_t i = 0; i < mnemonic.size(); ++i)
{
ssCurrentWord = "";
ssCurrentWord.resize(0); // we resize ssCurrentWord instead recreating to avoid new allocations
Comment thread
kwvg marked this conversation as resolved.
while (i + ssCurrentWord.size() < mnemonic.size() && mnemonic[i + ssCurrentWord.size()] != ' ') {
if (ssCurrentWord.size() >= 9) {
return false;
Expand All @@ -115,7 +114,7 @@ bool CMnemonic::Check(const SecureString& mnemonic)
return false;
}
if (ssCurrentWord == wordlist[nWordIndex]) { // word found on index nWordIndex
for (ki = 0; ki < 11; ki++) {
for (uint32_t ki = 0; ki < 11; ki++) {
if (nWordIndex & (1 << (10 - ki))) {
bits[nBitsCount / 8] |= 1 << (7 - (nBitsCount % 8));
}
Expand Down
2 changes: 1 addition & 1 deletion test/lint/all-lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

exit_code = 0
mod_path = Path(__file__).parent
lints = glob(f"{mod_path}/lint-*")
lints = glob(f"{mod_path}/lint-*.py")
if which("parallel") and which("column"):
logfile = "parallel_out.log"
command = ["parallel", "--jobs", "100%", "--will-cite", "--joblog", logfile, ":::"] + lints
Expand Down
147 changes: 147 additions & 0 deletions test/lint/lint-cppcheck-dash.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
#!/usr/bin/env python3
#
# Copyright (c) 2019 The Bitcoin Core developers
# Copyright (c) 2025 The Dash Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
#
# Run cppcheck for dash specific files

import multiprocessing
import os
import re
import subprocess
import sys

os.environ['LC_ALL'] = 'C'
Comment thread
kwvg marked this conversation as resolved.

ENABLED_CHECKS = (
"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.",
"Comparison of modulo result is predetermined",
"Local variable '.*' shadows outer argument",
"Redundant initialization for '.*'. The initialized value is overwritten before it is read.",
"Dereferencing '.*' after it is deallocated / released",
"The scope of the variable '.*' can be reduced.",
"Parameter '.*' can be declared with const",
"Variable '.*' can be declared with const",
"Variable '.*' is assigned a value that is never used.",
"Unused variable",
"The function '.*' overrides a function in a base class but is not marked with a 'override' specifier.",
# Enable to catch all warnings
# ".*",
)

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]",

"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.",

# The following can be useful to ignore when the catch all is used
# "Consider performing initialization in initialization list.",
"Consider using std::transform algorithm instead of a raw loop.",
"Consider using std::accumulate algorithm instead of a raw loop.",
"Consider using std::any_of algorithm instead of a raw loop.",
"Consider using std::copy_if algorithm instead of a raw loop.",
# "Consider using std::count_if algorithm instead of a raw loop.",
# "Consider using std::find_if algorithm instead of a raw loop.",
# "Member variable '.*' is not initialized in the constructor.",

"unusedFunction",
"unknownMacro",
"unusedStructMember",
)

def main():
warnings = []
exit_code = 0

try:
subprocess.check_output(['cppcheck', '--version'])
except FileNotFoundError:
print("Skipping cppcheck linting since cppcheck is not installed.")
sys.exit(0)

with open('test/util/data/non-backported.txt', 'r', encoding='utf-8') as f:
patterns = [line.strip() for line in f if line.strip()]

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)
files_regexp = '|'.join(re.escape(f) for f in files)

script_dir = os.path.dirname(os.path.abspath(__file__))
cache_dir = os.environ.get('CACHE_DIR')
if cache_dir:
cppcheck_dir = os.path.join(cache_dir, 'cppcheck')
else:
cppcheck_dir = os.path.join(script_dir, '.cppcheck')
os.makedirs(cppcheck_dir, exist_ok=True)

cppcheck_cmd = [
'cppcheck',
'--enable=all',
'--inline-suppr',
'--suppress=missingIncludeSystem',
f'--cppcheck-build-dir={cppcheck_dir}',
'-j', str(multiprocessing.cpu_count()),
Comment thread
kwvg marked this conversation as resolved.
'--language=c++',
'--std=c++20',
'--template=gcc',
'-D__cplusplus',
'-DENABLE_WALLET',
'-DCLIENT_VERSION_BUILD',
'-DCLIENT_VERSION_IS_RELEASE',
'-DCLIENT_VERSION_MAJOR',
'-DCLIENT_VERSION_MINOR',
'-DCOPYRIGHT_YEAR',
'-DDEBUG',
'-DUSE_EPOLL',
'-DCHAR_BIT=8',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consideration for followup, should we also define ENABLE_STACKTRACES and platform-specific macros like ENABLE_SSSE3, ENABLE_SSE41, ENABLE_X86_AESNI, ENABLE_ARM_AES, ENABLE_ARM_NEON to expand coverage?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to create follow-up PR, this PR is about replacing bash to python and some related necessary changes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, ENABLE_SSE41, ENABLE_X86_AESNI, ENABLE_ARM_AES will probably make a difference for src/crypto/x11 subdirectory which is kind of 3rd party code and not a subject for cpplint.

Though, ENABLE_STACKTRACES - could be useful

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accelerated routines aren't third-party code as we are maintaining it but we can defer this decision to after some more accelerated backends are implemented

'-I', 'src/',
'-q',
] + files

dependencies_output = subprocess.run(
cppcheck_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
)

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):
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__}")
# Uncomment to enforce the linter / comment to run locally
exit_code = 1

Comment on lines +124 to +143
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Propagate cppcheck execution failures

Right now we ignore cppcheck’s exit status. If it dies due to a configuration or invocation error (missing include paths, bad arguments, etc.), the run still reports success as long as no warning matches our filters. That can silently mask broken lint runs. Capture the non-zero return code and bail out immediately so the CI fails when cppcheck itself fails.

     dependencies_output = subprocess.run(
         cppcheck_cmd,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
         text=True,
     )
 
+    if dependencies_output.returncode != 0:
+        print(dependencies_output.stdout, end='')
+        sys.exit(dependencies_output.returncode)
+
     unique_sorted_lines = sorted(set(dependencies_output.stdout.splitlines()))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dependencies_output = subprocess.run(
cppcheck_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
)
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):
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__}")
# Uncomment to enforce the linter / comment to run locally
exit_code = 1
dependencies_output = subprocess.run(
cppcheck_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
)
if dependencies_output.returncode != 0:
print(dependencies_output.stdout, end='')
sys.exit(dependencies_output.returncode)
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):
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__}")
# Uncomment to enforce the linter / comment to run locally
exit_code = 1
🧰 Tools
🪛 Ruff (0.13.3)

124-124: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents
In test/lint/lint-cppcheck-dash.py around lines 124 to 143, the script currently
ignores cppcheck's exit status which can hide execution/configuration failures;
update the code to check dependencies_output.returncode immediately after
subprocess.run and if it's non-zero print dependencies_output.stdout (or a clear
error message including that output) and exit with a non-zero status (e.g.,
sys.exit(dependencies_output.returncode) or exit(1)); do this before processing
warnings so CI fails when cppcheck itself fails.

sys.exit(exit_code)

if __name__ == "__main__":
main()
108 changes: 0 additions & 108 deletions test/lint/lint-cppcheck-dash.sh

This file was deleted.

Loading