Skip to content
Open
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: 0 additions & 4 deletions cli/cppcheckexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,10 +432,6 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
returnValue = settings.exitCode;
}

if (!settings.checkConfiguration) {
cppcheck.tooManyConfigsError("",0U);
}

stdLogger.writeCheckersReport(supprs);

if (settings.outputFormat == Settings::OutputFormat::xml) {
Expand Down
51 changes: 19 additions & 32 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,9 @@ std::size_t CppCheck::calculateHash(const Preprocessor& preprocessor, const std:
toolinfo << (mSettings.severity.isEnabled(Severity::portability) ? 'p' : ' ');
toolinfo << (mSettings.severity.isEnabled(Severity::information) ? 'i' : ' ');
toolinfo << mSettings.userDefines;
toolinfo << (mSettings.checkConfiguration ? 'c' : ' '); // --check-config
toolinfo << (mSettings.force ? 'f' : ' ');
toolinfo << mSettings.maxConfigs;
toolinfo << std::to_string(static_cast<std::uint8_t>(mSettings.checkLevel));
for (const auto &a : mSettings.addonInfos) {
toolinfo << a.name;
Expand Down Expand Up @@ -1041,6 +1044,9 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
for (const std::string &config : configurations)
(void)preprocessor.getcode(config, files, false);

if (configurations.size() > mSettings.maxConfigs)
tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());

if (analyzerInformation)
mLogger->setAnalyzerInfo(nullptr);
return 0;
Expand All @@ -1060,14 +1066,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
}
#endif

if (!mSettings.force && configurations.size() > mSettings.maxConfigs) {
if (mSettings.severity.isEnabled(Severity::information)) {
tooManyConfigsError(Path::toNativeSeparators(file.spath()),configurations.size());
} else {
mTooManyConfigs = true;
}
}

FilesDeleter filesDeleter;

// write dump file xml prolog
Expand All @@ -1092,8 +1090,18 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str

// Check only a few configurations (default 12), after that bail out, unless --force
// was used.
if (!mSettings.force && ++checkCount > mSettings.maxConfigs)
if (!mSettings.force && ++checkCount > mSettings.maxConfigs) {
// If maxConfigs has default value then report information message that configurations are skipped.
// If maxConfigs does not have default value then the user is explicitly skipping configurations so
// the information message is not reported, the whole purpose of setting i.e. --max-configs=1 is to
// skip configurations. When --check-config is used then tooManyConfigs will be reported even if the
// value is non-default.
const Settings defaultSettings;
if (mSettings.maxConfigs == defaultSettings.maxConfigs && mSettings.severity.isEnabled(Severity::information))
tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());

Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels very strange.

Also the check for the default value should not be hard-coded - use a default Settings object to check against.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I wrote a comment to describe it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Seems to make sense but I have not thought too much about it - as I have not dug too much into #7424.

But I have to from a different angle anyways as part of preprocessor work I am doing. But merging this should be fine (possibly even helpful as there is less code to go through).

break;
}

std::string currentConfig;

