baloghadamsoftware accepted this revision. baloghadamsoftware added a comment. This revision is now accepted and ready to land.
LGTM! But wait for Artem's acceptance before submitting. ================ Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:311 + if (!Filter.CheckCStringOutOfBounds) + return StOutBound; ---------------- dkrupp wrote: > NoQ wrote: > > Could we preserve the other portion of the assertion on this branch? I.e., > > `assert(Filter.CheckCStringNullArg)`. > > > > Additionally, do you really want to continue analysis on this path? Maybe > > `return nullptr` to sink? > I was unsure whether to return nullptr or StOutBound. I thought that > alpha.unix.cstring.OutOfBounds is in alpha because it may falsely detect > buffer overflows and then we would cut the path unnecessarily. > But OK, it is safer this way. > > I could not put back the assertion, because if only unix.Malloc checker is > enabled (and CStringOutOfBounds and CStringNullArg are not) the assertion is > not true. > I think the assertion could be preserved after the early exit, but it has no point to repeat the same condition in subsequent lines. https://reviews.llvm.org/D48831 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
