martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Looks good.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) 
{
+    C.addSink();
     return;
----------------
Szelethus wrote:
> 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.
Okay, makes sense, the new name fits this.


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

Reply via email to