george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:154
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE, bool
Append) {
if (CE->getNumArgs() != 3)
----------------
`Append` can be deduced from a call expression; I think it would be better to
drop it from the parameter list and calculate again in the function body.
================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+ // - sizeof(dst)
+ if (Append && isSizeof(LenArg, DstArg))
+ return true;
----------------
I am confused on what is happening in this branch and why is it bad. Could we
add a commend?
================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:196
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
- if ((BufferLen - DstOff) < ILRawVal)
- return true;
+ BufferLen -= DstOff;
+ if (Append) {
----------------
Better to introduce a new variable, e.g. `RemainingBufferLen`
https://reviews.llvm.org/D49722
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
