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 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( "readability-static-definition-in-anonymous-namespace"); + CheckFactories.registerCheck( + "readability-strlen-argument"); CheckFactories.registerCheck( "readability-named-parameter"); CheckFactories.registerCheck( 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("B"); + const auto *CE = Result.Nodes.getNodeAs("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: 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 + `_ 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 `_ and other safety 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/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: 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; +}