Skip to content

Commit 88d8a46

Browse files
clock999clock999
committed
fix #5591 
1 parent 5ac33c8 commit 88d8a46

9 files changed

+253
-0
lines changed

‎Makefile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \
204204
$(libcppdir)/checkleakautovar.o \
205205
$(libcppdir)/checkmemoryleak.o \
206206
$(libcppdir)/checknullpointer.o \
207+
$(libcppdir)/checkoperatorcomma.o \
207208
$(libcppdir)/checkother.o \
208209
$(libcppdir)/checkpostfixoperator.o \
209210
$(libcppdir)/checksizeof.o \
@@ -300,6 +301,7 @@ TESTOBJ = test/fixture.o \
300301
test/testmathlib.o \
301302
test/testmemleak.o \
302303
test/testnullpointer.o \
304+
test/testoperatorcomma.o \
303305
test/testoptions.o \
304306
test/testother.o \
305307
test/testpath.o \
@@ -521,6 +523,9 @@ $(libcppdir)/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/addoninfo.h lib/astu
521523
$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkers.h lib/checknullpointer.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/findtoken.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
522524
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
523525

526+
$(libcppdir)/checkoperatorcomma.o: lib/checkoperatorcomma.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkoperatorcomma.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
527+
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkoperatorcomma.cpp
528+
524529
$(libcppdir)/checkother.o: lib/checkother.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkers.h lib/checkother.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
525530
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkother.cpp
526531

@@ -794,6 +799,9 @@ test/testmemleak.o: test/testmemleak.cpp externals/simplecpp/simplecpp.h lib/add
794799
test/testnullpointer.o: test/testnullpointer.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
795800
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testnullpointer.cpp
796801

802+
test/testoperatorcomma.o: test/testoperatorcomma.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/checkoperatorcomma.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
803+
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testoperatorcomma.cpp
804+
797805
test/testoptions.o: test/testoptions.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h test/options.h
798806
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testoptions.cpp
799807

‎lib/checkoperatorcomma.cpp

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "checkoperatorcomma.h"
20+
#include "tokenize.h"
21+
#include "errortypes.h"
22+
#include "settings.h"
23+
#include "token.h"
24+
25+
// CWE ids used
26+
static const CWE CWE398(398U); // Indicator of Poor Code Quality
27+
28+
namespace {
29+
CheckOpComma instance;
30+
}
31+
32+
void CheckOpComma::runChecks(const Tokenizer &tokenizer, ErrorLogger * errorLogger) {
33+
CheckOpComma checkOpComma(&tokenizer, &tokenizer.getSettings(), errorLogger);
34+
checkOpComma.assertWithSuspiciousComma();
35+
}
36+
37+
void CheckOpComma::getErrorMessages(ErrorLogger * errorLogger, const Settings * settings) const {
38+
CheckOpComma c(nullptr, settings, errorLogger);
39+
c.assertWithSuspiciousCommaError(nullptr);
40+
}
41+
42+
void CheckOpComma::assertWithSuspiciousComma() {
43+
if (!mSettings->severity.isEnabled(Severity::style)) {
44+
return;
45+
}
46+
47+
logChecker("CheckOpComma::assertWithSuspiciousComma");
48+
49+
for (const Token* tok = mTokenizer->list.front(); tok; tok = tok->next()) {
50+
if (tok->str() == "," && tok->isBinaryOp()) {
51+
const Token * parent = tok->astParent();
52+
if (parent && (Token::simpleMatch(parent->previous(), "if (") ||
53+
Token::simpleMatch(parent->previous(), "while ("))) {
54+
assertWithSuspiciousCommaError(tok);
55+
continue;
56+
}
57+
if (parent && parent->str() == ";") {
58+
parent = parent->astParent();
59+
if (parent && parent->str() == ";") {
60+
parent = parent->astParent();
61+
if (Token::simpleMatch(parent->previous(), "for (")) {
62+
if (tok->index() < tok->astParent()->index()) {
63+
assertWithSuspiciousCommaError(tok);
64+
continue;
65+
}
66+
}
67+
}
68+
}
69+
if (parent && parent->isBinaryOp() && (parent->str() == "?" ||
70+
parent->str() == "&&" ||
71+
parent->str() == "||")) {
72+
assertWithSuspiciousCommaError(tok);
73+
}
74+
}
75+
}
76+
}
77+
78+
void CheckOpComma::assertWithSuspiciousCommaError(const Token * tok) {
79+
reportError(tok, Severity::style,
80+
"assertWithSuspiciousComma",
81+
"There is an suspicious comma expression used as a condition.",
82+
CWE398, Certainty::normal);
83+
}

