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:
> 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.
================
Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20
+ char *p = new char[(strlen(s) - 1)]
+ strcpy(p, s);
+
----------------
JonasToth wrote:
> isnt that an overflow?
> an example:
> `strlen(s) == 10` -> `p` will be 9 characters long, since its substracted
> with `1`.
>
> the copy operation will then copy the content of `s` into `p`, therefore
> copying 10 characters into a buffer of length 9.
>
> as i understand it `strcpy(p, s + 1)` would be correct with the sizes.
yes it is overflow. My intention was to show that strlen(s+1) syntax is
dangerous.
================
Comment at: test/clang-tidy/readability-strlen-argument.cpp:11
+ X = strlen(Str + 10);
+ // CHECK-FIXES: {{^}} X = (strlen(Str) - 10);{{$}}
+
----------------
JonasToth wrote:
> 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` :)
I think that in theory, you have a point. It would be best that such users
don't use this check. I doubt that is a big problem in practice. We don't need
to speculate much, I will test this on all debian source code..
It's possible that strings in program executables are stored like that, but I'd
say it's ub to calculate the address Str+10 and then dereference that if Str is
a string that is 10 bytes long.
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
