[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A couple of drive-by comments.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:22-33
+constexpr llvm::StringLiteral FuncExprName = "entire-called-function-expr";
+constexpr llvm::StringLiteral CastExprName = "cast-expr";
+constexpr llvm::StringLiteral UnknownDestName = "destination-length-is-unknown";
+constexpr llvm::StringLiteral DestArrayTyName = "destination-is-array-type";
+constexpr llvm::StringLiteral DestVarDeclName = "destination-variable-decl";
+constexpr llvm::StringLiteral SrcVarDeclName = "source-variable-declaration";
+constexpr llvm::StringLiteral DestMallocExprName = "destination-malloc-expr";
----------------
Binding names are copied/read a lot (for memoization purposes, see BoundNodesMap) during matching. It makes sense to make them shorter. Ideally they should fit into the inline std::string buffer (15 characters in libc++ on x86-64, https://gcc.godbolt.org/z/oNBghM).


================
Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:276-277
+
+  if (DestCapacityExprStr != "" && DestCapacityExprStr == LengthExprStr)
+    return true;
+
----------------
This conditional statement never affects the result of the function. Either there's a typo somewhere or this statement can be removed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D45050