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