baloghadamsoftware added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:378-379
+
+ using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr
*CE,
+ ProgramStateRef State) const;
+
----------------
Szelethus wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > Whenever i see a (`CE`, `State`) pair, it screams `CallEvent` to me. That
> > > said, i'm worried that `State` in these callbacks isn't necessarily equal
> > > to `C.getState()` (the latter, by the way, is always equal to the
> > > `CallEvent`'s `.getState()` - that's a relief, right?), so if you'll ever
> > > be in the mood to check that, that'd be great :)
> > It should be always equal to it. I'll change it.
> Hmmm, I tried making this take a (`CallEvent`, `CheckerContext`), but it
> bloated the code for little gain, since every handler function would start
> with the retrieval of the state and the call expression. That said, I could
> cascade these down to `FreeMemAux`, `MallocMemAux`, etc, to also take this
> pair instead, but I'll be honest, I don't see point until something actually
> breaks.
This is the standard way in the checkers: almost every handler function starts
with the retrieval of the state from the `CheckerContext`. The only reason for
an extra `State` parameter is that sometimes we create more states in the lower
level functions but only add the final one to the `CheckerContext` as a new
transition. Does something like this happen here?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:865
+ if (FreeingMemFnMap.lookup(Call))
return true;
----------------
Maybe `return !!FreeingMemFnMap.lookup(Call)`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:879
+bool MallocChecker::isMemCall(const CallEvent &Call) const {
+ if (FreeingMemFnMap.lookup(Call) || NonFreeingMemFnMap.lookup(Call))
return true;
----------------
Same here: why not just `return FreeingMemFnMap.lookup(Call) ||
NonFreeingMemFn.lookup(Call)`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68165/new/
https://reviews.llvm.org/D68165
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
