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