Fix #14367: Support polyspace inline suppressions for misra c:20xx rules #8105
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Should this be behind a flag? Always parsing for something very niche (and premium only?) seems undesired. |
I think that would be good. |
ludviggunne
force-pushed
the
14367
branch
2 times, most recently
from
565b38a to
d27f25b
Compare
Since it is specific to certain MISRA rules it should only be active if those rules are active. |
ludviggunne
force-pushed
the
14367
branch
2 times, most recently
from
73a5159 to
da0456b
Compare
I agree it's niche use case. Especially for open source projects. imho.. if If misra checkers are not enabled then it's redundant to handle the polyspace comments. |
|
| if (s.isPolyspace) { | ||
| errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched polyspace suppression: " + s.errorId, "unmatchedPolyspaceSuppression", Certainty::normal)); | ||
| } else { | ||
| errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal)); | ||
| } |
There was a problem hiding this comment.
hmm.. we can make sure that only the id is different;
| if (s.isPolyspace) { | |
| errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched polyspace suppression: " + s.errorId, "unmatchedPolyspaceSuppression", Certainty::normal)); | |
| } else { | |
| errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal)); | |
| } | |
| const std::string unmatchSuppressionId = s.isPolyspace ? "unmatchedPolyspaceSuppression" : "unmatchedSuppression"; | |
| errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, unmatchedSuppression, Certainty::normal)); |
| return pos == args.size() - arg.size() || args[pos + arg.size()] == ' '; | ||
| }; | ||
|
|
||
| if (settings.addons.count("misra") != 0) { |
There was a problem hiding this comment.
I believe this is false if --addon=misra.json is used. I think it's more reliable to check if Settings::addonInfo contains an misra entry.
|
|
||
| void polyspace::Parser::parse(const std::string &comment, int line, const std::string &filename) | ||
| { | ||
| mComment = comment; |
| if (mComment.compare(0, 2, "/*") == 0 || mComment.compare(0, 2, "//") == 0) | ||
| mComment = mComment.substr(2); | ||
|
|
||
| std::string::size_type pos = 0; |
There was a problem hiding this comment.
mComment can be empty now can't it?
| return mPeeked; | ||
| } | ||
|
|
||
| if (mComment.compare(0, 2, "/*") == 0 || mComment.compare(0, 2, "//") == 0) |
There was a problem hiding this comment.
this looks suspicious. do you expect "//" inside a comment also? not just at the start.
should // inside a comment be silently skipped.
| Settings settings; | ||
| settings.addons.emplace("misra"); | ||
| polyspace::Parser parser(settings); | ||
| parser.parse("/* polyspace MISRA2012 : 2.7 */", 1, "file.c"); |
There was a problem hiding this comment.
it's unfortunate to duplicate so much code for each test. It makes it hard to see what you test.
There was a problem hiding this comment.
which are the possible formats that you handle? I have seen this:
/* polyspace +1 MISRA-C3:20.1 [Justified:Low] "testing"*/
Is comments that contains +1, [Justified:Low], "..." handled?
3 participants
No description provided.