Skip to content

fix #11824: Option --max-configs has no effect if -D is used #7424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
17 changes: 4 additions & 13 deletions cli/cmdlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
}

bool def = false;
bool maxconfigs = false;

ImportProject project;

Expand Down Expand Up @@ -984,10 +983,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
mLogger.printError("argument to '--max-configs=' must be greater than 0.");
return Result::Fail;
}

mSettings.maxConfigs = tmp;
mSettings.force = false;
maxconfigs = true;
}

// max ctu depth
Expand Down Expand Up @@ -1551,6 +1547,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
}
}

if (mSettings.maxConfigs > 0) {
mSettings.force = false;
}

if (!loadCppcheckCfg())
return Result::Fail;

Expand All @@ -1568,15 +1568,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
substituteTemplateFormatStatic(mSettings.templateFormat);
substituteTemplateLocationStatic(mSettings.templateLocation);

if (mSettings.force || maxconfigs)
mSettings.checkAllConfigurations = true;

if (mSettings.force)
mSettings.maxConfigs = INT_MAX;

else if ((def || mSettings.preprocessOnly) && !maxconfigs)
mSettings.maxConfigs = 1U;

if (mSettings.jobs > 1 && mSettings.buildDir.empty()) {
// TODO: bail out instead?
if (mSettings.checks.isEnabled(Checks::unusedFunction))
Expand Down
4 changes: 0 additions & 4 deletions gui/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1101,10 +1101,6 @@ bool MainWindow::getCppcheckSettings(Settings& settings, Suppressions& supprs)
supprs.nomsg.addSuppression(suppression); // TODO: check result
}

// Only check the given -D configuration
if (!defines.isEmpty())
settings.maxConfigs = 1;

// If importing a project, only check the given configuration
if (!mProjectFile->getImportProject().isEmpty())
settings.checkAllConfigurations = false;
Expand Down
66 changes: 35 additions & 31 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,16 +1014,37 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string

// Get configurations..
std::set<std::string> configurations;
if ((mSettings.checkAllConfigurations && mSettings.userDefines.empty()) || mSettings.force) {
Timer::run("Preprocessor::getConfigs", mSettings.showtime, &s_timerResults, [&]() {
configurations = preprocessor.getConfigs(tokens1);
});
Timer::run("Preprocessor::getConfigs", mSettings.showtime, &s_timerResults, [&]() {
Copy link
Owner

Choose a reason for hiding this comment

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

This code will always execute Preprocessor::getConfigs even if max configs is 1.

configurations = preprocessor.getConfigs(tokens1);
});

std::list<std::string> configs_total;
if (!mSettings.userDefines.empty()) {
configs_total.push_back(mSettings.userDefines);
}

int max_config_tmp;
if (mSettings.force && mSettings.maxConfigs <= 0) {
max_config_tmp = configurations.size();
} else if (mSettings.maxConfigs > 0) {
if (mSettings.preprocessOnly) {
max_config_tmp = 1;
} else {
max_config_tmp = mSettings.maxConfigs;
}
max_config_tmp = max_config_tmp - configs_total.size();
} else if (!mSettings.checkAllConfigurations) {
max_config_tmp = 1;
} else {
configurations.insert(mSettings.userDefines);
max_config_tmp = mSettings.maxConfigsDefault - configs_total.size();
}

for (auto it = configurations.begin(); it != configurations.end() && max_config_tmp > 0; ++it, max_config_tmp--) {
configs_total.push_back(*it);
}

if (mSettings.checkConfiguration) {
for (const std::string &config : configurations)
for (const std::string &config : configs_total)
(void)preprocessor.getcode(tokens1, config, files, true);

if (analyzerInformation)
Expand All @@ -1046,10 +1067,10 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
executeRules("define", tokenlist);
}
#endif

if (!mSettings.force && configurations.size() > mSettings.maxConfigs) {
int max_total = configurations.size() + (mSettings.userDefines.empty() ? 0 : 1);
if (!mSettings.force && max_total > mSettings.maxConfigsDefault) {
if (mSettings.severity.isEnabled(Severity::information)) {
tooManyConfigsError(Path::toNativeSeparators(file.spath()),configurations.size());
tooManyConfigsError(Path::toNativeSeparators(file.spath()), max_total);
} else {
mTooManyConfigs = true;
}
Expand All @@ -1069,30 +1090,14 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
}

std::set<unsigned long long> hashes;
int checkCount = 0;
bool hasValidConfig = false;
std::list<std::string> configurationError;
for (const std::string &currCfg : configurations) {
for (const std::string &currCfg : configs_total) {
// bail out if terminated
if (Settings::terminated())
break;

// Check only a few configurations (default 12), after that bail out, unless --force
// was used.
if (!mSettings.force && ++checkCount > mSettings.maxConfigs)
break;

if (!mSettings.userDefines.empty()) {
mCurrentConfig = mSettings.userDefines;
const std::vector<std::string> v1(split(mSettings.userDefines, ";"));
for (const std::string &cfg: split(currCfg, ";")) {
if (std::find(v1.cbegin(), v1.cend(), cfg) == v1.cend()) {
mCurrentConfig += ";" + cfg;
}
}
} else {
mCurrentConfig = currCfg;
}
mCurrentConfig = currCfg;

if (mSettings.preprocessOnly) {
std::string codeWithoutCfg;
Expand Down Expand Up @@ -1132,7 +1137,7 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
mLogger->setLocationMacros(tokenizer.tokens(), files);

// If only errors are printed, print filename after the check
if (!mSettings.quiet && (!mCurrentConfig.empty() || checkCount > 1)) {
if (!mSettings.quiet && (!mCurrentConfig.empty())) {
std::string fixedpath = Path::toNativeSeparators(file.spath());
mErrorLogger.reportOut("Checking " + fixedpath + ": " + mCurrentConfig + "...", Color::FgGreen);
}
Expand Down Expand Up @@ -1187,9 +1192,8 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
} catch (const simplecpp::Output &o) {
// #error etc during preprocessing
configurationError.push_back((mCurrentConfig.empty() ? "\'\'" : mCurrentConfig) + " : [" + o.location.file() + ':' + std::to_string(o.location.line) + "] " + o.msg);
--checkCount; // don't count invalid configurations

if (!hasValidConfig && currCfg == *configurations.rbegin()) {
if (!hasValidConfig && currCfg == *configs_total.rbegin()) {
// If there is no valid configuration then report error..
std::string locfile = Path::fromNativeSeparators(o.location.file());
if (mSettings.relativePaths)
Expand Down Expand Up @@ -1218,7 +1222,7 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
}
}

if (!hasValidConfig && configurations.size() > 1 && mSettings.severity.isEnabled(Severity::information)) {
if (!hasValidConfig && configs_total.size() > 1 && mSettings.severity.isEnabled(Severity::information)) {
std::string msg;
msg = "This file is not analyzed. Cppcheck failed to extract a valid configuration. Use -v for more details.";
msg += "\nThis file is not analyzed. Cppcheck failed to extract a valid configuration. The tested configurations have these preprocessor errors:";
Expand Down
4 changes: 3 additions & 1 deletion lib/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,11 @@ class CPPCHECKLIB WARN_UNUSED Settings {
int loadAverage{};
#endif

const static int maxConfigsDefault = 12;

/** @brief Maximum number of configurations to check before bailing.
Default is 12. (--max-configs=N) */
int maxConfigs = 12;
int maxConfigs = 0;

/** @brief --max-ctu-depth */
int maxCtuDepth = 2;
Expand Down
Loading