Skip to content

Commit 91a01a0

Browse files
PKEuSPKEuS
committed
- checkUnsignedDivision checks for variable/variable (inconclusive). General bailout for if-statements.
- Make use of recently implemented symboldatabase functions (catch-support, reference-support) - Other refactorizations
1 parent 46b8dc5 commit 91a01a0

7 files changed

Lines changed: 93 additions & 75 deletions

File tree

‎lib/check.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class Check {
8888
virtual void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) = 0;
8989

9090
/** class name, used to generate documentation */
91-
std::string name() const {
91+
const std::string& name() const {
9292
return _name;
9393
}
9494

‎lib/checkautovariables.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ bool CheckAutoVariables::isAutoVar(unsigned int varId)
5454
if (!var || !var->isLocal() || var->isStatic() || var->isArray() || var->isPointer())
5555
return false;
5656

57-
if (Token::simpleMatch(var->nameToken()->previous(), "&")) {
57+
if (var->isReference()) {
5858
// address of reference variable can be taken if the address
5959
// of the variable it points at is not a auto-var
6060
// TODO: check what the reference variable references.

‎lib/checkother.cpp‎

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,20 +1066,17 @@ void CheckOther::checkCatchExceptionByValue()
10661066
if (!_settings->isEnabled("style"))
10671067
return;
10681068

1069-
const char catchPattern[] = "} catch (";
1070-
const Token *tok = Token::findmatch(_tokenizer->tokens(), catchPattern);
1071-
const Token *endTok = tok ? tok->linkAt(2) : NULL;
1069+
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
1070+
1071+
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
1072+
if (i->type != Scope::eCatch)
1073+
continue;
10721074

1073-
while (tok && endTok) {
10741075
// Find a pass-by-value declaration in the catch(), excluding basic types
10751076
// e.g. catch (std::exception err)
1076-
const Token *tokType = Token::findmatch(tok, "%type% %var% )", endTok);
1077-
if (tokType && !tokType->isStandardType()) {
1078-
catchExceptionByValueError(tokType);
1079-
}
1080-
1081-
tok = Token::findmatch(endTok->next(), catchPattern);
1082-
endTok = tok ? tok->linkAt(2) : NULL;
1077+
const Variable* var = symbolDatabase->getVariableFromVarId(i->classStart->tokAt(-2)->varId());
1078+
if (var && var->isClass() && !var->isPointer() && !var->isReference())
1079+
catchExceptionByValueError(i->classDef);
10831080
}
10841081
}
10851082

@@ -1677,32 +1674,51 @@ void CheckOther::unreachableCodeError(const Token *tok)
16771674
//---------------------------------------------------------------------------
16781675
static bool isUnsigned(const Variable* var)
16791676
{
1680-
return(var && var->typeStartToken()->isUnsigned() && var->typeStartToken() == var->typeEndToken());
1677+
return(var && var->typeStartToken()->isUnsigned() && !var->isPointer() && !var->isArray());
1678+
}
1679+
static bool isSigned(const Variable* var)
1680+
{
1681+
return(var && !var->typeStartToken()->isUnsigned() && Token::Match(var->typeEndToken(), "int|char|short|long") && !var->isPointer() && !var->isArray());
16811682
}
16821683

16831684
void CheckOther::checkUnsignedDivision()
16841685
{
16851686
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
1687+
bool style = _settings->isEnabled("style");
16861688

1689+
const Token* ifTok = 0;
16871690
// Check for "ivar / uvar" and "uvar / ivar"
16881691
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
1689-
if (!Token::Match(tok, "[).]") && Token::Match(tok->next(), "%var% / %num%")) {
1692+
if (Token::Match(tok, "[).]")) // Don't check members or casted variables
1693+
continue;
1694+
1695+
if (Token::Match(tok->next(), "%var% / %num%")) {
16901696
if (tok->strAt(3)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->next()->varId()))) {
1691-
udivError(tok->next());
1697+
udivError(tok->next(), false);
16921698
}
1693-
}
1694-
1695-
else if (Token::Match(tok, "(|[|=|%op% %num% / %var%")) {
1699+
} else if (Token::Match(tok->next(), "%num% / %var%")) {
16961700
if (tok->strAt(1)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()))) {
1697-
udivError(tok->next());
1701+
udivError(tok->next(), false);
16981702
}
1699-
}
1703+
} else if (Token::Match(tok->next(), "%var% / %var%") && _settings->inconclusive && style && !ifTok) {
1704+
const Variable* var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId());
1705+
const Variable* var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId());
1706+
if ((isUnsigned(var1) && isSigned(var2)) || (isUnsigned(var2) && isSigned(var1))) {
1707+
udivError(tok->next(), true);
1708+
}
1709+
} else if (!ifTok && Token::Match(tok, "if ("))
1710+
ifTok = tok->next()->link()->next()->link();
1711+
else if (ifTok == tok)
1712+
ifTok = 0;
17001713
}
17011714
}
17021715

1703-
void CheckOther::udivError(const Token *tok)
1716+
void CheckOther::udivError(const Token *tok, bool inconclusive)
17041717
{
1705-
reportError(tok, Severity::error, "udivError", "Unsigned division. The result will be wrong.");
1718+
if (inconclusive)
1719+
reportError(tok, Severity::warning, "udivError", "Division with signed and unsigned operators. The result might be wrong.");
1720+
else
1721+
reportError(tok, Severity::error, "udivError", "Unsigned division. The result will be wrong.");
17061722
}
17071723

17081724
//---------------------------------------------------------------------------
@@ -1956,9 +1972,14 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
19561972
//---------------------------------------------------------------------------
19571973
// Check usage of char variables..
19581974
//---------------------------------------------------------------------------
1975+
static bool isChar(const Variable* var)
1976+
{
1977+
return(var && !var->isPointer() && !var->isArray() && (var->typeEndToken()->str() == "char" || var->typeEndToken()->previous()->str() == "char"));
1978+
}
1979+
19591980
static bool isSignedChar(const Variable* var)
19601981
{
1961-
return(var && var->nameToken()->previous()->str() == "char" && !var->nameToken()->previous()->isUnsigned() && var->nameToken()->next()->str() != "[");
1982+
return(isChar(var) && !var->typeEndToken()->isUnsigned() && !var->typeEndToken()->previous()->isUnsigned());
19621983
}
19631984

19641985
void CheckOther::checkCharVariable()
@@ -2092,10 +2113,6 @@ void CheckOther::constStatementError(const Token *tok, const std::string &type)
20922113
//---------------------------------------------------------------------------
20932114
// str plus char
20942115
//---------------------------------------------------------------------------
2095-
static bool isChar(const Variable* var)
2096-
{
2097-
return(var && var->nameToken()->previous()->str() == "char" && var->nameToken()->next()->str() != "[");
2098-
}
20992116

