- checkUnsignedDivision checks for variable/variable (inconclusive). General bailout for if-statements.

- Make use of recently implemented symboldatabase functions (catch-support, reference-support)
- Other refactorizations

lib/check.h

@@ -88,7 +88,7 @@ public:
     virtual void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) = 0;
 
     /** class name, used to generate documentation */
-    std::string name() const {
+    const std::string& name() const {
         return _name;
     }
 

lib/checkautovariables.cpp

@@ -54,7 +54,7 @@ bool CheckAutoVariables::isAutoVar(unsigned int varId)
     if (!var || !var->isLocal() || var->isStatic() || var->isArray() || var->isPointer())
         return false;
 
-    if (Token::simpleMatch(var->nameToken()->previous(), "&")) {
+    if (var->isReference()) {
         // address of reference variable can be taken if the address
         // of the variable it points at is not a auto-var
         // TODO: check what the reference variable references.

lib/checkother.cpp

@@ -1066,20 +1066,17 @@ void CheckOther::checkCatchExceptionByValue()
     if (!_settings->isEnabled("style"))
         return;
 
-    const char catchPattern[] = "} catch (";
+    const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
-    const Token *tok = Token::findmatch(_tokenizer->tokens(), catchPattern);
+
-    const Token *endTok = tok ? tok->linkAt(2) : NULL;
+    for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
+        if (i->type != Scope::eCatch)
+            continue;
 
-    while (tok && endTok) {
         // Find a pass-by-value declaration in the catch(), excluding basic types
         // e.g. catch (std::exception err)
-        const Token *tokType = Token::findmatch(tok, "%type% %var% )", endTok);
+        const Variable* var = symbolDatabase->getVariableFromVarId(i->classStart->tokAt(-2)->varId());
-        if (tokType && !tokType->isStandardType()) {
+        if (var && var->isClass() && !var->isPointer() && !var->isReference())
-            catchExceptionByValueError(tokType);
+            catchExceptionByValueError(i->classDef);
-        }
-
-        tok = Token::findmatch(endTok->next(), catchPattern);
-        endTok = tok ? tok->linkAt(2) : NULL;
     }
 }
 
@@ -1677,32 +1674,51 @@ void CheckOther::unreachableCodeError(const Token *tok)
 //---------------------------------------------------------------------------
 static bool isUnsigned(const Variable* var)
 {
-    return(var && var->typeStartToken()->isUnsigned() && var->typeStartToken() == var->typeEndToken());
+    return(var && var->typeStartToken()->isUnsigned() && !var->isPointer() && !var->isArray());
+}
+static bool isSigned(const Variable* var)
+{
+    return(var && !var->typeStartToken()->isUnsigned() && Token::Match(var->typeEndToken(), "int|char|short|long") && !var->isPointer() && !var->isArray());
 }
 
 void CheckOther::checkUnsignedDivision()
 {
     const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
+    bool style = _settings->isEnabled("style");
 
+    const Token* ifTok = 0;
     // Check for "ivar / uvar" and "uvar / ivar"
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
-        if (!Token::Match(tok, "[).]") && Token::Match(tok->next(), "%var% / %num%")) {
+        if (Token::Match(tok, "[).]")) // Don't check members or casted variables
-            if (tok->strAt(3)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->next()->varId()))) {
+            continue;
-                udivError(tok->next());
-            }
-        }
 
-        else if (Token::Match(tok, "(|[|=|%op% %num% / %var%")) {
+        if (Token::Match(tok->next(), "%var% / %num%")) {
-            if (tok->strAt(1)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()))) {
+            if (tok->strAt(3)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->next()->varId()))) {
-                udivError(tok->next());
+                udivError(tok->next(), false);
             }
-        }
+        } else if (Token::Match(tok->next(), "%num% / %var%")) {
+            if (tok->strAt(1)[0] == '-' && isUnsigned(symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()))) {
+                udivError(tok->next(), false);
+            }
+        } else if (Token::Match(tok->next(), "%var% / %var%") && _settings->inconclusive && style && !ifTok) {
+            const Variable* var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId());
+            const Variable* var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId());
+            if ((isUnsigned(var1) && isSigned(var2)) || (isUnsigned(var2) && isSigned(var1))) {
+                udivError(tok->next(), true);
+            }
+        } else if (!ifTok && Token::Match(tok, "if ("))
+            ifTok = tok->next()->link()->next()->link();
+        else if (ifTok == tok)
+            ifTok = 0;
     }
 }
 
-void CheckOther::udivError(const Token *tok)
+void CheckOther::udivError(const Token *tok, bool inconclusive)
 {
-    reportError(tok, Severity::error, "udivError", "Unsigned division. The result will be wrong.");
+    if (inconclusive)
+        reportError(tok, Severity::warning, "udivError", "Division with signed and unsigned operators. The result might be wrong.");
+    else
+        reportError(tok, Severity::error, "udivError", "Unsigned division. The result will be wrong.");
 }
 
 //---------------------------------------------------------------------------
@@ -1956,9 +1972,14 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
 //---------------------------------------------------------------------------
 // Check usage of char variables..
 //---------------------------------------------------------------------------
+static bool isChar(const Variable* var)
+{
+    return(var && !var->isPointer() && !var->isArray() && (var->typeEndToken()->str() == "char" || var->typeEndToken()->previous()->str() == "char"));
+}
+
 static bool isSignedChar(const Variable* var)
 {
-    return(var && var->nameToken()->previous()->str() == "char" && !var->nameToken()->previous()->isUnsigned() && var->nameToken()->next()->str() != "[");
+    return(isChar(var) && !var->typeEndToken()->isUnsigned() && !var->typeEndToken()->previous()->isUnsigned());
 }
 
 void CheckOther::checkCharVariable()
@@ -2092,10 +2113,6 @@ void CheckOther::constStatementError(const Token *tok, const std::string &type)
 //---------------------------------------------------------------------------
 // str plus char
 //---------------------------------------------------------------------------
-static bool isChar(const Variable* var)
-{
-    return(var && var->nameToken()->previous()->str() == "char" && var->nameToken()->next()->str() != "[");
-}
 
 void CheckOther::strPlusChar()
 {
@@ -2159,7 +2176,9 @@ void CheckOther::checkMathFunctions()
             continue;
 
         for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
-            if (tok->varId() == 0 && Token::Match(tok, "log|log10 ( %num% )")) {
+            if (tok->varId())
+                continue;
+            if (Token::Match(tok, "log|log10 ( %num% )")) {
                 bool isNegative = MathLib::isNegative(tok->strAt(2));
                 bool isInt = MathLib::isInt(tok->strAt(2));
                 bool isFloat = MathLib::isFloat(tok->strAt(2));
@@ -2175,33 +2194,29 @@ void CheckOther::checkMathFunctions()
             }
 
             // acos( x ), asin( x )  where x is defined for intervall [-1,+1], but not beyound
-            else if (tok->varId() == 0 &&
+            else if (Token::Match(tok, "acos|asin ( %num% )") &&
-                     Token::Match(tok, "acos|asin ( %num% )") &&
                      std::fabs(MathLib::toDoubleNumber(tok->strAt(2))) > 1.0) {
                 mathfunctionCallError(tok);
             }
             // sqrt( x ): if x is negative the result is undefined
-            else if (tok->varId() == 0 &&
+            else if (Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )") &&
-                     Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )") &&
                      MathLib::isNegative(tok->strAt(2))) {
                 mathfunctionCallError(tok);
             }
             // atan2 ( x , y): x and y can not be zero, because this is mathematically not defined
-            else if (tok->varId() == 0 &&
+            else if (Token::Match(tok, "atan2 ( %num% , %num% )") &&
-                     Token::Match(tok, "atan2 ( %num% , %num% )") &&
                      MathLib::isNullValue(tok->strAt(2)) &&
                      MathLib::isNullValue(tok->strAt(4))) {
                 mathfunctionCallError(tok, 2);
             }
             // fmod ( x , y) If y is zero, then either a range error will occur or the function will return zero (implementation-defined).
-            else if (tok->varId() == 0 &&
+            else if (Token::Match(tok, "fmod ( %any%")) {
-                     Token::Match(tok, "fmod ( %num% , %num% )") &&
+                const Token* nextArg = tok->tokAt(2)->nextArgument();
-                     MathLib::isNullValue(tok->strAt(4))) {
+                if (nextArg && nextArg->isNumber() && MathLib::isNullValue(nextArg->str()))
-                mathfunctionCallError(tok, 2);
+                    mathfunctionCallError(tok, 2);
             }
             // pow ( x , y) If x is zero, and y is negative --> division by zero
-            else if (tok->varId() == 0 &&
+            else if (Token::Match(tok, "pow ( %num% , %num% )") &&
-                     Token::Match(tok, "pow ( %num% , %num% )") &&
                      MathLib::isNullValue(tok->strAt(2))  &&
                      MathLib::isNegative(tok->strAt(4))) {
                 mathfunctionCallError(tok, 2);

lib/checkother.h

@@ -269,7 +269,7 @@ public:
     void cstyleCastError(const Token *tok);
     void dangerousUsageStrtolError(const Token *tok);
     void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
-    void udivError(const Token *tok);
+    void udivError(const Token *tok, bool inconclusive);
     void passedByValueError(const Token *tok, const std::string &parname);
     void constStatementError(const Token *tok, const std::string &type);
     void charArrayIndexError(const Token *tok);
@@ -320,7 +320,7 @@ public:
         // error
         c.assignBoolToPointerError(0);
         c.sprintfOverlappingDataError(0, "varname");
-        c.udivError(0);
+        c.udivError(0, false);
         c.zerodivError(0);
         c.mathfunctionCallError(0);
         c.fflushOnInputStreamError(0, "stdin");

lib/checkunusedvar.cpp

@@ -596,11 +596,11 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
             type = Variables::referenceArray;
         else if (i->isArray())
             type = Variables::array;
-        else if (i->nameToken()->previous()->str() == "&")
+        else if (i->isReference())
             type = Variables::reference;
         else if (i->nameToken()->previous()->str() == "*" && i->nameToken()->strAt(-2) == "*")
             type = Variables::pointerPointer;
-        else if (i->nameToken()->previous()->str() == "*" || Token::Match(i->nameToken()->tokAt(-2), "* %type%"))
+        else if (i->isPointer())
             type = Variables::pointer;
         else if (i->typeEndToken()->isStandardType() || isRecordTypeWithoutSideEffects(*i) || Token::simpleMatch(i->nameToken()->tokAt(-3), "std :: string"))
             type = Variables::standard;

lib/checkunusedvar.h

@@ -57,7 +57,9 @@ public:
 
     /** @brief Run checks against the simplified token list */
     void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
-        CheckUnusedVar checkUnusedVar(tokenizer, settings, errorLogger);
+        (void)tokenizer;
+        (void)settings;
+        (void)errorLogger;
     }
 
     /** @brief %Check for unused function variables */

test/testdivision.cpp

@@ -35,13 +35,13 @@ public:
     { }
 
 private:
-    void check(const char code[], bool style = true) {
+    void check(const char code[], bool inconclusive = false) {
         // Clear the error buffer..
         errout.str("");
 
         Settings settings;
-        if (style)
+        settings.addEnabled("style");
-            settings.addEnabled("style");
+        settings.inconclusive = inconclusive;
 
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -66,14 +66,20 @@ private:
     }
 
     void division1() {
+        check("void f() {\n"
+              "    int ivar = -2;\n"
+              "    unsigned int uvar = 2;\n"
+              "    return ivar / uvar;\n"
+              "}", false);
+        ASSERT_EQUALS("", errout.str());
+
         check("void f()\n"
               "{\n"
               "    int ivar = -2;\n"
               "    unsigned int uvar = 2;\n"
               "    return ivar / uvar;\n"
-              "}\n");
+              "}", true);
-        TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Division with signed and unsigned operators\n",
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
-                           "", errout.str());
     }
 
     void division2() {
@@ -82,9 +88,8 @@ private:
               "    int ivar = -2;\n"
               "    unsigned int uvar = 2;\n"
               "    return uvar / ivar;\n"
-              "}\n");
+              "}", true);
-        TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Division with signed and unsigned operators\n",
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
-                           "", errout.str());
     }
 
     void division4() {
@@ -96,8 +101,8 @@ private:
               "void f2(unsigned int i1)\n"
               "{\n"
               "    unsigned int i2;\n"
-              "    result = i2 / i1;}\n"
+              "    result = i2 / i1;"
-             );
+              "}", true);
         ASSERT_EQUALS("", errout.str());
 
         check("void f1()\n"
@@ -107,8 +112,8 @@ private:
               "\n"
               "void f2(int X)\n"
               "{\n"
-              "    X = X / z;}\n"
+              "    X = X / z;"
-             );
+              "}", true);
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -117,8 +122,8 @@ private:
               "void foo()\n"
               "{\n"
               "    unsigned int val = 32;\n"
