Skip to content

Show struct member in unsignedLessThanZeroError warning#830

Merged
danmar merged 1 commit into
cppcheck-opensource:masterfrom
JIghtuse:master
Sep 22, 2016
Merged

Show struct member in unsignedLessThanZeroError warning#830
danmar merged 1 commit into
cppcheck-opensource:masterfrom
JIghtuse:master

Conversation

@JIghtuse
Copy link
Copy Markdown
Contributor

Before:
[/tmp/test.c:8]: (style) Checking if unsigned variable '.' is less than zero.
[/tmp/test.c:12]: (style) Checking if unsigned variable '.' is less than zero.

After:
[/tmp/test.c:8]: (style) Checking if unsigned variable 'd.n' is less than zero.
[/tmp/test.c:12]: (style) Checking if unsigned variable 'd.n' is less than zero.

@JIghtuse
Copy link
Copy Markdown
Contributor Author

Should not break anything, I hope. I split Token::expressionString on smaller functions and reused them for making operand1/operand2 strings.

Comment thread lib/checkother.cpp Outdated
pointerLessThanZeroError(tok, inconclusive);
if (vt && vt->sign == ValueType::UNSIGNED)
unsignedLessThanZeroError(tok, tok->astOperand1()->str(), inconclusive);
unsignedLessThanZeroError(tok, tok->operand1String(), inconclusive);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not tok->astOperand1()->expressionString()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, didn't thought to try it. Looks like it works. Will fix it.

Comment thread lib/token.cpp
return false; // <- guess
}

std::string Token::expressionString() const
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitting this function is a good idea. Thanks.

Comment thread lib/token.cpp Outdated
}

std::string Token::expressionString() const
const Token* goToLeftParenthesis(const Token* start, const Token* end)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be static

Comment thread test/testother.cpp
" return;\n"
" }\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:8]: (style) Checking if unsigned variable 'd.n' is less than zero.\n"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are parentheses around the expression I believe it is ok to remove them:

if ((d.n+1) < 0) {}

[test.c:8]: (style) Checking if unsigned variable 'd.n+1' is less than zero.

Isn't it?

As far as I see the operand1String will output:

[test.c:8]: (style) Checking if unsigned variable '(d.n+1)' is less than zero.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow after adding parentheses Cppcheck can't find this issue anymore. Is it a bug?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was looking at another test file.
Looks like using tok->astOperand1()->expressionString() fixed this, there are no parentheses in warning.

@JIghtuse
Copy link
Copy Markdown
Contributor Author

@danmar I've completed changes you requested. Is it good now?
I have a feeling functions goTo can be named better, but I don't know how.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Sep 21, 2016

yes it looks good now. as far as I see you should remove the changes in lib/token.h. when that has been done this can be applied. I also think there could be better names but I have no suggestion and these will work for now.

Before:
    [/tmp/test.c:8]: (style) Checking if unsigned variable '.' is less than zero.
    [/tmp/test.c:12]: (style) Checking if unsigned variable '.' is less than zero.

After:
    [/tmp/test.c:8]: (style) Checking if unsigned variable 'd.n' is less than zero.
    [/tmp/test.c:12]: (style) Checking if unsigned variable 'd.n' is less than zero.
@JIghtuse
Copy link
Copy Markdown
Contributor Author

Oh, missed these header changes. Done.

@danmar danmar merged commit a3a392a into cppcheck-opensource:master Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants