[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1
+#include "NotNullTerminatedResultCheck.h"
+#include "clang/AST/ASTContext.h"
----------------
Missing header blurb


================
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:14
+                      const MatchFinder::MatchResult &Result) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Expr->getSourceRange()),
----------------
Doesn't `Lexer::getSourceText()` return `StringRef`?


================
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:264
+    const auto FirstArg = FuncExpr->getArg(0);
+    std::string NewSecondArg = " strlen(" + exprToStr(FirstArg, Result) + "),";
+
----------------
This should probably be `SmallString<32>`


================
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:40
+                   DiagnosticBuilder &Diag);
+  void memchrFix(StringRef Name,
+                 const ast_matchers::MatchFinder::MatchResult &Result,
----------------
Why are all these internal functions `public`?
They should be either in anonymous namespace (best), or at least have `private` visibility.


================
Comment at: docs/ReleaseNotes.rst:68
+
+- The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-incorrect-roundings.html>`_
----------------
This seems out-of-place. Why is this in the diff?


================
Comment at: test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp:11
+void bad_memcpy(char *dest, const char *src) {
+  memcpy(dest, src, strlen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
----------------
What about these functions, but in `std::` namespace?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050