Skip to content

Commit c5ae13d

Browse files
elinor-fungelinor-fung
committed
PR feedback
1 parent cff2b5d commit c5ae13d

6 files changed

Lines changed: 95 additions & 86 deletions

File tree

‎docs/design/libraries/LibraryImportGenerator/Compatibility.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ When marshalling as [ANSI](https://docs.microsoft.com/windows/win32/intl/code-pa
5656
- Best-fit mapping will be disabled and no exception will be thrown for unmappable characters. In the built-in system, this behaviour was configured through [`DllImportAttribute.BestFitMapping`] and [`DllImportAttribute.ThrowOnUnmappableChar`]. The generated marshalling code will have the equivalent behaviour of `BestFitMapping=false` and `ThrowOnUnmappableChar=false`.
5757

5858
The p/invoke source generator does not provide an equivalent to using `CharSet.Auto` in the built-in system. If platform-dependent behaviour is desired, it is left to the user to define different p/invokes with different marshalling configurations.
59-
Similarly, `UnmanagedType.LPStr` will only mean ANSI rather than ANSI on Windows and UTF-8 on non-Windows. `UnmanagedType.LPUTF8Str` or `StringMarshalling.Utf8` can be used for UTF-8 and it is left to the user to define different p/invokes if platform-dependent behaviour is desired.
6059

6160
### `bool` marshalling
6261

‎src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/AnsiStringMarshaller.cs‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ namespace System.Runtime.InteropServices
1313
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
1414
public unsafe ref struct AnsiStringMarshaller
1515
{
16-
private byte* allocated;
17-
private Span<byte> span;
16+
private byte* _allocated;
17+
private readonly Span<byte> _span;
1818

1919
/// <summary>
2020
/// Initializes a new instance of the <see cref="AnsiStringMarshaller"/>.
2121
/// </summary>
2222
/// <param name="str">The string to marshal.</param>
2323
public AnsiStringMarshaller(string str)
24-
: this(str, default(Span<byte>))
24+
: this(str, default)
2525
{ }
2626

2727
/// <summary>
@@ -30,26 +30,30 @@ public AnsiStringMarshaller(string str)
3030
/// <param name="str">The string to marshal.</param>
3131
/// <param name="buffer">Buffer that may be used for marshalling.</param>
3232
/// <remarks>
33+
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
34+
/// on the managed heap or it should be pinned.
3335
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
3436
/// </remarks>
3537
public AnsiStringMarshaller(string str, Span<byte> buffer)
3638
{
37-
allocated = null;
38-
span = default;
39+
_allocated = null;
3940
if (str is null)
41+
{
42+
_span = default;
4043
return;
44+
}
4145

42-
// +1 for null terminator
43-
// + 1 for the null character from the user. + 1 for the null character we put in.
44-
int maxLength = (str.Length + 1) * Marshal.SystemMaxDBCSCharSize + 1;
45-
if (buffer.Length >= maxLength)
46+
// + 1 for null terminator
47+
int maxByteCount = (str.Length + 1) * Marshal.SystemMaxDBCSCharSize + 1;
48+
if (buffer.Length >= maxByteCount)
4649
{
47-
int length = Marshal.StringToAnsiString(str, (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)), buffer.Length);
48-
span = buffer.Slice(0, length);
50+
Marshal.StringToAnsiString(str, (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)), buffer.Length);
51+
_span = buffer;
4952
}
5053
else
5154
{
52-
allocated = (byte*)Marshal.StringToCoTaskMemAnsi(str);
55+
_allocated = (byte*)Marshal.StringToCoTaskMemAnsi(str);
56+
_span = default;
5357
}
5458
}
5559

@@ -59,7 +63,7 @@ public AnsiStringMarshaller(string str, Span<byte> buffer)
5963
/// <remarks>
6064
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
6165
/// </remarks>
62-
public byte* ToNativeValue() => allocated != null ? allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
66+
public byte* ToNativeValue() => _allocated != null ? _allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));
6367

6468
/// <summary>
6569
/// Sets the native value representing the string.
@@ -68,22 +72,26 @@ public AnsiStringMarshaller(string str, Span<byte> buffer)
6872
/// <remarks>
6973
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
7074
/// </remarks>
71-
public void FromNativeValue(byte* value) => allocated = value;
75+
public void FromNativeValue(byte* value) => _allocated = value;
7276

7377
/// <summary>
7478
/// Returns the managed string.
7579
/// </summary>
7680
/// <remarks>
7781
/// <seealso cref="CustomTypeMarshallerDirection.Out"/>
7882
/// </remarks>
79-
public string? ToManaged() => allocated == null ? null : new string((sbyte*)allocated);
83+
public string? ToManaged() => _allocated == null ? null : new string((sbyte*)_allocated);
8084

8185
/// <summary>
8286
/// Frees native resources.
8387
/// </summary>
8488
/// <remarks>
8589
/// <seealso cref="CustomTypeMarshallerFeatures.UnmanagedResources"/>
8690
/// </remarks>
87-
public void FreeNative() => Marshal.FreeCoTaskMem((IntPtr)allocated);
91+
public void FreeNative()
92+
{
93+
if (_allocated != null)
94+
Marshal.FreeCoTaskMem((IntPtr)_allocated);
95+
}
8896
}
8997
}

‎src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Utf16StringMarshaller.cs‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ namespace System.Runtime.InteropServices
1313
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
1414
public unsafe ref struct Utf16StringMarshaller
1515
{
16-
private ushort* allocated;
17-
private Span<ushort> span;
18-
private bool isNullString;
16+
private ushort* _allocated;
17+
private readonly Span<ushort> _span;
18+
private bool _isNullString;
1919

2020
/// <summary>
2121
/// Initializes a new instance of the <see cref="Utf16StringMarshaller"/>.
2222
/// </summary>
2323
/// <param name="str">The string to marshal.</param>
2424
public Utf16StringMarshaller(string str)
25-
: this(str, default(Span<ushort>))
25+
: this(str, default)
2626
{
2727
}
2828

@@ -32,29 +32,31 @@ public Utf16StringMarshaller(string str)
3232
/// <param name="str">The string to marshal.</param>
3333
/// <param name="buffer">Buffer that may be used for marshalling.</param>
3434
/// <remarks>
35+
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
36+
/// on the managed heap or it should be pinned.
3537
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
3638
/// </remarks>
3739
public Utf16StringMarshaller(string str, Span<ushort> buffer)
3840
{
39-
isNullString = false;
40-
span = default;
41+
_isNullString = false;
42+
_allocated = null;
4143
if (str is null)
4244
{
43-
allocated = null;
44-
isNullString = true;
45+
_isNullString = true;
46+
_span = default;
4547
}
4648
else if ((str.Length + 1) < buffer.Length)
4749
{
48-
span = buffer;
49-
str.AsSpan().CopyTo(MemoryMarshal.Cast<ushort, char>(buffer));
50+
_span = buffer;
51+
str.CopyTo(MemoryMarshal.Cast<ushort, char>(buffer));
5052
// Supplied memory is in an undefined state so ensure
5153
// there is a trailing null in the buffer.
52-
span[str.Length] = '\0';
53-
allocated = null;
54+
_span[str.Length] = '\0';
5455
}
5556
else
5657
{
57-
allocated = (ushort*)Marshal.StringToCoTaskMemUni(str);
58+
_allocated = (ushort*)Marshal.StringToCoTaskMemUni(str);
59+
_span = default;
5860
}
5961
}
6062

@@ -63,10 +65,10 @@ public Utf16StringMarshaller(string str, Span<ushort> buffer)
6365
/// </summary>
6466
public ref ushort GetPinnableReference()
6567
{
66-
if (allocated != null)
67-
return ref Unsafe.AsRef<ushort>(allocated);
68+
if (_allocated != null)
69+
return ref Unsafe.AsRef<ushort>(_allocated);
6870

69-
return ref span.GetPinnableReference();
71+
return ref _span.GetPinnableReference();
7072
}
7173

7274
/// <summary>
@@ -75,7 +77,7 @@ public ref ushort GetPinnableReference()
7577
/// <remarks>
7678
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
7779
/// </remarks>
78-
public ushort* ToNativeValue() => allocated != null ? allocated : (ushort*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
80+
public ushort* ToNativeValue() => _allocated != null ? _allocated : (ushort*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));
7981

8082
/// <summary>
8183
/// Sets the native value representing the string.
@@ -86,8 +88,8 @@ public ref ushort GetPinnableReference()
8688
/// </remarks>
8789
public void FromNativeValue(ushort* value)
8890
{
89-
allocated = value;
90-
isNullString = value == null;
91+
_allocated = value;
92+
_isNullString = value == null;
9193
}
9294

9395
/// <summary>
@@ -98,13 +100,13 @@ public void FromNativeValue(ushort* value)
98100
/// </remarks>
99101
public string? ToManaged()
100102
{
101-
if (isNullString)
103+
if (_isNullString)
102104
return null;
103105

104-
if (allocated != null)
105-
return new string((char*)allocated);
106+
if (_allocated != null)
107+
return new string((char*)_allocated);
106108

107-
return MemoryMarshal.Cast<ushort, char>(span).ToString();
109+
return MemoryMarshal.Cast<ushort, char>(_span).ToString();
108110
}
109111

110112
/// <summary>
@@ -115,8 +117,8 @@ public void FromNativeValue(ushort* value)
115117
/// </remarks>
116118
public void FreeNative()
117119
{
118-
if (allocated != null)
119-
Marshal.FreeCoTaskMem((IntPtr)allocated);
120+
if (_allocated != null)
121+
Marshal.FreeCoTaskMem((IntPtr)_allocated);
120122
}
121123
}
122124
}

