Skip to content

Conversation

@ivmai
Copy link
Contributor

@ivmai ivmai commented Feb 1, 2018

This is a fix of commit 97ffec8 (of pull request #1026)

Tobias Burnus tobias.burnus@physik.fu-berlin.de reported me this issue:

To my problem: We use cppcheck with
--error-exitcode=1
--suppress=unusedFunction:xptiles.cc:805

Since the following patch that fails because cppcheck returns
the exit code 1 (as an unused function is found) but the patch
does not honor that it can be suppressed.
Namely, the test in lib/checkunusedfunctions.cpp's CheckUnusedFunctions::check
finds an unused function - and, hence, uses:
unusedFunctionError(errorLogger, filename, func.lineNumber, it->first);
errors = true;

This commit fixes the issue.

@ivmai
Copy link
Contributor Author

ivmai commented Feb 3, 2018

The default implementation of hasErrors() is needed at least for tests.

CWE788, false);
errorLogger.reportErr(errmsg);
errors = true;
if (errorLogger.hasErrors())
Copy link
Owner

Choose a reason for hiding this comment

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

I would like that we put this logic in lib/cppcheck.cpp instead. It's clumpsy that I have to remember this every time I add whole program analysis.

bool CppCheck::analyseWholeProgram()
{
    .....
    return errors && errorLogger.hasErrors();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with your concern.
But I don't have a clear vision how to put the logic in cppcheck,cpp.

Copy link
Owner

Choose a reason for hiding this comment

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

hmm.. doesnt it help to modify the existing CppCheck::analyseWholeProgram() as I suggested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, got it. Done, please review.

@ivmai ivmai force-pushed the zero-exitcode-if-suppressed-unused-func branch from bd46016 to 688906a Compare February 5, 2018 17:48
@danmar
Copy link
Owner

danmar commented Feb 6, 2018

hmm.. that was not how I envisioned that it would work. I will merge this and try to change it..

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