fix OsStr conversion for non-utf8 strings on windows

[bschoenmaeckers]

While testing OsStr conversions with ill-formed UTF-16 strings on windows, I found out that the current implementation is wrong.

When wstr is NULL, instead return the size that would be required to store all of unicode including a terminating null.

Note that the resulting wchar_t* string may or may not be null-terminated. It is the responsibility of the caller to make sure that the wchar_t* string is null-terminated in case this is required by the application. Also, note that the wchar_t* string might contain null characters, which would cause the string to be truncated when used with most C functions.

PyUnicode_AsWideChar returns the size including the null byte, but does not copy the null byte. This fails the assert in this function. Furthermore we want the byte slice without the null byte.

ref #5368

[davidhewitt]

I'm trying to assess if this is better to class as a "fix" or a breaking change. At the moment I guess the result is that we always include a trailing NUL on the extracted string, only on Windows?

So this is a good consistency fix, to match the other platforms, but it also might cause issues in the worst case? 🤔

[bschoenmaeckers]

The current case is that it can only succeed when the string is valid utf8. Because it will take the shortcut using utf8 conversion. When the string is WTF16 it will continue and use PyUnicode_AsWideChar. But this currently never succeeds because the assertion assert_eq!(bytes_read, size); always fails (bytes_read does not contain the null byte but size does). So this is not a breaking change.

[bschoenmaeckers]

I guess no one is hitting this assert because valid utf8 is the common case.

[davidhewitt]

Makes sense, thanks!