Skip to content

Commit 82f33cd

Browse files
danmardanmar
committed
Fix #11824 (Option --max-configs has no effect if -D is used)
1 parent f077ec4 commit 82f33cd

9 files changed

Lines changed: 114 additions & 43 deletions

File tree

‎cli/cmdlineparser.cpp‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
407407
}
408408
}
409409

410-
bool def = false;
411-
bool maxconfigs = false;
412410
bool debug = false;
413411
bool inputAsFilter = false; // set by: --file-filter=+
414412

@@ -457,8 +455,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
457455
if (!mSettings.userDefines.empty())
458456
mSettings.userDefines += ";";
459457
mSettings.userDefines += define;
460-
461-
def = true;
462458
}
463459

464460
// -E
@@ -801,8 +797,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
801797
}
802798

803799
// Force checking of files that have "too many" configurations
804-
else if (std::strcmp(argv[i], "-f") == 0 || std::strcmp(argv[i], "--force") == 0)
800+
else if (std::strcmp(argv[i], "-f") == 0 || std::strcmp(argv[i], "--force") == 0) {
805801
mSettings.force = true;
802+
mSettings.maxConfigsOption = Settings::maxConfigsNotAssigned;
803+
}
806804

807805
else if (std::strcmp(argv[i], "--fsigned-char") == 0)
808806
defaultSign = 's';
@@ -974,9 +972,8 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
974972
return Result::Fail;
975973
}
976974

977-
mSettings.maxConfigs = tmp;
975+
mSettings.maxConfigsOption = tmp;
978976
mSettings.force = false;
979-
maxconfigs = true;
980977
}
981978

982979
// max ctu depth
@@ -1160,7 +1157,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
11601157
return Result::Fail;
11611158
}
11621159

1163-
mSettings.checkAllConfigurations = false; // Can be overridden with --max-configs or --force
11641160
std::string projectFile = argv[i]+10;
11651161
projectType = project.import(projectFile, &mSettings, &mSuppressions);
11661162
if (projectType == ImportProject::Type::CPPCHECK_GUI) {
@@ -1187,6 +1183,8 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
11871183
}
11881184
}
11891185
}
1186+
if (projectType == ImportProject::Type::COMPILE_DB)
1187+
mSettings.maxConfigsProject = 1;
11901188
if (projectType == ImportProject::Type::VS_SLN || projectType == ImportProject::Type::VS_VCXPROJ) {
11911189
mSettings.libraries.emplace_back("windows");
11921190
}
@@ -1591,14 +1589,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
15911589
substituteTemplateFormatStatic(mSettings.templateFormat, !mSettings.outputFile.empty());
15921590
substituteTemplateLocationStatic(mSettings.templateLocation, !mSettings.outputFile.empty());
15931591

1594-
if (mSettings.force || maxconfigs)
1595-
mSettings.checkAllConfigurations = true;
1596-
1597-
if (mSettings.force)
1598-
mSettings.maxConfigs = INT_MAX;
1599-
else if ((def || mSettings.preprocessOnly) && !maxconfigs)
1600-
mSettings.maxConfigs = 1U;
1601-
16021592
if (debug) {
16031593
mSettings.debugnormal = true;
16041594
mSettings.debugvalueflow = true;

‎gui/mainwindow.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,13 +1136,9 @@ bool MainWindow::getCppcheckSettings(Settings& settings, Suppressions& supprs)
11361136
supprs.nomsg.addSuppression(suppression); // TODO: check result
11371137
}
11381138

1139-
// Only check the given -D configuration
1140-
if (!defines.isEmpty())
1141-
settings.maxConfigs = 1;
1142-
11431139
// If importing a project, only check the given configuration
1144-
if (!mProjectFile->getImportProject().isEmpty())
1145-
settings.checkAllConfigurations = false;
1140+
if (mProjectFile->getImportProject().endsWith("json", Qt::CaseInsensitive))
1141+
settings.maxConfigsProject = 1;
11461142

