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

[Jonas Toth via Phabricator via cfe-commits]

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 to test on.
> > What happens with `char**`, an array of strings? Accessing those one by one would be possible with an addition or subscriptoperation.
> how do you mean with char**. If you access those one by one it will look something like this right?
> 
>     char **A;
>     strlen(*(A+N));
> 
> such code is not matched as far as I see.
Iam not sure if I understood the goal of the check.
For me, it looks like the determination of the length of a substring, say we skip the first 10 characters, who long would the end still be.

your example is correctly what i thought of and i think as well it would not be matched .

```strlen(A+N)``` would not compile? The types do not match, but could there weird things happen?


================
Comment at: test/clang-tidy/readability-strlen-argument.cpp:11
+  X = strlen(Str + 10);
+  // CHECK-FIXES: {{^}}  X = (strlen(Str) - 10);{{$}}
+
----------------
sberg wrote:
> but if any of the first ten chars in Str can be NUL, then the change is bogus?  (I guess I fail to see the reason for this check)
intersting point. If the Array holds a long list of string concatenated and seperated by `0`, the offsetting would be a valid usecase.
(are strings in programm executables stored like this?)

But i have no idea if that is actually a scenario. usually i use `std::string` :)


Repository:
  rL LLVM

https://reviews.llvm.org/D32346