-              "    val = val / USER_HASH;}\n"
+              "    val = val / USER_HASH;"
-             );
+              "}", true);
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -147,7 +152,7 @@ private:
               "         unsigned int a;\n"
               "         unsigned int c = a / b;\n"
               "    }\n"
-              "}\n", true);
+              "}", true);
         ASSERT_EQUALS("", errout.str());
 
         check("void foo(int b)\n"
@@ -157,16 +162,16 @@ private:
               "         unsigned int a;\n"
               "         unsigned int c = a / b;\n"
               "    }\n"
-              "}\n", true);
+              "}", true);
-        TODO_ASSERT_EQUALS("unsigned division",
+        TODO_ASSERT_EQUALS("[test.cpp:6]: (warning) Division with signed and unsigned operators. The result might be wrong.\n",
-                           "",  errout.str());
+                           "", errout.str());
 
         check("void a(int i) { }\n"
               "int foo( unsigned int sz )\n"
               "{\n"
               "    register unsigned int i=1;\n"
               "    return i/sz;\n"
-              "}\n", true);
+              "}", true);
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -176,27 +181,23 @@ private:
               "    int ivar = -2;\n"
               "    unsigned long uvar = 2;\n"
               "    return ivar / uvar;\n"
-              "}\n");
+              "}", true);
-        TODO_ASSERT_EQUALS("unsigned division",
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
-                           "", errout.str());
 
         check("void f()\n"
               "{\n"
               "    int ivar = -2;\n"
               "    unsigned long long uvar = 2;\n"
               "    return ivar / uvar;\n"
-              "}\n");
+              "}", true);
-        TODO_ASSERT_EQUALS("unsigned division",
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", errout.str());
-                           "", errout.str());
     }
 
     void division10() {
         // Ticket: #2932 - don't segfault
-        check("i / i");
+        check("i / i", true);
         ASSERT_EQUALS("", errout.str());
     }
 };
 
 REGISTER_TEST(TestDivision)
-
-