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

Reply via email to