Expand Down Expand Up @@ -1630,32 +1638,14 @@ void CppCheck::executeAddonsWholeProgram(const std::list<FileWithDetails> &files

void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfConfigurations)
{
if (!mSettings.severity.isEnabled(Severity::information) && !mTooManyConfigs)
return;

mTooManyConfigs = false;

if (mSettings.severity.isEnabled(Severity::information) && file.empty())
return;

std::list<ErrorMessage::FileLocation> loclist;
if (!file.empty()) {
loclist.emplace_back(file, 0, 0);
}

std::ostringstream msg;
msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs;
if (numberOfConfigurations > mSettings.maxConfigs)
msg << " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.\n";
if (file.empty())
msg << " configurations. Use --force to check all configurations. For more details, use --enable=information.\n";
msg << "The checking of the file will be interrupted because there are too many "
"#ifdef configurations. Checking of all #ifdef configurations can be forced "
"by --force command line option or from GUI preferences. However that may "
"increase the checking time.";
if (file.empty())
msg << " For more details, use --enable=information.";

msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs
<< " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.";

ErrorMessage errmsg(std::move(loclist),
"",
Expand All @@ -1669,8 +1659,6 @@ void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfCo

void CppCheck::purgedConfigurationMessage(const std::string &file, const std::string& configuration)
{
mTooManyConfigs = false;

if (mSettings.severity.isEnabled(Severity::information) && file.empty())
return;

Expand Down Expand Up @@ -1699,7 +1687,6 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger)

CppCheck cppcheck(settings, supprs, errorlogger, true, nullptr);
cppcheck.purgedConfigurationMessage("","");
cppcheck.mTooManyConfigs = true;
cppcheck.tooManyConfigsError("",0U);
// TODO: add functions to get remaining error messages

Expand Down
3 changes: 0 additions & 3 deletions lib/cppcheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,6 @@ class CPPCHECKLIB CppCheck {

bool mUseGlobalSuppressions;

/** Are there too many configs? */
bool mTooManyConfigs{};

/** File info used for whole program analysis */
std::list<Check::FileInfo*> mFileInfo;

Expand Down
37 changes: 37 additions & 0 deletions test/cli/other_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3948,3 +3948,40 @@ def test_simplecpp_syntax_error(tmp_path):
'{}:1:0: error: No header in #include [syntaxError]'.format(test_file),
'{}:1:0: error: No header in #include [syntaxError]'.format(test_file)
]


@pytest.mark.parametrize('max_configs,number_of_configs,check_config,expected_warn', [
# max configs = default, max configs < number of configs => warn
(12, 20, False, True),
(12, 20, True, True),

# max configs != default, max configs < number of configs => warn if --check-config
(6, 20, False, False),
(6, 20, True, True),

# max configs >= number of configs => no warning
(20, 20, False, False),
(20, 20, False, False)
])
def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, expected_warn):
test_file = tmp_path / 'test.cpp'
with open(test_file, "w") as f:
for i in range(1,number_of_configs):
dir = 'if' if i == 1 else 'elif'
f.write(f'#{dir} defined(X{i})\nx = {i};\n')
f.write('#endif\n')

args = [f'--max-configs={max_configs}', '--enable=information', '--template=simple', str(test_file)]

if check_config:
args = ['--check-config'] + args

# default max configs is set to 12, warn if code contains more configurations than that
_, _, stderr = cppcheck(args)
if not expected_warn:
assert stderr.splitlines() == []
else:
assert stderr.splitlines() == [
'{}:0:0: information: Too many #ifdef configurations - cppcheck only checks {} of {} configurations. Use --force to check all configurations. [toomanyconfigs]'
.format(test_file, max_configs, number_of_configs)
]
30 changes: 0 additions & 30 deletions test/testcppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ class TestCppcheck : public TestFixture {
TEST_CASE(getDumpFileContentsLibrary);
TEST_CASE(checkPlistOutput);
TEST_CASE(premiumResultsCache);
TEST_CASE(toomanyconfigs);
TEST_CASE(purgedConfiguration);
}

Expand Down Expand Up @@ -594,35 +593,6 @@ class TestCppcheck : public TestFixture {
ASSERT(hash1 != hash2);
}

void toomanyconfigs() const
{
ScopedFile test_file_a("a.c",
"#if DEF_1\n"
"#endif\n"
"#if DEF_2\n"
"#endif\n"
"#if DEF_3\n"
"#endif");

// this is the "simple" format
const auto s = dinit(Settings,
$.templateFormat = templateFormat, // TODO: remove when we only longer rely on toString() in unique message handling
$.severity.enable (Severity::information);
$.maxConfigs = 2);
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_a.path(), Path::identify(test_file_a.path(), false), 0)));
// TODO: how to properly disable these warnings?
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
return msg.id == "logChecker";
}), errorLogger.errmsgs.end());
// the internal errorlist is cleared after each check() call
ASSERT_EQUALS(1, errorLogger.errmsgs.size());
const auto it = errorLogger.errmsgs.cbegin();
ASSERT_EQUALS("a.c:0:0: information: Too many #ifdef configurations - cppcheck only checks 2 of 4 configurations. Use --force to check all configurations. [toomanyconfigs]", it->toString(false, templateFormat, ""));
}

void purgedConfiguration() const
{
ScopedFile test_file("test.cpp",
Expand Down
Loading