Implement Unicode and Ansi StringBuilder Marshalling#3315
Implement Unicode and Ansi StringBuilder Marshalling#3315shrah merged 2 commits intodotnet:masterfrom
Conversation
|
@yizhang82 PTAL |
yizhang82
left a comment
There was a problem hiding this comment.
Please fix the [out] case by splitting marshaling and allocation. Also please make sure the length calculation is done correctly in the border cases. This is a very tricky area and we should do it correctly the first time.
| private static unsafe byte* CharArrayToAnsi(char* pStr, int stringLength) | ||
| { | ||
| // CORERT-TODO: Use same encoding as the rest of the interop | ||
| var encoding = Encoding.UTF8; |
There was a problem hiding this comment.
Sigh... We should fix this...
| // CORERT-TODO: Use same encoding as the rest of the interop | ||
| var encoding = Encoding.UTF8; | ||
| int bufferLength = encoding.GetByteCount(pStr, stringLength); | ||
| byte* buffer = (byte*)PInvokeMarshal.CoTaskMemAlloc((UIntPtr)(void*)(bufferLength + 1)).ToPointer(); |
There was a problem hiding this comment.
Also, note that you need to have separate the marshaling and the allocation. There are cases you only marshal, but without allocation. Example: Native -> Managed with [out]. Also, in that case, you might also need to pass the native array length to avoid overwriting the buffer. Finally, the allocation needs to be of the right size, which isn't necessarily the ansi char array size.
| return null; | ||
| } | ||
|
|
||
| char* buffer = (char*)PInvokeMarshal.CoTaskMemAlloc((UIntPtr)(sizeof(char) * (sb.Capacity + 1))).ToPointer(); |
There was a problem hiding this comment.
See the array case. You need to support [out] stringBuilder case correctly.
| [DllImport("*", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode)] | ||
| private static extern int VerifyStringBuilder(StringBuilder sb); | ||
| [DllImport("*", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, EntryPoint = "VerifyUnicodeStringBuilder")] | ||
| private static extern int VerifyUnicodeStringBuilder(StringBuilder sb); |
There was a problem hiding this comment.
Please add [out] stringbuilder case.
There was a problem hiding this comment.
Also, where are the ansi array case? Please add [out] array case as well.
There was a problem hiding this comment.
We don't support Ansi array case yet. I will send out another PR with that and add test cases for it.
|
I have send out a change for review in .NET Native to move the ANSI string marshalling helpers from S.P.Interop to S.P.CoreLIb. One that change is checked in and ported here, I will update this PR to hookup those helpers. |
It does the following: * Enable support for correct ANSI marshalling semantics (dotnet#2476) by calling into PInvokeMarshal ANSI string marshalling helpers. * Split Alloc and Transform for StringBuilder marshalling * Added test case for [Out] StringBuilder marshalling * Added null check in array marshaller.
|
@yizhang82 PTAL |
|
LGTM. Thanks! |
We previously had only [OUT] UnicodeStringBuilder marshalling support. This change implements support for both Unicode and ANSI StringBuilder. It should also fix issue #2959