‎lib/checkoperatorcomma.h

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/* -*- C++ -*-
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#ifndef checkoperatorcommaH
20+
#define checkoperatorcommaH
21+
22+
#include "check.h"
23+
24+
class CPPCHECKLIB CheckOpComma : public Check {
25+
public:
26+
CheckOpComma() : Check(myName()) {}
27+
private:
28+
CheckOpComma(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
29+
: Check(myName(), tokenizer, settings, errorLogger) {}
30+
static std::string myName() {
31+
return "CheckOpComma";
32+
}
33+
void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override;
34+
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
35+
std::string classInfo() const override {
36+
return "warn if there is any suspicious comma expression used as a condition, which might be a bad coding style\n";
37+
}
38+
39+
void assertWithSuspiciousComma();
40+
void assertWithSuspiciousCommaError(const Token *tok);
41+
};
42+
#endif

‎lib/cppcheck.vcxproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
<ClCompile Include="checkleakautovar.cpp" />
5050
<ClCompile Include="checkmemoryleak.cpp" />
5151
<ClCompile Include="checknullpointer.cpp" />
52+
<ClCompile Include="checkoperatorcomma.cpp" />
5253
<ClCompile Include="checkother.cpp" />
5354
<ClCompile Include="checkpostfixoperator.cpp" />
5455
<ClCompile Include="checksizeof.cpp" />
@@ -120,6 +121,7 @@
120121
<ClInclude Include="checkleakautovar.h" />
121122
<ClInclude Include="checkmemoryleak.h" />
122123
<ClInclude Include="checknullpointer.h" />
124+
<ClInclude Include="checkoperatorcomma.h" />
123125
<ClInclude Include="checkother.h" />
124126
<ClInclude Include="checkpostfixoperator.h" />
125127
<ClInclude Include="checksizeof.h" />

‎lib/cppcheck.vcxproj.filters

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838
<ClCompile Include="checkother.cpp">
3939
<Filter>Source Files</Filter>
4040
</ClCompile>
41+
<ClCompile Include="checkoperatorcomma.cpp">
42+
<Filter>Source Files</Filter>
43+
</ClCompile>
4144
<ClCompile Include="checkpostfixoperator.cpp">
4245
<Filter>Source Files</Filter>
4346
</ClCompile>
@@ -316,6 +319,9 @@
316319
<ClInclude Include="checkother.h">
317320
<Filter>Header Files</Filter>
318321
</ClInclude>
322+
<ClInclude Include="checkoperatorcomma.h">
323+
<Filter>Header Files</Filter>
324+
</ClInclude>
319325
<ClInclude Include="checkpostfixoperator.h">
320326
<Filter>Header Files</Filter>
321327
</ClInclude>

‎oss-fuzz/Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \
5757
$(libcppdir)/checkleakautovar.o \
5858
$(libcppdir)/checkmemoryleak.o \
5959
$(libcppdir)/checknullpointer.o \
60+
$(libcppdir)/checkoperatorcomma.o \
6061
$(libcppdir)/checkother.o \
6162
$(libcppdir)/checkpostfixoperator.o \
6263
$(libcppdir)/checksizeof.o \
@@ -216,6 +217,9 @@ $(libcppdir)/checkmemoryleak.o: ../lib/checkmemoryleak.cpp ../lib/addoninfo.h ..
216217
$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/checknullpointer.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/findtoken.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h
217218
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
218219

220+
$(libcppdir)/checkoperatorcomma.o: ../lib/checkoperatorcomma.cpp ../lib/addoninfo.h ../lib/check.h ../lib/checkers.h ../lib/checkoperatorcomma.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/standards.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
221+
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkoperatorcomma.cpp
222+
219223
$(libcppdir)/checkother.o: ../lib/checkother.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/checkother.h ../lib/config.h ../lib/errortypes.h ../lib/fwdanalysis.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h
220224
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkother.cpp
221225

‎test/testoperatorcomma.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2024 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "checkoperatorcomma.h"
20+
#include "errortypes.h"
21+
#include "fixture.h"
22+
#include "helpers.h"
23+
#include "settings.h"
24+
25+
#include <cstddef>
26+
27+
class TestOpComma : public TestFixture {
28+
public:
29+
TestOpComma() : TestFixture("TestOpComma") {}
30+
31+
private:
32+
const Settings settings = settingsBuilder().severity(Severity::style).build();
33+
34+
#define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
35+
template<size_t size>
36+
void check_(const char* file, int line, const char (&code)[size]) {
37+
SimpleTokenizer tokenizer(settings, *this);
38+
ASSERT_LOC(tokenizer.tokenize(code), file, line);
39+
40+
runChecks<CheckOpComma>(tokenizer, this);
41+
}
42+
43+
void run() override {
44+
TEST_CASE(commaInIfCondition);
45+
TEST_CASE(commaInWhileCondition);
46+
TEST_CASE(commaInForCondition);
47+
TEST_CASE(commaInForConditionalExp1);
48+
TEST_CASE(commaInForConditionalExp2);
49+
TEST_CASE(commaInForConditionalExp3);
50+
}
51+
52+
void commaInIfCondition() {
53+
check("void f() {\n"
54+
" int a; a = 100;\n"
55+
" if(a == 100, a != 100) {\n"
56+
" }\n"
57+
"}");
58+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
59+
}
60+
61+
void commaInWhileCondition() {
62+
check("void f() {\n"
63+
" int a; a = 100;\n"
64+
" while(a == 100, a != 100) {\n"
65+
" }\n"
66+
"}");
67+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
68+
}
69+
70+
void commaInForCondition() {
71+
check("void f() {\n"
72+
" int a;\n"
73+
" for(a = 0;a == 100, a != 100;a++) {\n"
74+
" }\n"
75+
"}");
76+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
77+
}
78+
79+
void commaInForConditionalExp1() {
80+
check("void f() {\n"
81+
" int a;int z;\n"
82+
" z = (a == 100, a != 100)?100 : 200;\n"
83+
"}");
84+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
85+
}
86+
87+
void commaInForConditionalExp2() {
88+
check("void f() {\n"
89+
" int a;int z;\n"
90+
" z = (a == 100, a != 100) && a == 0;\n"
91+
"}");
92+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
93+
}
94+
95+
void commaInForConditionalExp3() {
96+
check("void f() {\n"
97+
" int a;int z;\n"
98+
" z = (a == 100, a != 100) || a == 0;\n"
99+
"}");
100+
ASSERT_EQUALS("[test.cpp:3]: (style) There is an suspicious comma expression used as a condition.\n", errout_str());
101+
}
102+
};
103+
104+
REGISTER_TEST(TestOpComma)

‎test/testrunner.vcxproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
<ClCompile Include="testmathlib.cpp" />
7676
<ClCompile Include="testmemleak.cpp" />
7777
<ClCompile Include="testnullpointer.cpp" />
78+
<ClCompile Include="testoperatorcomma.cpp" />
7879
<ClCompile Include="testoptions.cpp" />
7980
<ClCompile Include="testother.cpp" />
8081
<ClCompile Include="testpath.cpp" />

‎test/testrunner.vcxproj.filters

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@
7676
<ClCompile Include="testother.cpp">
7777
<Filter>Source Files</Filter>
7878
</ClCompile>
79+
<ClCompile Include="testoperatorcomma.cpp">
80+
<Filter>Source Files</Filter>
81+
</ClCompile>
7982
<ClCompile Include="testpath.cpp">
8083
<Filter>Source Files</Filter>
8184
</ClCompile>

0 commit comments

Comments
 (0)