Skip to content

Commit

Permalink
Remove interop wrappers, pin in call sites, don't guess the output le…
Browse files Browse the repository at this point in the history 
…ngth on Windows
  • Loading branch information
@MihaZupan
MihaZupan committed Apr 9, 2020
1 parent 4dce18a commit ee37454
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 82 deletions.
17 changes: 2 additions & 15 deletions src/libraries/Common/src/Interop/Unix/System.Globalization.Native/Interop.Normalization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;
using System.Text;

Expand All @@ -11,21 +10,9 @@ internal static partial class Interop
internal static partial class Globalization
{
[DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IsNormalized")]
internal static extern int IsNormalized(NormalizationForm normalizationForm, string src, int srcLen);

internal static int NormalizeString(NormalizationForm normalizationForm, string src, int srcLen, Span<char> dstBuffer)
{
unsafe
{
fixed (char* pSrc = src)
fixed (char* pDest = &MemoryMarshal.GetReference(dstBuffer))
{
return NormalizeString(normalizationForm, pSrc, srcLen, pDest, dstBuffer.Length);
}
}
}
internal static extern int IsNormalized(NormalizationForm normalizationForm, char* src, int srcLen);

[DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_NormalizeString")]
private static extern unsafe int NormalizeString(NormalizationForm normalizationForm, char* src, int srcLen, char* dstBuffer, int dstBufferCapacity);
internal static extern unsafe int NormalizeString(NormalizationForm normalizationForm, char* src, int srcLen, char* dstBuffer, int dstBufferCapacity);
}
}
17 changes: 2 additions & 15 deletions src/libraries/Common/src/Interop/Windows/Normaliz/Interop.Normalization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;
using System.Text;

Expand All @@ -11,22 +10,10 @@ internal static partial class Interop
internal static partial class Normaliz
{
[DllImport("Normaliz.dll", CharSet = CharSet.Unicode, SetLastError = true)]
internal static extern bool IsNormalizedString(NormalizationForm normForm, string source, int length);

internal static int NormalizeString(NormalizationForm normForm, string source, int sourceLength, Span<char> destination)
{
unsafe
{
fixed (char* pSrc = source)
fixed (char* pDest = &MemoryMarshal.GetReference(destination))
{
return NormalizeString(normForm, pSrc, sourceLength, pDest, destination.Length);
}
}
}
internal static extern unsafe BOOL IsNormalizedString(NormalizationForm normForm, char* source, int length);

[DllImport("Normaliz.dll", CharSet = CharSet.Unicode, SetLastError = true)]
private static extern unsafe int NormalizeString(
internal static extern unsafe int NormalizeString(
NormalizationForm normForm,
char* source,
int sourceLength,
Expand Down
20 changes: 18 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/Normalization.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Buffers;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text;

namespace System.Globalization
Expand All @@ -21,7 +22,14 @@ internal static bool IsNormalized(string strInput, NormalizationForm normalizati

ValidateArguments(strInput, normalizationForm);

int ret = Interop.Globalization.IsNormalized(normalizationForm, strInput, strInput.Length);
int ret;
unsafe
{
fixed (char* pInput = strInput)
{
ret = Interop.Globalization.IsNormalized(normalizationForm, pInput, strInput.Length);
}
}

if (ret == -1)
{
Expand Down Expand Up @@ -51,7 +59,15 @@ internal static string Normalize(string strInput, NormalizationForm normalizatio

for (int attempt = 0; attempt < 2; attempt++)
{
realLen = Interop.Globalization.NormalizeString(normalizationForm, strInput, strInput.Length, buffer);
int realLen;
unsafe
{
fixed (char* pInput = strInput)
fixed (char* pDest = &MemoryMarshal.GetReference(buffer))
{
realLen = Interop.Globalization.NormalizeString(normalizationForm, pInput, strInput.Length, pDest, buffer.Length);
}
}

if (realLen == -1)
{
Expand Down
92 changes: 42 additions & 50 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/Normalization.Windows.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ internal static bool IsNormalized(string strInput, NormalizationForm normalizati
// The only way to know if IsNormalizedString failed is through checking the Win32 last error
// IsNormalizedString pinvoke has SetLastError attribute property which will set the last error
// to 0 (ERROR_SUCCESS) before executing the calls.
bool result = Interop.Normaliz.IsNormalizedString(normalizationForm, strInput, strInput.Length);
Interop.BOOL result;
unsafe
{
fixed (char* pInput = strInput)
{
result = Interop.Normaliz.IsNormalizedString(normalizationForm, pInput, strInput.Length);
}
}

int lastError = Marshal.GetLastWin32Error();
switch (lastError)
Expand All @@ -52,7 +59,7 @@ internal static bool IsNormalized(string strInput, NormalizationForm normalizati
throw new InvalidOperationException(SR.Format(SR.UnknownError_Num, lastError));
}

return result;
return result == Interop.BOOL.TRUE;
}

internal static string Normalize(string strInput, NormalizationForm normalizationForm)
Expand All @@ -66,79 +73,67 @@ internal static string Normalize(string strInput, NormalizationForm normalizatio

Debug.Assert(strInput != null);

// we depend on Win32 last error when calling NormalizeString
// NormalizeString pinvoke has SetLastError attribute property which will set the last error
// to 0 (ERROR_SUCCESS) before executing the calls.

// Guess our buffer size first
int iLength = Interop.Normaliz.NormalizeString(normalizationForm, strInput, strInput.Length, Span<char>.Empty);

int lastError = Marshal.GetLastWin32Error();
// Could have an error (actually it'd be quite hard to have an error here)
if ((lastError != Interop.Errors.ERROR_SUCCESS) || iLength < 0)
if (strInput.Length == 0)
{
if (lastError == Interop.Errors.ERROR_INVALID_PARAMETER)
{
if (normalizationForm != NormalizationForm.FormC &&
normalizationForm != NormalizationForm.FormD &&
normalizationForm != NormalizationForm.FormKC &&
normalizationForm != NormalizationForm.FormKD)
{
throw new ArgumentException(SR.Argument_InvalidNormalizationForm, nameof(normalizationForm));
}

throw new ArgumentException(SR.Argument_InvalidCharSequenceNoIndex, nameof(strInput));
}

// We shouldn't really be able to get here..., guessing length is
// a trivial math function...
// Can't really be Out of Memory, but just in case:
if (lastError == Interop.Errors.ERROR_NOT_ENOUGH_MEMORY)
throw new OutOfMemoryException();

// Who knows what happened? Not us!
throw new InvalidOperationException(SR.Format(SR.UnknownError_Num, lastError));
return string.Empty;
}

// Don't break for empty strings (only possible for D & KD and not really possible at that)
if (iLength == 0) return string.Empty;

char[]? toReturn = null;
try
{
Span<char> buffer = iLength <= 512
Span<char> buffer = strInput.Length <= 512
? stackalloc char[512]
: (toReturn = ArrayPool<char>.Shared.Rent(iLength));
: (toReturn = ArrayPool<char>.Shared.Rent(strInput.Length));

while (true)
{
// we depend on Win32 last error when calling NormalizeString
// NormalizeString pinvoke has SetLastError attribute property which will set the last error
// to 0 (ERROR_SUCCESS) before executing the calls.
iLength = Interop.Normaliz.NormalizeString(normalizationForm, strInput, strInput.Length, buffer);
lastError = Marshal.GetLastWin32Error();

if (lastError == Interop.Errors.ERROR_SUCCESS)
break;
int realLength;
unsafe
{
fixed (char* pInput = strInput)
fixed (char* pDest = &MemoryMarshal.GetReference(buffer))
{
realLength = Interop.Normaliz.NormalizeString(normalizationForm, pInput, strInput.Length, pDest, buffer.Length);
}
}
int lastError = Marshal.GetLastWin32Error();

// Could have an error (actually it'd be quite hard to have an error here)
switch (lastError)
{
case Interop.Errors.ERROR_SUCCESS:
if (realLength == 0)

This comment has been minimized.

Sign in to view

Copy link
@jkotas

jkotas Apr 9, 2020

Member

You do not need to do this. The string constructor will do it for you.
{
return string.Empty;
}
return new string(buffer.Slice(0, realLength));

// Do appropriate stuff for the individual errors:
case Interop.Errors.ERROR_INSUFFICIENT_BUFFER:
iLength = Math.Abs(iLength);
Debug.Assert(iLength > buffer.Length, "Buffer overflow should have iLength > cBuffer.Length");
realLength = Math.Abs(realLength);
Debug.Assert(realLength > buffer.Length, "Buffer overflow should have iLength > cBuffer.Length");
if (toReturn != null)
{
// Clear toReturn first to ensure we don't return the same buffer twice
char[] temp = toReturn;
toReturn = null;
ArrayPool<char>.Shared.Return(temp);
}
buffer = toReturn = ArrayPool<char>.Shared.Rent(iLength);
buffer = toReturn = ArrayPool<char>.Shared.Rent(realLength);
continue;

case Interop.Errors.ERROR_INVALID_PARAMETER:
case Interop.Errors.ERROR_NO_UNICODE_TRANSLATION:
if (normalizationForm != NormalizationForm.FormC &&
normalizationForm != NormalizationForm.FormD &&
normalizationForm != NormalizationForm.FormKC &&
normalizationForm != NormalizationForm.FormKD)
{
throw new ArgumentException(SR.Argument_InvalidNormalizationForm, nameof(normalizationForm));
}

// Illegal code point or order found. Ie: FFFE or D800 D800, etc.
throw new ArgumentException(SR.Argument_InvalidCharSequenceNoIndex, nameof(strInput));

Expand All @@ -150,9 +145,6 @@ internal static string Normalize(string strInput, NormalizationForm normalizatio
throw new InvalidOperationException(SR.Format(SR.UnknownError_Num, lastError));
}
}

// Copy our buffer into our new string, which will be the appropriate size
return new string(buffer.Slice(0, iLength));
}
finally
{
Expand Down

0 comments on commit ee37454

Please sign in to comment.