danielmarjamaki abandoned this revision.
danielmarjamaki added a comment.
I will not continue working on this checker
Repository:
rL LLVM
https://reviews.llvm.org/D32346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20
+char *p = new char[(strlen(s) - 1)]
+strcpy(p, s);
+
JonasToth w
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",
+ hasAnyArgume
JonasToth added inline comments.
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.
+
danielmarjamaki wrote:
danielmarjamaki added inline comments.
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:
JonasToth added a comment.
> #include
> void dostuff(const char *P) {
>
> if (strncmp(P+2,"xyz",3)==0) {}
>
> }
>
>
Iam not super familiar with C, but the intend of the function is to check the
following:
P = "foxyz", P2 = "abxyz", P3 = "opxyz", ...
And if P matches this kind of st
danielmarjamaki added a comment.
I am thinking about making my check more strict so it only warns in
allocations. I believe the example code is much more motivating when there is
allocation.
In https://reviews.llvm.org/D32346#733430, @JonasToth wrote:
> My thoughts on the check added.
> Have
JonasToth added inline comments.
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.
+
when the intend was to
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/Rea
JonasToth added inline comments.
Comment at: test/clang-tidy/readability-strlen-argument.cpp:1
+// RUN: %check_clang_tidy %s readability-strlen-argument %t
+
danielmarjamaki wrote:
> JonasToth wrote:
> > Same as documentation, maybe a little more telling examples
sberg added inline comments.
Comment at: test/clang-tidy/readability-strlen-argument.cpp:11
+ X = strlen(Str + 10);
+ // CHECK-FIXES: {{^}} X = (strlen(Str) - 10);{{$}}
+
but if any of the first ten chars in Str can be NUL, then the change is bogus?
(I guess
Eugene.Zelenko added a comment.
> In https://reviews.llvm.org/D32346#733906, @Eugene.Zelenko wrote:
>
>> It may be good idea to add check for arguments which taken from C++
>> containers like std::string, std::string_view, etc.
>
>
> Not sure how you want. Do you envision something like:
>
>
danielmarjamaki added a comment.
Thanks for all comments. I am working on fixing them. Updated patch will be
uploaded soon.
> Have you run it over a big codebase? What is the turnout?
I did not see this warning yet. Maybe after fixing the comments (::strlen)
there will be a difference.
In htt
Eugene.Zelenko added a comment.
It may be good idea to add check for arguments which taken from C++ containers
like std::string, std::string_view, etc.
Repository:
rL LLVM
https://reviews.llvm.org/D32346
___
cfe-commits mailing list
cfe-commits@
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:6
+
+This checker will detect addition in strlen() argument. Example code:
+
JonasToth
JonasToth added a comment.
My thoughts on the check added.
Have you run it over a big codebase? What is the turnout?
Comment at: clang-tidy/readability/StrlenArgumentCheck.cpp:23
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("strlen"))),
+ has
danielmarjamaki created this revision.
Herald added a subscriber: mgorny.
I propose this new readability checker.
Repository:
rL LLVM
https://reviews.llvm.org/D32346
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/Str
17 matches
Mail list logo
