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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits