misra: Fix 8.2 false positives (#3309)
* misra: Fix 8.2 false positives Fix false positives in rule 8.2 that occurred in cases when we have a function definition and declaration in the same file. For example, the following code generated false positives before this commit: ``` void f(uint8_t * const x); void f(uint8_t * const x) { (void)x; } ``` We need to distinguish the declaration and the definition, so the dump file generation routine was extended to keep token where the definition of the function. The analysis in the addon also been improved. Closes Trac issue: https://trac.cppcheck.net/ticket/10219
This commit is contained in:
parent
05df31c12a
commit
b89f5fbeff
|
@ -415,6 +415,7 @@ class Function:
|
|||
|
||||
Attributes
|
||||
argument Argument list
|
||||
token Token in function implementation
|
||||
tokenDef Token in function definition
|
||||
isVirtual Is this function is virtual
|
||||
isImplicitlyVirtual Is this function is virtual this in the base classes
|
||||
|
@ -424,6 +425,8 @@ class Function:
|
|||
Id = None
|
||||
argument = None
|
||||
argumentId = None
|
||||
token = None
|
||||
tokenId = None
|
||||
tokenDef = None
|
||||
tokenDefId = None
|
||||
name = None
|
||||
|
@ -435,6 +438,7 @@ class Function:
|
|||
|
||||
def __init__(self, element, nestedIn):
|
||||
self.Id = element.get('id')
|
||||
self.tokenId = element.get('token')
|
||||
self.tokenDefId = element.get('tokenDef')
|
||||
self.name = element.get('name')
|
||||
self.type = element.get('type')
|
||||
|
@ -450,7 +454,7 @@ class Function:
|
|||
self.argumentId = {}
|
||||
|
||||
def __repr__(self):
|
||||
attrs = ["Id", "tokenDefId", "name", "type", "isVirtual",
|
||||
attrs = ["Id", "tokenId", "tokenDefId", "name", "type", "isVirtual",
|
||||
"isImplicitlyVirtual", "isStatic", "argumentId"]
|
||||
return "{}({})".format(
|
||||
"Function",
|
||||
|
@ -460,6 +464,7 @@ class Function:
|
|||
def setId(self, IdMap):
|
||||
for argnr, argid in self.argumentId.items():
|
||||
self.argument[argnr] = IdMap[argid]
|
||||
self.token = IdMap.get(self.tokenId, None)
|
||||
self.tokenDef = IdMap[self.tokenDefId]
|
||||
|
||||
|
||||
|
|
|
@ -1476,21 +1476,10 @@ class MisraChecker:
|
|||
if rawToken is None:
|
||||
break
|
||||
following.append(rawToken)
|
||||
|
||||
return following
|
||||
|
||||
# Check arguments in function declaration
|
||||
for func in data.functions:
|
||||
|
||||
startCall = func.tokenDef.next
|
||||
if startCall is None or startCall.str != '(':
|
||||
continue
|
||||
|
||||
endCall = startCall.link
|
||||
if endCall is None or endCall.str != ')':
|
||||
continue
|
||||
|
||||
# Zero arguments should be in form ( void )
|
||||
# Zero arguments should be in form ( void )
|
||||
def checkZeroArguments(func, startCall, endCall):
|
||||
if (len(func.argument) == 0):
|
||||
voidArg = startCall.next
|
||||
while voidArg is not endCall:
|
||||
|
@ -1500,23 +1489,85 @@ class MisraChecker:
|
|||
if not voidArg.str == 'void':
|
||||
self.reportError(func.tokenDef, 8, 2)
|
||||
|
||||
def checkDeclarationArgumentsViolations(func, startCall, endCall):
|
||||
# Collect the tokens for the arguments in function definition
|
||||
argNameTokens = set()
|
||||
for arg in func.argument:
|
||||
argument = func.argument[arg]
|
||||
typeStartToken = argument.typeStartToken
|
||||
if typeStartToken is None:
|
||||
continue
|
||||
nameToken = argument.nameToken
|
||||
if nameToken is None:
|
||||
continue
|
||||
argNameTokens.add(nameToken)
|
||||
|
||||
# Check if we have the same number of variables in both the
|
||||
# declaration and the definition.
|
||||
#
|
||||
# TODO: We actually need to check if the names of the arguments are
|
||||
# the same. But we can't do this because we have no links to
|
||||
# variables in the arguments in function defintion in the dump file.
|
||||
foundVariables = 0
|
||||
while startCall and startCall != endCall:
|
||||
if startCall.varId:
|
||||
foundVariables += 1
|
||||
startCall = startCall.next
|
||||
|
||||
if len(argNameTokens) != foundVariables:
|
||||
self.reportError(func.tokenDef, 8, 2)
|
||||
|
||||
def checkDefinitionArgumentsViolations(func, startCall, endCall):
|
||||
for arg in func.argument:
|
||||
argument = func.argument[arg]
|
||||
typeStartToken = argument.typeStartToken
|
||||
if typeStartToken is None:
|
||||
continue
|
||||
|
||||
nameToken = argument.nameToken
|
||||
# Arguments should have a name unless variable length arg
|
||||
nameToken = argument.nameToken
|
||||
if nameToken is None and typeStartToken.str != '...':
|
||||
self.reportError(typeStartToken, 8, 2)
|
||||
|
||||
# Type declaration on next line (old style declaration list) is not allowed
|
||||
if ((typeStartToken.file == endCall.file) and
|
||||
((typeStartToken.linenr > endCall.linenr) or
|
||||
(typeStartToken.column > endCall.column))):
|
||||
if ((typeStartToken.linenr > endCall.linenr) or
|
||||
(typeStartToken.column > endCall.column)):
|
||||
self.reportError(typeStartToken, 8, 2)
|
||||
|
||||
# Check arguments in function declaration
|
||||
for func in data.functions:
|
||||
|
||||
# Check arguments in function definition
|
||||
tokenImpl = func.token
|
||||
if tokenImpl:
|
||||
startCall = tokenImpl.next
|
||||
if startCall is None or startCall.str != '(':
|
||||
continue
|
||||
endCall = startCall.link
|
||||
if endCall is None or endCall.str != ')':
|
||||
continue
|
||||
checkZeroArguments(func, startCall, endCall)
|
||||
checkDefinitionArgumentsViolations(func, startCall, endCall)
|
||||
|
||||
# Check arguments in function declaration
|
||||
tokenDef = func.tokenDef
|
||||
if tokenDef:
|
||||
startCall = func.tokenDef.next
|
||||
if startCall is None or startCall.str != '(':
|
||||
continue
|
||||
endCall = startCall.link
|
||||
if endCall is None or endCall.str != ')':
|
||||
continue
|
||||
checkZeroArguments(func, startCall, endCall)
|
||||
if tokenImpl:
|
||||
checkDeclarationArgumentsViolations(func, startCall, endCall)
|
||||
else:
|
||||
# When there is no function definition, we should execute
|
||||
# its checks for the declaration token. The point is that without
|
||||
# a known definition we have no Function.argument list required
|
||||
# for declaration check.
|
||||
checkDefinitionArgumentsViolations(func, startCall, endCall)
|
||||
|
||||
# Check arguments in pointer declarations
|
||||
for var in data.variables:
|
||||
if not var.isPointer:
|
||||
|
|
|
@ -319,6 +319,11 @@ static int misra_8_2_k ( //
|
|||
void);
|
||||
static int misra_8_2_l ( // 8.2
|
||||
);
|
||||
void misra_8_2_m(uint8_t * const x);
|
||||
void misra_8_2_m(uint8_t * const x)
|
||||
{
|
||||
(void)x;
|
||||
}
|
||||
int16_t ( *misra_8_2_p_a ) (); // 8.2
|
||||
int16_t ( *misra_8_2_p_b ) (void);
|
||||
int16_t ( *misra_8_2_p_c ) (int);
|
||||
|
@ -732,8 +737,8 @@ extern uint32_t misra_12_3_fn7(const uint32_t * const, const uint8_t); // 8.2
|
|||
#define MISRA_12_3_FN3_2(A, B) (misra_12_3_fn3(A, \
|
||||
B))
|
||||
#define MISRA_12_3_FN3_2_MSG(x) x, fflush(stderr)
|
||||
void misra_12_3(int, int, int); // no warning
|
||||
void misra_12_3(int a, int b, int c) { // 8.2
|
||||
void misra_12_3(int, int, int); // 8.2
|
||||
void misra_12_3(int a, int b, int c) {
|
||||
int a1, a2; // 12.3
|
||||
int a3; int a4; // no warning
|
||||
int a5 = 9, a6; // 12.3
|
||||
|
|
|
@ -3679,7 +3679,10 @@ void SymbolDatabase::printXml(std::ostream &out) const
|
|||
if (!scope->functionList.empty()) {
|
||||
out << " <functionList>" << std::endl;
|
||||
for (std::list<Function>::const_iterator function = scope->functionList.begin(); function != scope->functionList.end(); ++function) {
|
||||
out << " <function id=\"" << &*function << "\" tokenDef=\"" << function->tokenDef << "\" name=\"" << ErrorLogger::toxml(function->name()) << '\"';
|
||||
out << " <function id=\"" << &*function
|
||||
<< "\" token=\"" << function->token
|
||||
<< "\" tokenDef=\"" << function->tokenDef
|
||||
<< "\" name=\"" << ErrorLogger::toxml(function->name()) << '\"';
|
||||
out << " type=\"" << (function->type == Function::eConstructor? "Constructor" :
|
||||
function->type == Function::eCopyConstructor ? "CopyConstructor" :
|
||||
function->type == Function::eMoveConstructor ? "MoveConstructor" :
|
||||
|
|
Loading…
Reference in New Issue