[PATCH] D32346: [clang-tidy] New readability check for strlen argument

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 22 06:41:14 PDT 2017

alexfh added inline comments.

Comment at: clang-tidy/readability/StrlenArgumentCheck.cpp:23-24
+  Finder->addMatcher(
+      callExpr(anyOf(callee(functionDecl(hasName("::strlen"))),
+                     callee(functionDecl(hasName("std::strlen")))),
+               hasAnyArgument(ignoringParenImpCasts(
`anyOf(callee(functionDecl(hasName(x))), callee(functionDecl(hasName(y))))` unnecessarily duplicates `callee` and `functionDecl`. A better option is `callee(functionDecl(hasAnyName(x, y)))`.

Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8
+In the example code below the developer probably wanted to make room for an extra char in the allocation but misplaced the addition.
JonasToth wrote:
> danielmarjamaki wrote:
> > JonasToth wrote:
> > > when the intend was to allocate one more char, he would need to do `strlen(s) + 1`, why is it changed to subtraction then?
> > If I change it to strlen(s) + 1 then the logic of the program is changed.
> > 
> > If I change it to subtraction then the logic is the same and the program is still wrong, but imho it is easier to see.
> it should be made clear in the docs, that the code is bad, especially if its UB. the dangers of copy & paste ;) (even its unlikely that this will be copy&pasted).
If we have good reasons to think the original code is wrong, so it seems that it's better to fix it, than to retain the behavior and just hide the bug?



More information about the cfe-commits mailing list