[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39121#911736, @baloghadamsoftware wrote:

> Can you give me please an example where `malloc(strlen(s+1))` is intentional. It allocates `2` byte less than needed to store `s`. If it is really the goal (e.g. `s` has a `2` character prefix which we do not want to copy) then the correct solution is to use `malloc(strlen(s+2)+1)` instead of putting `s+1` into extra parentheses. So I think that we are overcomplicating things here. I would simply delete the suggestion about the extra parentheses from the error message and leave them only in the documentation, at least until no real example comes to my mind where taking the length of `s+1` and not adding `+1` is the fully correct solution. (Please note the `malloc(strlen(s+1)+1)` is already ignored.


As I pointed out earlier in the thread, it is common to have double-null-terminated strings in Win32 APIs. This is a case where strlen(s + N) is valid. Since 1-byte strings would also be a valid value of N, strlen(s + 1) is feasible, though unlikely. If you're okay dropping the fixit from your check and rewording the diagnostic to remove the "surround with parens" bit, I think the check would be fine. However, fix-its are generally only used when we know the transformation is correct. We have no way to know that in this case.


https://reviews.llvm.org/D39121