From 7f193fb7a14af162ac86f038e1066a9ffcf50f68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 30 Jul 2011 19:51:06 +0200 Subject: [PATCH] Fixed #2788 (null pointer: dereference and then check 'if (abc->a == 3) { } if (!abc) ..') --- lib/checknullpointer.cpp | 14 +++++++++++++- test/testnullpointer.cpp | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 545af92e5..6b0f0f0dd 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -378,6 +378,12 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec() tok1 = tok1->tokAt(3); } + // dereference in condition + else if (Token::Match(tok1, "if ( %var% .")) + { + tok1 = tok1->tokAt(2); + } + // dereference in function call (but not sizeof|decltype) else if ((Token::Match(tok1->tokAt(-2), "%var% ( %var% . %var%") && !Token::Match(tok1->tokAt(-2), "sizeof|decltype ( %var% . %var%")) || Token::Match(tok1->previous(), ", %var% . %var%")) @@ -441,7 +447,7 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec() else if (tok2->str() == "}") { - if (indentlevel2 <= 1) + if (indentlevel2 == 0) break; --indentlevel2; } @@ -454,7 +460,13 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec() else if (tok2->varId() == varid1) { if (tok2->next()->str() == "=") + { + // Avoid false positives when there is 'else if' + // TODO: can this be handled better? + if (tok1->strAt(-2) == "if") + skipvar.insert(varid1); break; + } if (Token::Match(tok2->tokAt(-2), "[,(] &")) break; } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 005c25657..ce9c17a7f 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -229,7 +229,7 @@ private: " }\n" " if (abc) {}\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 5\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 5\n", errout.str()); // TODO: False negative if member of member is dereferenced check("void foo(ABC *abc) {\n" @@ -516,7 +516,7 @@ private: " a = b ? c : d;\n" " if (item) { }\n" "}\n"); - TODO_ASSERT_EQUALS("error", "", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (error) Possible null pointer dereference: item - otherwise it is redundant to check if item is null at line 4\n", errout.str()); check("BOOL GotoFlyAnchor()\n" // #2243 "{\n"