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?
Repository:
rL LLVM
https://reviews.llvm.org/D32346
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
