xazax.hun added a comment.

Some nits inline.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:995
 
-void MallocChecker::checkBasicAlloc(CheckerContext &C, const CallExpr *CE,
-                                    ProgramStateRef State) const {
-  State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Malloc);
-  State = ProcessZeroAllocCheck(C, CE, 0, State);
+void MallocChecker::checkBasicAlloc(const CallEvent &Call,
+                                    CheckerContext &C) const {
----------------
What is the main reason of getting rid of the state parameter? I think this 
could make using these functions more error prone overall as it would be easier 
to introduce splits in the exploded graph this way (if we somehow wanted to 
model a function as successive calls of some other functions).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1241
     Arg = CE->getArg(IndexOfSizeArg);
-  }
-  else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
-    if (NE->isArray())
+  } else if (const CXXNewExpr *NE =
+                 dyn_cast<CXXNewExpr>(Call.getOriginExpr())) {
----------------
Use `auto` here to avoid repeating the type.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1641
 ProgramStateRef MallocChecker::FreeMemAux(
-    CheckerContext &C, const Expr *ArgExpr, const Expr *ParentExpr,
     ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
----------------
In functions where we only use `ArgExpr` to retrieve the corresponding `SVal` 
we could pass the `SVal` in the first place.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75432/new/

https://reviews.llvm.org/D75432



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to