[PATCH] D70411: [analyzer] CERT: STR31-C

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""""""""""""""""""""""""""
----------------
There are already more checkers that can check for CERT related problems but not specially made for these. These checkers do not reside in this new `cert` group. And generally a checker does not check for specifically a CERT rule, instead for more of them or other things too, or more checkers can detect a single rule. (And the user can think that only these CERT rules are checkable that exist in this package, that is not true.) So I do not like the introduction of this new `cert` package. (The documentation of existing checkers lists if the checker is designed for a CERT rule.)


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h:22
+extern const char *const CXXObjectLifecycle;
+extern const char *const SecurityError;
+} // namespace categories
----------------
Are there already not other checkers that find security related bugs (the taint checker?)? Why do these not use a `SecurityError`? It is not bad to have a `SecurityError` but maybe there is a reason why was it not there already. If these categories are exclusive it is hard to find out what problem (probably already existing bug type in other checkers) belongs to what category (it can be for this checker `UnixAPI` or `MemoryError` too?). 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70411/new/

https://reviews.llvm.org/D70411