diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 72a20315272..82d4d70c0a0 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -661,8 +661,13 @@ void StdLogger::reportErr(const ErrorMessage &msg) msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType); // TODO: there should be no need for verbose and default messages here + // Don't perform redundant reads for these formats, the code is not needed + // for deduplication + const bool noCode = mSettings.outputFormat == Settings::OutputFormat::xml || + mSettings.outputFormat == Settings::OutputFormat::sarif; const std::string msgStr = - msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation); + msgCopy.toString(mSettings.verbose, mSettings.templateFormat, + mSettings.templateLocation, noCode); // Alert only about unique errors if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 2b8e9691cef..8e32e3e3152 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -210,7 +210,9 @@ class CppCheck::CppCheckLogger : public ErrorLogger } // TODO: there should be no need for the verbose and default messages here - std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation); + // Code is not needed for deduplication + const bool noCode = true; + std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation, noCode); if (errmsg.empty()) return; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index d57ad999734..4e917d4ae28 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -696,7 +696,7 @@ static void replaceColors(std::string& source, bool erase) { replace(source, substitutionMapErase); } -std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const +std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation, bool noCode) const { assert(!templateFormat.empty()); @@ -737,7 +737,8 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm endl = "\r\n"; else endl = "\r"; - findAndReplace(result, "{code}", readCode(callStack.back().getOrigFile(), callStack.back().line, callStack.back().column, endl)); + const std::string code = noCode ? "" : readCode(callStack.back().getOrigFile(), callStack.back().line, callStack.back().column, endl); + findAndReplace(result, "{code}", code); } } else { static const std::unordered_map callStackSubstitutionMap = @@ -768,7 +769,8 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm endl = "\r\n"; else endl = "\r"; - findAndReplace(text, "{code}", readCode(fileLocation.getOrigFile(), fileLocation.line, fileLocation.column, endl)); + const std::string code = noCode ? "" : readCode(fileLocation.getOrigFile(), fileLocation.line, fileLocation.column, endl); + findAndReplace(text, "{code}", code); } result += '\n' + text; } diff --git a/lib/errorlogger.h b/lib/errorlogger.h index b28fdba244e..bf4bef22ba7 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -152,11 +152,13 @@ class CPPCHECKLIB ErrorMessage { * or template to be used. E.g. "{file}:{line},{severity},{id},{message}" * @param templateLocation Format Empty string to use default output format * or template to be used. E.g. "{file}:{line},{info}" + * @param noCode Always replace {code} with an empty string * @return formatted string */ std::string toString(bool verbose, const std::string &templateFormat, - const std::string &templateLocation) const; + const std::string &templateLocation, + bool noCode = false) const; std::string serialize() const; /** diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 567b214ede1..ace82b9ac32 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -4861,4 +4861,46 @@ def test_ipc_inline_suppressions(tmp_path): stdout_lines = stdout.splitlines() stdout_lines.sort() assert stdout_lines == stdout_exp - assert stderr.splitlines() == [] \ No newline at end of file + assert stderr.splitlines() == [] + +test_redundant_file_reads_params = [ + ([], 3), + (['--suppress=zerodiv'], 1), + (['--template=cppcheck1'], 1), + (['--xml'], 1), +] + +@pytest.mark.skipif(sys.platform != 'linux' or 'ASAN_OPTIONS' in os.environ, reason="uses strace") +@pytest.mark.parametrize('flags,expected', test_redundant_file_reads_params) +def test_redundant_file_reads(tmpdir, flags, expected): + source_pathname = os.path.join(tmpdir, 'test.c') + content = """ +void f(int x) { + int y = x / 0; + int z = x / 0; +} +""" + cppcheck_path = __lookup_cppcheck_exe() + + with open(source_pathname, 'wt') as f: + f.write(content) + + args = [ + 'strace', + '--summary-only', + '--summary-columns=count', + '--trace=openat', + '--follow-forks', + f'--trace-path={source_pathname}', + cppcheck_path, + '-q', + source_pathname, + ] + + args += flags + proc = subprocess.Popen(args, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) + + _, stderr = proc.communicate() + + assert proc.returncode == 0 + assert stderr.splitlines()[-1].strip() == f'{expected} total'.encode('utf-8') diff --git a/test/cli/performance_test.py b/test/cli/performance_test.py index b4d2f33be51..ea1ad8bd5ff 100644 --- a/test/cli/performance_test.py +++ b/test/cli/performance_test.py @@ -441,7 +441,7 @@ def test_slow_many_headers(tmpdir): @pytest.mark.skipif(sys.platform == 'darwin', reason='GitHub macOS runners are too slow') -@pytest.mark.timeout(10) +@pytest.mark.timeout(5) def test_large_number_of_violations_and_suppressions(tmpdir): filename_main = os.path.join(tmpdir, 'main.c') # This name causes the PathMatch::match() to iterate ~70 times, which is not unrealistic for a header file placed in subdirs. diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index c849d3a0c61..730d091d634 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -55,6 +55,7 @@ class TestErrorLogger : public TestFixture { TEST_CASE(ErrorMessageVerboseNewline); TEST_CASE(ErrorMessageFromInternalError); TEST_CASE(ErrorMessageCode); + TEST_CASE(ErrorMessageNoCode); TEST_CASE(CustomFormat); TEST_CASE(CustomFormat2); TEST_CASE(CustomFormatLocations); @@ -386,6 +387,22 @@ class TestErrorLogger : public TestFixture { msg.toString(false, "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]\n{code}", "")); } + void ErrorMessageNoCode() const { + ScopedFile file("code.cpp", + "int i;\n" + "int i2;\n" + "int i3;\n" + ); + + ErrorMessage::FileLocation code{"code.cpp", 3, 5}; + std::list locs = { code }; + ErrorMessage msg(std::move(locs), "", Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ASSERT_EQUALS(1, msg.callStack.size()); + const bool noCode = true; + ASSERT_EQUALS("code.cpp:3:5: error: Programming error. [errorId]\n", + msg.toString(false, "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]\n{code}", "", noCode)); + } + void CustomFormat() const { std::list locs(1, fooCpp5); ErrorMessage msg(std::move(locs), "", Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal);