Szelethus added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:195
+ // Say this 3 times fast.
+ State = State ? State : getState();
+ addTransition(State, generateSink(State, getPredecessor()));
----------------
martong wrote:
> balazske wrote:
> > ```
> > if (!State)
> > State = getState();
> > ```
> > is better? (I put the same comment into some (other?) patch already but
> > maybe it disappeared?)
> +1 for balazske's suggestion.
Landed this part of the change in D77866.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024
+ if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker])
{
+ C.addSink();
return;
----------------
martong wrote:
> This seems to be inverse logic to me.
> I'd expect that in a function called `Report...` we do stuff that is related
> to reporting only. That is why I think it would be better to have the
> condition and addSink before calling `Report...`. That way reporting and
> modeling would be even more separated.
Very good point, but I think the actual problem lies in the name of the method.
`ReportBadFree` in particular is called from multiple places, and I like how
this is the function that takes over once we find a bug, because a bad free
(which in this context means the deallocation of a non-heap allocated object)
can never be a non-fatal error, so it make sense that the sink is solved here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77474/new/
https://reviews.llvm.org/D77474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
