Szelethus accepted this revision.
Szelethus added a comment.
This is a very neat checker, the source code reads so easily, we might as well
put it as the official CERT rule description.
I think adding the non-compliant and compliant code examples would be nice. I
also wrote some inline comments, but I'm fine with leaving them for a later
patch. LGTM, granted that @NoQ and @Charusso are happy.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h:16-22
+extern const char *const CoreFoundationObjectiveC;
+extern const char *const LogicError;
+extern const char *const MemoryRefCount;
+extern const char *const MemoryError;
+extern const char *const UnixAPI;
+extern const char *const CXXObjectLifecycle;
+extern const char *const SecurityError;
----------------
We could turns these into `llvm::StringLiteral`s one day.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/AllocationState.h:28-30
+/// \returns The MallocBugVisitor.
+std::unique_ptr<BugReporterVisitor> getMallocBRVisitor(SymbolRef Sym);
+
----------------
Is this deadcode now?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3383-3386
+std::unique_ptr<BugReporterVisitor> getMallocBRVisitor(SymbolRef Sym) {
+ return std::make_unique<MallocBugVisitor>(Sym);
+}
+
----------------
And this?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:50
+ StringRef ErrorMsg =
+ "'putenv' function should not be called with auto variables";
+ ExplodedNode *N = C.generateErrorNode();
----------------
How about `The 'putenv' function should not be called with arguments that have
automatic storage`. But I guess we could leave wordsmithing for later.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:55
+ // Track the argument.
+ if (isa<StackSpaceRegion>(MSR)) {
+ bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr,
*Report);
----------------
This should always be true, since we bail out a couple lines up if it isn't,
right?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71433/new/
https://reviews.llvm.org/D71433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