21002117
void CheckOther::strPlusChar()
21012118
{
@@ -2159,7 +2176,9 @@ void CheckOther::checkMathFunctions()
21592176
continue;
21602177

21612178
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
2162-
if (tok->varId() == 0 && Token::Match(tok, "log|log10 ( %num% )")) {
2179+
if (tok->varId())
2180+
continue;
2181+
if (Token::Match(tok, "log|log10 ( %num% )")) {
21632182
bool isNegative = MathLib::isNegative(tok->strAt(2));
21642183
bool isInt = MathLib::isInt(tok->strAt(2));
21652184
bool isFloat = MathLib::isFloat(tok->strAt(2));
@@ -2175,33 +2194,29 @@ void CheckOther::checkMathFunctions()
21752194
}
21762195

21772196
// acos( x ), asin( x ) where x is defined for intervall [-1,+1], but not beyound
2178-
else if (tok->varId() == 0 &&
2179-
Token::Match(tok, "acos|asin ( %num% )") &&
2197+
else if (Token::Match(tok, "acos|asin ( %num% )") &&
21802198
std::fabs(MathLib::toDoubleNumber(tok->strAt(2))) > 1.0) {
21812199
mathfunctionCallError(tok);
21822200
}
21832201
// sqrt( x ): if x is negative the result is undefined
2184-
else if (tok->varId() == 0 &&
2185-
Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )") &&
2202+
else if (Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )") &&
21862203
MathLib::isNegative(tok->strAt(2))) {
21872204
mathfunctionCallError(tok);
21882205
}
21892206
// atan2 ( x , y): x and y can not be zero, because this is mathematically not defined
2190-
else if (tok->varId() == 0 &&
2191-
Token::Match(tok, "atan2 ( %num% , %num% )") &&
2207+
else if (Token::Match(tok, "atan2 ( %num% , %num% )") &&
21922208
MathLib::isNullValue(tok->strAt(2)) &&
21932209
MathLib::isNullValue(tok->strAt(4))) {
21942210
mathfunctionCallError(tok, 2);
21952211
}
21962212
// fmod ( x , y) If y is zero, then either a range error will occur or the function will return zero (implementation-defined).
2197-
else if (tok->varId() == 0 &&
2198-
Token::Match(tok, "fmod ( %num% , %num% )") &&
2199-
MathLib::isNullValue(tok->strAt(4))) {
2200-
mathfunctionCallError(tok, 2);
2213+
else if (Token::Match(tok, "fmod ( %any%")) {
2214+
const Token* nextArg = tok->tokAt(2)->nextArgument();
2215+
if (nextArg && nextArg->isNumber() && MathLib::isNullValue(nextArg->str()))
2216+
mathfunctionCallError(tok, 2);
22012217
}
22022218
// pow ( x , y) If x is zero, and y is negative --> division by zero
2203-
else if (tok->varId() == 0 &&
2204-
Token::Match(tok, "pow ( %num% , %num% )") &&
2219+
else if (Token::Match(tok, "pow ( %num% , %num% )") &&
22052220
MathLib::isNullValue(tok->strAt(2)) &&
22062221
MathLib::isNegative(tok->strAt(4))) {
22072222
mathfunctionCallError(tok, 2);

‎lib/checkother.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ class CheckOther : public Check {
269269
void cstyleCastError(const Token *tok);
270270
void dangerousUsageStrtolError(const Token *tok);
271271
void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
272-
void udivError(const Token *tok);
272+
void udivError(const Token *tok, bool inconclusive);
273273
void passedByValueError(const Token *tok, const std::string &parname);
274274
void constStatementError(const Token *tok, const std::string &type);
275275
void charArrayIndexError(const Token *tok);
@@ -320,7 +320,7 @@ class CheckOther : public Check {
320320
// error
321321
c.assignBoolToPointerError(0);
322322
c.sprintfOverlappingDataError(0, "varname");
323-
c.udivError(0);
323+
c.udivError(0, false);
324324
c.zerodivError(0);
325325
c.mathfunctionCallError(0);
326326
c.fflushOnInputStreamError(0, "stdin");

‎lib/checkunusedvar.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,11 +596,11 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
596596
type = Variables::referenceArray;
597597
else if (i->isArray())
598598
type = Variables::array;
599-
else if (i->nameToken()->previous()->str() == "&")
599+
else if (i->isReference())
600600
type = Variables::reference;
601601
else if (i->nameToken()->previous()->str() == "*" && i->nameToken()->strAt(-2) == "*")
602602
type = Variables::pointerPointer;
603-
else if (i->nameToken()->previous()->str() == "*" || Token::Match(i->nameToken()->tokAt(-2), "* %type%"))
603+
else if (i->isPointer())
604604
type = Variables::pointer;
605605
else if (i->typeEndToken()->isStandardType() || isRecordTypeWithoutSideEffects(*i) || Token::simpleMatch(i->nameToken()->tokAt(-3), "std :: string"))
606606
type = Variables::standard;

‎lib/checkunusedvar.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ class CheckUnusedVar : public Check {
5757

5858
/** @brief Run checks against the simplified token list */
5959
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
60-
CheckUnusedVar checkUnusedVar(tokenizer, settings, errorLogger);
60+
(void)tokenizer;
61+
(void)settings;
62+
(void)errorLogger;
6163
}
6264

6365
/** @brief %Check for unused function variables */

‎test/testdivision.cpp‎

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ class TestDivision : public TestFixture {
3535
{ }
3636

3737
private:
38-
void check(const char code[], bool style = true) {
38+
void check(const char code[], bool inconclusive = false) {
3939
// Clear the error buffer..
4040
errout.str("");
4141

4242
Settings settings;
43-
if (style)
44-
settings.addEnabled("style");
43+
settings.addEnabled("style");
44+
settings.inconclusive = inconclusive;
4545

4646
// Tokenize..
4747
Tokenizer tokenizer(&settings, this);
@@ -66,14 +66,20 @@ class TestDivision : public TestFixture {
6666
}
6767

6868
void division1() {
69+
check("void f() {\n"
70+
" int ivar = -2;\n"
71+
" unsigned int uvar = 2;\n"
72+
" return ivar / uvar;\n"
73+
"}", false);
74+
ASSERT_EQUALS("", errout.str());
75+
6976
check("void f()\n"
7077
"{\n"
7178
" int ivar = -2;\n"
7279
" unsigned int uvar = 2;\n"
7380
" return ivar / uvar;\n"
74-
"}\n");
75-
TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Division with signed and unsigned operators\n",
76-
"", errout.str());
81+
"}", true);
82+
ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
7783
}
7884

7985
void division2() {
@@ -82,9 +88,8 @@ class TestDivision : public TestFixture {
8288
" int ivar = -2;\n"
8389
" unsigned int uvar = 2;\n"
8490
" return uvar / ivar;\n"
85-
"}\n");
86-
TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Division with signed and unsigned operators\n",
87-
"", errout.str());
91+
"}", true);
92+
ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
8893
}
8994

9095
void division4() {
@@ -96,8 +101,8 @@ class TestDivision : public TestFixture {
96101
"void f2(unsigned int i1)\n"
97102
"{\n"
98103
" unsigned int i2;\n"
99-
" result = i2 / i1;}\n"
100-
);
104+
" result = i2 / i1;"
105+
"}", true);
101106
ASSERT_EQUALS("", errout.str());
102107

103108
check("void f1()\n"
@@ -107,8 +112,8 @@ class TestDivision : public TestFixture {
107112
"\n"
108113
"void f2(int X)\n"
109114
"{\n"
110-
" X = X / z;}\n"
111-
);
115+
" X = X / z;"
116+
"}", true);
112117
ASSERT_EQUALS("", errout.str());
113118
}
114119

@@ -117,8 +122,8 @@ class TestDivision : public TestFixture {
117122
"void foo()\n"
118123
"{\n"
119124
" unsigned int val = 32;\n"
120-
" val = val / USER_HASH;}\n"
121-
);
125+
" val = val / USER_HASH;"
126+
"}", true);
122127
ASSERT_EQUALS("", errout.str());
123128
}
124129

@@ -147,7 +152,7 @@ class TestDivision : public TestFixture {
147152
" unsigned int a;\n"
148153
" unsigned int c = a / b;\n"
149154
" }\n"
150-
"}\n", true);
155+
"}", true);
151156
ASSERT_EQUALS("", errout.str());
152157

153158
check("void foo(int b)\n"
@@ -157,16 +162,16 @@ class TestDivision : public TestFixture {
157162
" unsigned int a;\n"
158163
" unsigned int c = a / b;\n"
159164
" }\n"
160-
"}\n", true);
161-
TODO_ASSERT_EQUALS("unsigned division",
162-
"", errout.str());
165+
"}", true);
166+
TODO_ASSERT_EQUALS("[test.cpp:6]: (warning) Division with signed and unsigned operators. The result might be wrong.\n",
167+
"", errout.str());
163168

164169
check("void a(int i) { }\n"
165170
"int foo( unsigned int sz )\n"
166171
"{\n"
167172
" register unsigned int i=1;\n"
168173
" return i/sz;\n"
169-
"}\n", true);
174+
"}", true);
170175
ASSERT_EQUALS("", errout.str());
171176
}
172177

@@ -176,27 +181,23 @@ class TestDivision : public TestFixture {
176181
" int ivar = -2;\n"
177182
" unsigned long uvar = 2;\n"
178183
" return ivar / uvar;\n"
179-
"}\n");
180-
TODO_ASSERT_EQUALS("unsigned division",
181-
"", errout.str());
184+
"}", true);
185+
ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
182186

183187
check("void f()\n"
184188
"{\n"
185189
" int ivar = -2;\n"
186190
" unsigned long long uvar = 2;\n"
187191
" return ivar / uvar;\n"
188-
"}\n");
189-
TODO_ASSERT_EQUALS("unsigned division",
190-
"", errout.str());
192+
"}", true);
193+
ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
191194
}
192195

193196
void division10() {
194197
// Ticket: #2932 - don't segfault
195-
check("i / i");
198+
check("i / i", true);
196199
ASSERT_EQUALS("", errout.str());
197200
}
198201
};
199202

200203
REGISTER_TEST(TestDivision)
201-
202-

0 commit comments

Comments
 (0)