balazske added inline comments.
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803
+
+} // end "cert.str"
+
----------------
NoQ wrote:
> `alpha.cert.str`.
This text may be hard to understand. The option means "if it is off, the
problem report happens only if there is no hand-written null termination of the
string"? I looked at the CERT rule but did not find out why is null termination
by hand important here. (I did not look at the code to find out what it does.)
And "WarnOnCall" suggests that there is warning made on calls if on, or not at
all or at other location if off, is this correct?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/AllocationState.h:28
+/// \returns The MallocBugVisitor.
+std::unique_ptr<BugReporterVisitor> getMallocBRVisitor(SymbolRef Sym);
----------------
This documentation could be improved or left out.
================
Comment at: clang/test/Analysis/cert/str31-c-notes-warn-on-call-off.cpp:68
+
+ do_something(dest);
+ // expected-warning@-1 {{'dest' is not null-terminated}}
----------------
Maybe I have wrong expectations about what this checker does but did understand
it correctly yet. The main problem here is that `dest` may be not large enough
(if size of `src` is not known). This would be a better text for the error
message? And if this happens (the write outside) it is already a fatal error,
can cause crash. If no buffer overflow happens the terminating zero is copied
from `src`, so why would `dst` be not null terminated?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70411/new/
https://reviews.llvm.org/D70411
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits