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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
