Charusso updated this revision to Diff 254420.
Charusso marked 4 inline comments as done.
Charusso added a comment.
- Simplify tests.
- Remove dead code, they are far away to being used.
- Add an extra test case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70411/new/
Charusso updated this revision to Diff 254423.
Charusso added a comment.
- Remove the last dead comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70411/new/
https://reviews.llvm.org/D70411
Files:
clang/docs/analyzer/checkers.rst
Charusso added a comment.
Thanks for the review, hopefully if I ping @NoQ in every round, it will be
green-marked soon.
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55
+ // they can cause a not null-terminated string. In this case we store the
+ //
balazske added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55
+ // they can cause a not null-terminated string. In this case we store the
+ // string is being not null-terminated in the 'NullTerminationMap'.
+ //
The
Charusso added a comment.
Given that the secondary behavior confuse people I have removed it for now. May
if someone introduce a NullTerminationChecker then we introduce such option to
warn on insecure calls instant. Thanks @balazske for influencing that change.
@NoQ this project had a
Charusso updated this revision to Diff 254061.
Charusso marked 6 inline comments as done.
Charusso edited the summary of this revision.
Charusso added a comment.
- Get rid of the secondary behavior for now.
- Fix review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70411/new/
Charusso marked 6 inline comments as done.
Charusso added a comment.
"To prevent such errors, either limit copies through truncation or, preferably,
ensure that the destination is of sufficient size to hold the character data" -
from the rule's page.
Most of the projects are fine truncating by
balazske added inline comments.
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""
Charusso wrote:
> Szelethus wrote:
> > balazske wrote:
> > > There are already more checkers that can check for CERT
Charusso marked 4 inline comments as done.
Charusso added inline comments.
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803
+
+} // end "cert.str"
+
balazske wrote:
> NoQ wrote:
> > `alpha.cert.str`.
> This text may be hard to understand.
balazske added inline comments.
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803
+
+} // end "cert.str"
+
NoQ wrote:
> `alpha.cert.str`.
This text may be hard to understand. The option means "if it is off, the
problem report happens only
Charusso marked 5 inline comments as done.
Charusso added a comment.
Thanks for the feedback! Given that it will remain an `alpha` checker for a
long time (~1 year), no one really should use it.
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
Szelethus added inline comments.
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""
balazske wrote:
> There are already more checkers that can check for CERT related problems but
> not specially made
balazske added inline comments.
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""
There are already more checkers that can check for CERT related problems but
not specially made for these. These
Charusso updated this revision to Diff 241752.
Charusso edited the summary of this revision.
Charusso added a comment.
- 2020-ify the checker writing
- Remove extra bullet-points of CERT checker documentation: we only need the
checker's documentation, not the packages.
CHANGES SINCE LAST
Charusso updated this revision to Diff 233903.
Charusso retitled this revision from "[analyzer] CERT: StrChecker: Implementing
most of the STR31-C" to "[analyzer] CERT: STR31-C".
Charusso added a comment.
- Iterate over parameters rather arguments for searching string-reading.
- Better notes of
15 matches
Mail list logo
