danielmarjamaki updated this revision to Diff 96526.
danielmarjamaki added a comment.

Fixed review comments. Made code examples and documentation more motivational.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/StrlenArgumentCheck.cpp
  clang-tidy/readability/StrlenArgumentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-strlen-argument.rst
  test/clang-tidy/readability-strlen-argument.cpp

Index: test/clang-tidy/readability-strlen-argument.cpp
===================================================================
--- test/clang-tidy/readability-strlen-argument.cpp
+++ test/clang-tidy/readability-strlen-argument.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s readability-strlen-argument %t
+
+namespace std {
+typedef __typeof(sizeof(int)) size_t;
+size_t strlen(const char *s);
+}; // namespace std
+using namespace std;
+
+void warn1(const char *Str) {
+  char *P;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:27: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[strlen(Str + 1)];
+  // CHECK-FIXES: {{^}}  P = new char[(strlen(Str) - 1)];{{$}}
+  delete[] P;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[std::strlen(1 + Str)];
+  delete[] P;
+}
+
+struct S {
+  char *P;
+};
+void warn2(const struct S &Par) {
+  char *P;
+  // CHECK-MESSAGES: :[[@LINE+1]]:34: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[std::strlen(Par.P + 1)];
+  // CHECK-FIXES: {{^}}  P = new char[(std::strlen(Par.P) - 1)];{{$}}
+  delete[] P;
+}
Index: docs/clang-tidy/checks/readability-strlen-argument.rst
===================================================================
--- docs/clang-tidy/checks/readability-strlen-argument.rst
+++ docs/clang-tidy/checks/readability-strlen-argument.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - readability-strlen-argument
+
+readability-strlen-argument
+===========================
+
+This check will detect addition in `strlen()` argument.
+
+In the example code below the developer probably wanted to make room for an extra char in the allocation but misplaced the addition.
+
+.. code-block:: c++
+
+    char *p = new char[strlen(s + 1)];
+    strcpy(p, s);
+
+With -fix that becomes:
+
+.. code-block:: c++
+
+    char *p = new char[(strlen(s) - 1)]
+    strcpy(p, s);
+
+For readability it is considered better to subtract the result outside the `strlen()`.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -169,4 +169,5 @@
    readability-redundant-string-init
    readability-simplify-boolean-expr
    readability-static-definition-in-anonymous-namespace
+   readability-strlen-argument
    readability-uniqueptr-delete-release
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -76,6 +76,12 @@
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
 
+- New `readability-strlen-argument
+  <http://clang.llvm.org/extra/clang-tidy/checks/readability-strlen-argument.html>`_ check
+
+  Finds misleading `strlen()` argument. If addition is used in the argument then the effect is that
+  the strlen() result is subtracted. 
+
 - New `hicpp` module
 
   Adds checks that implement the `High Integrity C++ Coding Standard <http://www.codingstandard.com/section/index/>`_ and other safety
Index: clang-tidy/readability/StrlenArgumentCheck.h
===================================================================
--- clang-tidy/readability/StrlenArgumentCheck.h
+++ clang-tidy/readability/StrlenArgumentCheck.h
@@ -0,0 +1,35 @@
+//===--- StrlenArgumentCheck.h - clang-tidy----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLEN_ARGUMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLEN_ARGUMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detect pointer addition in strlen() argument.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-strlen-argument.html
+class StrlenArgumentCheck : public ClangTidyCheck {
+public:
+  StrlenArgumentCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLEN_ARGUMENT_H
Index: clang-tidy/readability/StrlenArgumentCheck.cpp
===================================================================
--- clang-tidy/readability/StrlenArgumentCheck.cpp
+++ clang-tidy/readability/StrlenArgumentCheck.cpp
@@ -0,0 +1,49 @@
+//===--- StrlenArgumentCheck.cpp - clang-tidy------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StrlenArgumentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void StrlenArgumentCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(anyOf(callee(functionDecl(hasName("::strlen"))),
+                     callee(functionDecl(hasName("std::strlen")))),
+               hasAnyArgument(ignoringParenImpCasts(
+                   binaryOperator(hasOperatorName("+")).bind("B"))))
+          .bind("CE"),
+      this);
+}
+
+void StrlenArgumentCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *B = Result.Nodes.getNodeAs<BinaryOperator>("B");
+  const auto *CE = Result.Nodes.getNodeAs<CallExpr>("CE");
+  auto D =
+      diag(B->getExprLoc(), "strlen() argument has pointer addition, it is "
+                            "recommended to subtract the result instead.");
+  if (!B->getRHS()->getType()->isAnyPointerType()) {
+    SourceLocation LHSEndLoc = Lexer::getLocForEndOfToken(
+        B->getLHS()->getLocEnd(), 0, *Result.SourceManager, getLangOpts());
+    D << FixItHint::CreateInsertion(LHSEndLoc, ")")
+      << FixItHint::CreateInsertion(CE->getLocStart(), "(")
+      << FixItHint::CreateReplacement(
+             SourceRange(B->getExprLoc(), B->getExprLoc()), "-");
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -33,6 +33,7 @@
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
+#include "StrlenArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
@@ -72,6 +73,8 @@
         "readability-redundant-member-init");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
+    CheckFactories.registerCheck<StrlenArgumentCheck>(
+        "readability-strlen-argument");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<NonConstParameterCheck>(
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -26,6 +26,7 @@
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
+  StrlenArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to