danielmarjamaki requested changes to this revision.
danielmarjamaki added inline comments.
This revision now requires changes to proceed.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:788
+SVal MallocChecker::SValBinMulOp(CheckerContext &C, const Expr *Blocks,
+ const Expr *BlockBytes, ProgramStateRef
State) {
----------------
I have the feeling this should be renamed. Since its purpose is to calculate
the total size maybe MallocChecker::calculateBufferSize()
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:790
+ const Expr *BlockBytes, ProgramStateRef
State) {
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ SVal nBlocks = C.getSVal(Blocks);
----------------
Please begin variables with Capitals. svalBuilder,nBlocks,nBlockBytes
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:911
+ if (FunI == II_g_malloc0_n || FunI == II_g_try_malloc0_n) {
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ Init = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
----------------
Capital. svalBuilder
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2040
const Expr *arg0Expr = CE->getArg(0);
const LocationContext *LCtx = C.getLocationContext();
----------------
Capital variable: arg0Expr, arg0Val
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2054
const Expr *Arg1 = CE->getArg(1);
if (!Arg1)
return nullptr;
----------------
is this "if (!Arg1)" needed? It seems to me that if CE->getNumArgs() returns >=
2 then CE->getArg(1) should be non-NULL. Does it crash/assert if you remove
this condition?
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2061
+ const Expr *Arg2 = CE->getArg(2);
+ if (!Arg2)
+ return nullptr;
----------------
hmm.. seems weird if CE->getArg(2) returns nullptr even though CE->getNumArgs()
>= 3 .. is it possible to remove this check or does that cause some crash etc?
> Generally, i'd prefer the code for computing the buffer size to be structured
> as
I would also prefer such code.
Repository:
rL LLVM
https://reviews.llvm.org/D30771
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
