[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#911744, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39121#911741, @aaron.ballman wrote:
>
> > 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.
>
>
> Yes, you pointed it out, but even in the example you wrote a `+1` to the result as well. I a double-null terminated string in Win32 you must have at least 1-byte strings, which are 2-bytes long together with their null terminator. So the minimum offset is 2, since 1 would mean 0-byte string, but then we have two null terminators after each other which is impossible since double null is the terminator of the whole list. So `s+1` cannot be valid. Furthermore, even if it would be valid, we must also allocate memory for the zero terminator of the string list item at the given offest, so we have an extra +1 outside of `strlen` as well.


Hmm, this is a good point -- I was thinking of the generic +N case with the original example, but with an explicit +1, you can't run into that situation with Win32 APIs. I will think on this a bit further and report back when I have a spare moment.


https://reviews.llvm.org/D39121