classPublicInterfaceDivZero: Try to make the error message a bit better. Added variable name and what the bad input value is.

danmar · danmar · commit 15d814e609d5 · 2017-10-21T13:00:52.000+02:00
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -2469,14 +2469,17 @@ void CheckClass::checkPublicInterfaceDivZero(bool test)
                 if (!tok->astOperand2())
                     continue;
                 const Variable *var = tok->astOperand2()->variable();
-                if (var && var->isArgument())
-                    publicInterfaceDivZeroError(tok, classScope->className + "::" + func->name());
+                if (!var || !var->isArgument())
+                    continue;
+                publicInterfaceDivZeroError(tok, classScope->className, func->name(), var->name());
+                break;
             }
         }
     }
 }
 
-void CheckClass::publicInterfaceDivZeroError(const Token *tok, const std::string &functionName)
+void CheckClass::publicInterfaceDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName)
 {
-    reportError(tok, Severity::warning, "classPublicInterfaceDivZero", "Arbitrary usage of public method " + functionName + "() could result in division by zero.");
+    const std::string s = className + "::" + methodName + "()";
+    reportError(tok, Severity::warning, "classPublicInterfaceDivZero", "Public interface of " + className + " is not safe. When calling " + s + ", if parameter " + varName + " is 0 that leads to division by zero.");
 }
diff --git a/lib/checkclass.h b/lib/checkclass.h
@@ -187,7 +187,7 @@ class CPPCHECKLIB CheckClass : public Check {
     void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
     void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct);
     void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
-    void publicInterfaceDivZeroError(const Token *tok, const std::string &functionName);
+    void publicInterfaceDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckClass c(nullptr, settings, errorLogger);
@@ -218,7 +218,7 @@ class CPPCHECKLIB CheckClass : public Check {
         c.selfInitializationError(nullptr, "var");
         c.duplInheritedMembersError(nullptr, nullptr, "class", "class", "variable", false, false);
         c.copyCtorAndEqOperatorError(nullptr, "class", false, false);
-        c.publicInterfaceDivZeroError(nullptr, "Class::dostuff");
+        c.publicInterfaceDivZeroError(nullptr, "Class", "dostuff", "x");
     }
 
     static std::string myName() {
diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -6517,7 +6517,7 @@ class TestClass : public TestFixture {
                                     "  void dostuff(int x);\n"
                                     "}\n"
                                     "void A::dostuff(int x) { int a = 1000 / x; }");
-        ASSERT_EQUALS("[test.cpp:5]: (warning) Arbitrary usage of public method A::dostuff() could result in division by zero.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Public interface of A is not safe. When calling A::dostuff(), if parameter x is 0 that leads to division by zero.\n", errout.str());
 
         checkPublicInterfaceDivZero("class A {\n"
                                     "public:\n"
@@ -6526,7 +6526,7 @@ class TestClass : public TestFixture {
                                     "}\n"
                                     "void A::f1() {}\n"
                                     "void A::f2(int x) { int a = 1000 / x; }");
-        ASSERT_EQUALS("[test.cpp:7]: (warning) Arbitrary usage of public method A::f2() could result in division by zero.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:7]: (warning) Public interface of A is not safe. When calling A::f2(), if parameter x is 0 that leads to division by zero.\n", errout.str());
 
         checkPublicInterfaceDivZero("class A {\n"
                                     "public:\n"

Original file line number	Diff line number	Diff line change
`@@ -2469,14 +2469,17 @@ void CheckClass::checkPublicInterfaceDivZero(bool test)`
`2469`	`2469`	`if (!tok->astOperand2())`
`2470`	`2470`	`continue;`
`2471`	`2471`	`const Variable *var = tok->astOperand2()->variable();`
`2472`		`- if (var && var->isArgument())`
`2473`		`- publicInterfaceDivZeroError(tok, classScope->className + "::" + func->name());`
	`2472`	`+ if (!var \|\| !var->isArgument())`
	`2473`	`+ continue;`
	`2474`	`+ publicInterfaceDivZeroError(tok, classScope->className, func->name(), var->name());`
	`2475`	`+ break;`
`2474`	`2476`	`}`
`2475`	`2477`	`}`
`2476`	`2478`	`}`
`2477`	`2479`	`}`
`2478`	`2480`
`2479`		`-void CheckClass::publicInterfaceDivZeroError(const Token *tok, const std::string &functionName)`
	`2481`	`+void CheckClass::publicInterfaceDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName)`
`2480`	`2482`	`{`
`2481`		`- reportError(tok, Severity::warning, "classPublicInterfaceDivZero", "Arbitrary usage of public method " + functionName + "() could result in division by zero.");`
	`2483`	`+ const std::string s = className + "::" + methodName + "()";`
	`2484`	`+ reportError(tok, Severity::warning, "classPublicInterfaceDivZero", "Public interface of " + className + " is not safe. When calling " + s + ", if parameter " + varName + " is 0 that leads to division by zero.");`
`2482`	`2485`	`}`