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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to