Skip to content

Commit 80dee36

Browse files
jubnzvjubnzv
authored
library: Add new warning: ignoredReturnErrorCode (cppcheck-opensource#2877)
* library: Add optional "type" attribute to "use-retval" Added an optional "type" attribute to "use-retval" nodes in the configuration. When the return type of a function configured with `<use-retval type="error-code"\>` node does not used, the new style error "ignoredReturnErrorCode" will be generated. * Fix and improve patch after the initial review * Fixed severity level and [[nodiscard]] attribute * Fix incorrect condition * Remove redundant condition
1 parent 6f7f508 commit 80dee36

5 files changed

Lines changed: 51 additions & 13 deletions

File tree

‎lib/checkfunctions.cpp‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ void CheckFunctions::invalidFunctionArgStrError(const Token *tok, const std::str
192192
//---------------------------------------------------------------------------
193193
void CheckFunctions::checkIgnoredReturnValue()
194194
{
195-
if (!mSettings->isEnabled(Settings::WARNING))
195+
if (!mSettings->isEnabled(Settings::WARNING) && !mSettings->isEnabled(Settings::STYLE))
196196
return;
197197

198198
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
@@ -216,9 +216,15 @@ void CheckFunctions::checkIgnoredReturnValue()
216216
}
217217

218218
if ((!tok->function() || !Token::Match(tok->function()->retDef, "void %name%")) &&
219-
(mSettings->library.isUseRetVal(tok) || (tok->function() && tok->function()->isAttributeNodiscard())) &&
220-
!WRONG_DATA(!tok->next()->astOperand1(), tok)) {
221-
ignoredReturnValueError(tok, tok->next()->astOperand1()->expressionString());
219+
!WRONG_DATA(!tok->next()->astOperand1(), tok)) {
220+
const Library::UseRetValType retvalTy = mSettings->library.getUseRetValType(tok);
221+
if (mSettings->isEnabled(Settings::WARNING) &&
222+
((retvalTy == Library::UseRetValType::DEFAULT) ||
223+
(tok->function() && tok->function()->isAttributeNodiscard())))
224+
ignoredReturnValueError(tok, tok->next()->astOperand1()->expressionString());
225+
else if (mSettings->isEnabled(Settings::STYLE) &&
226+
retvalTy == Library::UseRetValType::ERROR_CODE)
227+
ignoredReturnErrorCode(tok, tok->next()->astOperand1()->expressionString());
222228
}
223229
}
224230
}
@@ -230,6 +236,11 @@ void CheckFunctions::ignoredReturnValueError(const Token* tok, const std::string
230236
"$symbol:" + function + "\nReturn value of function $symbol() is not used.", CWE252, false);
231237
}
232238

239+
void CheckFunctions::ignoredReturnErrorCode(const Token* tok, const std::string& function)
240+
{
241+
reportError(tok, Severity::style, "ignoredReturnErrorCode",
242+
"$symbol:" + function + "\nError code from the return value of function $symbol() is not used.", CWE252, false);
243+
}
233244

234245
//---------------------------------------------------------------------------
235246
// Detect passing wrong values to <cmath> functions like atan(0, x);

‎lib/checkfunctions.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ class CPPCHECKLIB CheckFunctions : public Check {
109109
void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr);
110110
void invalidFunctionArgStrError(const Token *tok, const std::string &functionName, nonneg int argnr);
111111
void ignoredReturnValueError(const Token* tok, const std::string& function);
112+
void ignoredReturnErrorCode(const Token* tok, const std::string& function);
112113
void mathfunctionCallWarning(const Token *tok, const nonneg int numParam = 1);
113114
void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp);
114115
void memsetZeroBytesError(const Token *tok);

‎lib/library.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -655,9 +655,12 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
655655
func.isconst = true; // a constant function is pure
656656
} else if (functionnodename == "leak-ignore")
657657
func.leakignore = true;
658-
else if (functionnodename == "use-retval")
659-
func.useretval = true;
660-
else if (functionnodename == "returnValue") {
658+
else if (functionnodename == "use-retval") {
659+
func.useretval = Library::UseRetValType::DEFAULT;
660+
if (const char *type = functionnode->Attribute("type"))
661+
if (std::strcmp(type, "error-code") == 0)
662+
func.useretval = Library::UseRetValType::ERROR_CODE;
663+
} else if (functionnodename == "returnValue") {
661664
if (const char *expr = functionnode->GetText())
662665
mReturnValue[name] = expr;
663666
if (const char *type = functionnode->Attribute("type"))
@@ -1278,14 +1281,14 @@ bool Library::formatstr_secure(const Token* ftok) const
12781281
return functions.at(getFunctionName(ftok)).formatstr_secure;
12791282
}
12801283

1281-
bool Library::isUseRetVal(const Token* ftok) const
1284+
Library::UseRetValType Library::getUseRetValType(const Token *ftok) const
12821285
{
12831286
if (isNotLibraryFunction(ftok))
1284-
return false;
1287+
return Library::UseRetValType::NONE;
12851288
const std::map<std::string, Function>::const_iterator it = functions.find(getFunctionName(ftok));
12861289
if (it != functions.cend())
12871290
return it->second.useretval;
1288-
return false;
1291+
return Library::UseRetValType::NONE;
12891292
}
12901293

12911294
const std::string& Library::returnValue(const Token *ftok) const

‎lib/library.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ class CPPCHECKLIB Library {
179179
bool isNotLibraryFunction(const Token *ftok) const;
180180
bool matchArguments(const Token *ftok, const std::string &functionName) const;
181181

182-
bool isUseRetVal(const Token* ftok) const;
182+
enum class UseRetValType { NONE, DEFAULT, ERROR_CODE };
183+
UseRetValType getUseRetValType(const Token* ftok) const;
183184

184185
const std::string& returnValue(const Token *ftok) const;
185186
const std::string& returnValueType(const Token *ftok) const;
@@ -306,12 +307,12 @@ class CPPCHECKLIB Library {
306307
bool leakignore;
307308
bool isconst;
308309
bool ispure;
309-
bool useretval;
310+
UseRetValType useretval;
310311
bool ignore; // ignore functions/macros from a library (gtk, qt etc)
311312
bool formatstr;
312313
bool formatstr_scan;
313314
bool formatstr_secure;
314-
Function() : use(false), leakignore(false), isconst(false), ispure(false), useretval(false), ignore(false), formatstr(false), formatstr_scan(false), formatstr_secure(false) {}
315+
Function() : use(false), leakignore(false), isconst(false), ispure(false), useretval(UseRetValType::NONE), ignore(false), formatstr(false), formatstr_scan(false), formatstr_secure(false) {}
315316
};
316317

317318
const Function *getFunction(const Token *ftok) const;

‎test/testfunctions.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class TestFunctions : public TestFixture {
7676

7777
// Ignored return value
7878
TEST_CASE(checkIgnoredReturnValue);
79+
TEST_CASE(checkIgnoredErrorCode);
7980

8081
// memset..
8182
TEST_CASE(memsetZeroBytes);
@@ -1232,6 +1233,27 @@ class TestFunctions : public TestFixture {
12321233
ASSERT_EQUALS("", errout.str());
12331234
}
12341235

1236+
void checkIgnoredErrorCode() {
1237+
Settings settings2;
1238+
settings2.addEnabled("style");
1239+
const char xmldata[] = "<?xml version=\"1.0\"?>\n"
1240+
"<def version=\"2\">\n"
1241+
" <function name=\"mystrcmp\">\n"
1242+
" <use-retval type=\"error-code\"/>\n"
1243+
" <arg nr=\"1\"/>\n"
1244+
" <arg nr=\"2\"/>\n"
1245+
" </function>\n"
1246+
"</def>";
1247+
tinyxml2::XMLDocument doc;
1248+
doc.Parse(xmldata, sizeof(xmldata));
1249+
settings2.library.load(doc);
1250+
1251+
check("void foo() {\n"
1252+
" mystrcmp(a, b);\n"
1253+
"}", "test.cpp", &settings2);
1254+
ASSERT_EQUALS("[test.cpp:2]: (style) Error code from the return value of function mystrcmp() is not used.\n", errout.str());
1255+
}
1256+
12351257
void memsetZeroBytes() {
12361258
check("void f() {\n"
12371259
" memset(p, 10, 0x0);\n"

0 commit comments

Comments
 (0)