11471143
const QString &buildDir = fromNativePath(mProjectFile->getBuildDir());
11481144
if (!buildDir.isEmpty()) {

‎lib/cppcheck.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -859,7 +859,7 @@ std::size_t CppCheck::calculateHash(const Preprocessor& preprocessor, const std:
859859
toolinfo << mSettings.userDefines;
860860
toolinfo << (mSettings.checkConfiguration ? 'c' : ' '); // --check-config
861861
toolinfo << (mSettings.force ? 'f' : ' ');
862-
toolinfo << mSettings.maxConfigs;
862+
toolinfo << mSettings.maxConfigsOption;
863863
toolinfo << std::to_string(static_cast<std::uint8_t>(mSettings.checkLevel));
864864
for (const auto &a : mSettings.addonInfos) {
865865
toolinfo << a.name;
@@ -908,6 +908,8 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
908908
if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
909909
mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
910910

911+
const int maxConfigs = mSettings.getMaxConfigs();
912+
911913
mLogger->resetExitCode();
912914

913915
if (Settings::terminated())
@@ -1032,7 +1034,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
10321034

10331035
// Get configurations..
10341036
std::set<std::string> configurations;
1035-
if ((mSettings.checkAllConfigurations && mSettings.userDefines.empty()) || mSettings.force) {
1037+
if (maxConfigs > 1) {
10361038
Timer::run("Preprocessor::getConfigs", mSettings.showtime, &s_timerResults, [&]() {
10371039
configurations = preprocessor.getConfigs();
10381040
});
@@ -1044,7 +1046,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
10441046
for (const std::string &config : configurations)
10451047
(void)preprocessor.getcode(config, files, false);
10461048

1047-
if (configurations.size() > mSettings.maxConfigs)
1049+
if (configurations.size() > maxConfigs)
10481050
tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
10491051

10501052
if (analyzerInformation)
@@ -1090,14 +1092,13 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
10901092

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

11031104
break;
@@ -1198,7 +1199,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
11981199
}
11991200

12001201
// Skip if we already met the same simplified token list
1201-
if (mSettings.force || mSettings.maxConfigs > 1) {
1202+
if (maxConfigs > 1) {
12021203
const std::size_t hash = tokenizer.list.calculateHash();
12031204
if (hashes.find(hash) != hashes.end()) {
12041205
if (mSettings.debugwarnings)
@@ -1644,7 +1645,7 @@ void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfCo
16441645
}
16451646

16461647
std::ostringstream msg;
1647-
msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs
1648+
msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.getMaxConfigs()
16481649
<< " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.";
16491650

16501651
ErrorMessage errmsg(std::move(loclist),

‎lib/settings.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343

4444
std::atomic<bool> Settings::mTerminated;
4545

46+
const int Settings::maxConfigsNotAssigned = 0;
47+
const int Settings::maxConfigsDefault = 12;
48+
4649
const char Settings::SafeChecks::XmlRootName[] = "safe-checks";
4750
const char Settings::SafeChecks::XmlClasses[] = "class-public";
4851
const char Settings::SafeChecks::XmlExternalFunctions[] = "external-functions";

‎lib/settings.h‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
136136
/** @brief --cppcheck-build-dir. Always uses / as path separator. No trailing path separator. */
137137
std::string buildDir;
138138

139-
/** @brief check all configurations (false if -D or --max-configs is used */
140-
bool checkAllConfigurations = true;
141-
142139
/** Is the 'configuration checking' wanted? */
143140
bool checkConfiguration{};
144141

@@ -294,9 +291,31 @@ class CPPCHECKLIB WARN_UNUSED Settings {
294291
int loadAverage{};
295292
#endif
296293

297-
/** @brief Maximum number of configurations to check before bailing.
298-
Default is 12. (--max-configs=N) */
299-
int maxConfigs = 12;
294+
/** --max-configs value */
295+
int maxConfigsOption = 0; // "Not Assigned" value
296+
297+
/** max configs from --project option */
298+
int maxConfigsProject = 0; // "Not Assigned" value
299+
300+
static const int maxConfigsNotAssigned;
301+
static const int maxConfigsDefault;
302+
303+
bool isMaxConfigsAssigned() const {
304+
return maxConfigsOption != maxConfigsNotAssigned || maxConfigsProject != maxConfigsNotAssigned;
305+
}
306+
307+
/** @brief Maximum number of configurations to check before bailing. */
308+
int getMaxConfigs() const {
309+
if (force)
310+
return 0x7fffffff;
311+
if (maxConfigsOption != maxConfigsNotAssigned)
312+
return maxConfigsOption;
313+
if (maxConfigsProject != maxConfigsNotAssigned)
314+
return maxConfigsProject;
315+
if (!userDefines.empty())
316+
return 1;
317+
return maxConfigsDefault;
318+
}
300319

301320
/** @brief --max-ctu-depth */
302321
int maxCtuDepth = 2;

‎test/cli/helloworld_test.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ def test_addon_with_gui_project(tmp_path):
123123
ret, stdout, stderr = cppcheck(args, cwd=tmp_path)
124124
filename = os.path.join('helloworld', 'main.c')
125125
assert ret == 0, stdout
126-
assert stdout == 'Checking %s ...\n' % filename
126+
assert stdout.strip().split('\n') == [
127+
'Checking %s ...' % filename,
128+
'Checking %s: SOME_CONFIG...' % filename
129+
]
127130
assert stderr == ('[%s:5]: (error) Division by zero.\n'
128131
'[%s:4]: (style) misra violation (use --rule-texts=<file> to get proper output)\n' % (filename, filename))
129132

‎test/cli/other_test.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3952,8 +3952,8 @@ def test_simplecpp_syntax_error(tmp_path):
39523952

39533953
@pytest.mark.parametrize('max_configs,number_of_configs,check_config,expected_warn', [
39543954
# max configs = default, max configs < number of configs => warn
3955-
(12, 20, False, True),
3956-
(12, 20, True, True),
3955+
(None, 20, False, True),
3956+
(None, 20, True, True),
39573957
39583958
# max configs != default, max configs < number of configs => warn if --check-config
39593959
(6, 20, False, False),
@@ -3971,7 +3971,12 @@ def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, exp
39713971
f.write(f'#{dir} defined(X{i})\nx = {i};\n')
39723972
f.write('#endif\n')
39733973

3974-
args = [f'--max-configs={max_configs}', '--enable=information', '--template=simple', str(test_file)]
3974+
args = ['--enable=information', '--template=simple', str(test_file)]
3975+
3976+
if max_configs is None:
3977+
max_configs = 12 # default value
3978+
else:
3979+
args = [f'--max-configs={max_configs}'] + args
39753980

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

‎test/testcmdlineparser.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1415,7 +1415,7 @@ class TestCmdlineParser : public TestFixture {
14151415
REDIRECT;
14161416
const char * const argv[] = {"cppcheck", "-f", "--max-configs=12", "file.cpp"};
14171417
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
1418-
ASSERT_EQUALS(12, settings->maxConfigs);
1418+
ASSERT_EQUALS(12, settings->maxConfigsOption);
14191419
ASSERT_EQUALS(false, settings->force);
14201420
}
14211421

‎test/testsettings.cpp‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ class TestSettings : public TestFixture {
3434
TEST_CASE(loadCppcheckCfgSafety);
3535
TEST_CASE(getNameAndVersion);
3636
TEST_CASE(checkLevelDefault);
37+
38+
TEST_CASE(getMaxConfigsDefault);
39+
TEST_CASE(getMaxConfigsOpt);
40+
TEST_CASE(getMaxConfigsForce);
41+
TEST_CASE(getMaxConfigsOptAndForce);
42+
TEST_CASE(getMaxConfigsDefines);
43+
TEST_CASE(getMaxConfigsDefinesAndOpt);
44+
TEST_CASE(getMaxConfigsOptAndProject);
3745
}
3846

3947
void simpleEnableGroup() const {
@@ -287,6 +295,52 @@ class TestSettings : public TestFixture {
287295
ASSERT_EQUALS(true, s.vfOptions.doConditionExpressionAnalysis);
288296
ASSERT_EQUALS(-1, s.vfOptions.maxForwardBranches);
289297
}
298+
299+
void getMaxConfigsDefault() const {
300+
Settings s;
301+
ASSERT_EQUALS(12, s.getMaxConfigs());
302+
}
303+
304+
void getMaxConfigsOpt() const {
305+
Settings s;
306+
s.maxConfigsOption = 1;
307+
ASSERT_EQUALS(1, s.getMaxConfigs());
308+
}
309+
310+
void getMaxConfigsForce() const {
311+
Settings s;
312+
s.force = true;
313+
ASSERT(s.getMaxConfigs() > 1000);
314+
}
315+
316+
void getMaxConfigsOptAndForce() const {
317+
Settings s;
318+
s.maxConfigsOption = 1;
319+
s.force = true;
320+
ASSERT(s.getMaxConfigs() > 1000);
321+
}
322+
323+
void getMaxConfigsDefines() const {
324+
Settings s;
325+
s.userDefines = "X=1";
326+
ASSERT_EQUALS(1, s.getMaxConfigs());
327+
}
328+
329+
void getMaxConfigsDefinesAndOpt() const {
330+
Settings s;
331+
s.userDefines = "X=1";
332+
s.maxConfigsOption = 3;
333+
ASSERT_EQUALS(3, s.getMaxConfigs());
334+
}
335+
336+
void getMaxConfigsOptAndProject() const {
337+
Settings s;
338+
s.maxConfigsOption = 3;
339+
s.maxConfigsProject = 1;
340+
ASSERT_EQUALS(3, s.getMaxConfigs());
341+
s.maxConfigsProject = 10;
342+
ASSERT_EQUALS(3, s.getMaxConfigs());
343+
}
290344
};
291345

292346
REGISTER_TEST(TestSettings)

0 commit comments

Comments
 (0)