xazax.hun added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h:54
+ // initialization.
+ if (getCheckName().empty() && Checker) {
+ Check = Checker->getCheckName();
----------------
a.sidorin wrote:
> Possibly I am missing the context, but could you please explain, why do we
> modify CheckName in getName() but not in getCheckName()? It seems a bit
> unclear to me.
Good catch, it is just an oversight. I got confused by `CheckName` vs `Name`.
And since the analyzer called `getName` before `getCheckName` the tests passed.
================
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:277
+ BT_leakedvalist.reset(new BugType(
+ CheckNames[ReportUninit ? CK_Uninitialized : CK_Unterminated],
+ "Leaked va_list", categories::MemoryError));
----------------
a.sidorin wrote:
> If I understand correctly, if we report uninitialized and then unterminated,
> the second report will have wrong CheckName because it is never reset.
That is right. Unfortunately, If unterminated check is not turned on but
uninitialized is, we can end up with empty check names. I replaced this
workaround with a slightly more robust one.
================
Comment at: test/Analysis/malloc.c:1723
-char *dupstrWarn(const char *s) {
- const int len = strlen(s);
----------------
a.sidorin wrote:
> Should we enable the checker instead of removing test?
I tried that once, but this caused more new warnings (and sinks) so more
changes would be required to make the tests pass. And this case is already
tested in `string.c`.
https://reviews.llvm.org/D41538
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