‎src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Utf8StringMarshaller.cs‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,15 @@ namespace System.Runtime.InteropServices
1414
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
1515
public unsafe ref struct Utf8StringMarshaller
1616
{
17-
private byte* allocated;
18-
private Span<byte> span;
19-
20-
// Conversion from a 2-byte 'char' in UTF-16 to bytes in UTF-8 has a maximum of 3 bytes per 'char'
21-
// Two bytes ('char') in UTF-16 can be either:
22-
// - Code point in the Basic Multilingual Plane: all 16 bits are that of the code point
23-
// - Part of a pair for a code point in the Supplementary Planes: 10 bits are that of the code point
24-
// In UTF-8, 3 bytes are need to represent the code point in first and 4 bytes in the second. Thus, the
25-
// maximum number of bytes per 'char' is 3.
26-
private const int MaxByteCountPerChar = 3;
17+
private byte* _allocated;
18+
private readonly Span<byte> _span;
2719

2820
/// <summary>
2921
/// Initializes a new instance of the <see cref="Utf8StringMarshaller"/>.
3022
/// </summary>
3123
/// <param name="str">The string to marshal.</param>
3224
public Utf8StringMarshaller(string str)
33-
: this(str, default(Span<byte>))
25+
: this(str, default)
3426
{ }
3527

3628
/// <summary>
@@ -39,27 +31,37 @@ public Utf8StringMarshaller(string str)
3931
/// <param name="str">The string to marshal.</param>
4032
/// <param name="buffer">Buffer that may be used for marshalling.</param>
4133
/// <remarks>
34+
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
35+
/// on the managed heap or it should be pinned.
4236
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
4337
/// </remarks>
4438
public Utf8StringMarshaller(string str, Span<byte> buffer)
4539
{
46-
allocated = null;
47-
span = default;
40+
_allocated = null;
4841
if (str is null)
42+
{
43+
_span = default;
4944
return;
45+
}
5046

51-
// + 1 for number of characters in case left over high surrogate is ?
52-
// * <MaxByteCountPerChar> (3 for UTF-8)
53-
// +1 for null terminator
54-
if (buffer.Length >= (str.Length + 1) * MaxByteCountPerChar + 1)
47+
// + 1 for null terminator
48+
int maxByteCount = Encoding.UTF8.GetMaxByteCount(str.Length) + 1;
49+
if (buffer.Length >= maxByteCount)
5550
{
5651
int byteCount = Encoding.UTF8.GetBytes(str, buffer);
5752
buffer[byteCount] = 0; // null-terminate
58-
span = buffer;
53+
_span = buffer;
5954
}
6055
else
6156
{
62-
allocated = (byte*)Marshal.StringToCoTaskMemUTF8(str);
57+
_allocated = (byte*)Marshal.AllocCoTaskMem(maxByteCount);
58+
int byteCount;
59+
fixed (char* ptr = str)
60+
{
61+
byteCount = Encoding.UTF8.GetBytes(ptr, str.Length, _allocated, maxByteCount);
62+
}
63+
_allocated[byteCount] = 0; // null-terminate
64+
_span = default;
6365
}
6466
}
6567

@@ -69,7 +71,7 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
6971
/// <remarks>
7072
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
7173
/// </remarks>
72-
public byte* ToNativeValue() => allocated != null ? allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
74+
public byte* ToNativeValue() => _allocated != null ? _allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));
7375

7476
/// <summary>
7577
/// Sets the native value representing the string.
@@ -78,15 +80,15 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
7880
/// <remarks>
7981
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
8082
/// </remarks>
81-
public void FromNativeValue(byte* value) => allocated = value;
83+
public void FromNativeValue(byte* value) => _allocated = value;
8284

8385
/// <summary>
8486
/// Returns the managed string.
8587
/// </summary>
8688
/// <remarks>
8789
/// <seealso cref="CustomTypeMarshallerDirection.Out"/>
8890
/// </remarks>
89-
public string? ToManaged() => allocated == null ? null : Marshal.PtrToStringUTF8((IntPtr)allocated);
91+
public string? ToManaged() => _allocated == null ? null : Marshal.PtrToStringUTF8((IntPtr)_allocated);
9092

9193
/// <summary>
9294
/// Frees native resources.
@@ -96,8 +98,8 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
9698
/// </remarks>
9799
public void FreeNative()
98100
{
99-
if (allocated != null)
100-
Marshal.FreeCoTaskMem((IntPtr)allocated);
101+
if (_allocated != null)
102+
Marshal.FreeCoTaskMem((IntPtr)_allocated);
101103
}
102104
}
103105
}

0 commit comments

Comments
 (0)