NoQ added a comment. I have a few immediate style issues but otherwise it looks good :)
================ Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:237 -const char GenericTaintChecker::MsgUncontrolledFormatString[] = +const llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString = "Untrusted data is used as a format string " ---------------- `StringLiteral`s need to always be declared as `constexpr`. ================ Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:251 +const llvm::StringLiteral GenericTaintChecker::MsgCustomSink = + "Untrusted data is passed to a user defined sink"; +; ---------------- It should be "user-defined" because it's a single adjective. I recommend against using the word "sink" in user-facing messages because it's too jargony. Do we have a better word for this? ================ Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:252 + "Untrusted data is passed to a user defined sink"; +; } // end of anonymous namespace ---------------- This semicolon looks unnecessary. ================ Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:595-596 + StringRef Name = C.getCalleeName(C.getCalleeDecl(CE)); + llvm::errs() << "Skip out of bound SrcArg: " << Name << ":" << ArgNum + << '\n'; + continue; ---------------- These debug prints should not land. ================ Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:840 + + const GenericTaintChecker::ArgVector &Args = It->getValue(); + for (unsigned ArgNum : Args) { ---------------- The `GenericTaintChecker::` qualifier is unnecessary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59637/new/ https://reviews.llvm.org/D59637 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits