Skip to content

Commit 29a6031

Browse files
PKEuSdanmar
authored andcommitted
Fixed more false negatives (and recently introduced false positives) literalWithCharPtrCompare by using ValueType
Merged from LCppC.
1 parent b1ccad5 commit 29a6031

2 files changed

Lines changed: 23 additions & 36 deletions

File tree

lib/checkstring.cpp

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -181,46 +181,18 @@ void CheckString::checkSuspiciousStringCompare()
181181
else if (!Token::Match(litTok, "%char%|%num%|%str%"))
182182
continue;
183183

184-
if (mTokenizer->isCPP() && (!varTok->valueType() || !varTok->valueType()->isIntegral()))
184+
if (varTok->isLiteral())
185185
continue;
186186

187-
// Pointer addition?
188-
if (varTok->str() == "+" && mTokenizer->isC()) {
189-
const Token * const tokens[2] = { varTok->astOperand1(), varTok->astOperand2() };
190-
for (const Token * t : tokens) {
191-
while (t && (t->str() == "." || t->str() == "::"))
192-
t = t->astOperand2();
193-
if (t && t->variable() && t->variable()->isPointer())
194-
varTok = t;
195-
}
196-
}
197-
198-
const Token* oldVarTok = varTok;
199-
while (varTok->str() == "*") {
200-
if (!mTokenizer->isC() || varTok->astOperand2() != nullptr || litTok->tokType() != Token::eString)
201-
break;
202-
varTok = varTok->astOperand1();
203-
}
204-
205-
if (mTokenizer->isC() && varTok->str() == "&")
206-
varTok = varTok->astOperand1();
207-
208-
if (varTok->str() == "[")
209-
varTok = varTok->astOperand1();
210-
211-
while (varTok && (varTok->str() == "." || varTok->str() == "::"))
212-
varTok = varTok->astOperand2();
213-
if (!varTok || !varTok->isName())
187+
const ValueType* varType = varTok->valueType();
188+
if (mTokenizer->isCPP() && (!varType || !varType->isIntegral()))
214189
continue;
215190

216-
const Variable *var = varTok->variable();
217-
218-
const bool ischar(litTok->tokType() == Token::eChar);
219191
if (litTok->tokType() == Token::eString) {
220-
if (mTokenizer->isC() || (var && var->isArrayOrPointer()))
221-
suspiciousStringCompareError(tok, oldVarTok->expressionString(), litTok->isLong());
222-
} else if (ischar && var && var->isPointer()) {
223-
suspiciousStringCompareError_char(tok, oldVarTok->expressionString());
192+
if (mTokenizer->isC() || (varType && varType->pointer))
193+
suspiciousStringCompareError(tok, varTok->expressionString(), litTok->isLong());
194+
} else if (litTok->tokType() == Token::eChar && varType && varType->pointer) {
195+
suspiciousStringCompareError_char(tok, varTok->expressionString());
224196
}
225197
}
226198
}

test/teststring.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ class TestString : public TestFixture {
317317
check("bool foo(char* c) {\n"
318318
" return \"x\" == c+foo;\n"
319319
"}", "test.c");
320-
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
320+
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c+foo'. Did you intend to use strcmp() instead?\n", errout.str());
321321

322322
check("bool foo(Foo c) {\n"
323323
" return \"x\" == c.foo;\n"
@@ -424,6 +424,21 @@ class TestString : public TestFixture {
424424
"}");
425425
ASSERT_EQUALS("", errout.str());
426426

427+
check("bool foo(char* c) {\n"
428+
" return c[0] == '\\0';\n"
429+
"}");
430+
ASSERT_EQUALS("", errout.str());
431+
432+
check("bool foo(char** c) {\n"
433+
" return c[0] == '\\0';\n"
434+
"}");
435+
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c[0]'. Did you intend to dereference it?\n", errout.str());
436+
437+
check("bool foo(char** c) {\n"
438+
" return *c == '\\0';\n"
439+
"}");
440+
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer '*c'. Did you intend to dereference it?\n", errout.str());
441+
427442
check("bool foo(char c) {\n"
428443
" return c == 0;\n"
429444
"}");

0 commit comments

Comments
 (0)