From: Matt Ellis Date: Wed, 2 Sep 2015 01:40:04 +0000 (-0700) Subject: Don't use StringBuilderCache for casing X-Git-Tag: accepted/tizen/base/20180629.140029~6381^2~7 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb057fc92d732fd438c8e7f96195e67e3c92f7f4;p=platform%2Fupstream%2Fcoreclr.git Don't use StringBuilderCache for casing During code review, Steve proposed we moved to using StringBuilderCache to the allocation of the temporary char[] array during casing operations. I made the change but later realized that this caused a few issues. - The native layer operates on UChar/length pairs, not null terminated strings. Because of this, we don't actually write a terminating null into the destination buffer (since we just do a 1:1 casing operation on the input and never see the null terminator). However, the marshalling code for StringBuilder assumes the buffer will be null terminated and uses that to compute the new length value after a marshalling call. Because there can be left over data from previous calls in the buffer the string length calculation will be incorrect and we will end up leaking in extra data into the cased string. - The StringBuilder marshalling as a whole won't work if we have embdedded nulls in a string (we'll end up dropping characters on the transition from native back to managed) but that's something that we need to be able to handle. Ideally there would be a way to construct a mutable string, case into its buffer and the freeze the string, but we don't have a way to do that today, so we need to stick with this allocation. --- diff --git a/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Casing.cs b/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Casing.cs index f0980a7..5fdcf30 100644 --- a/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Casing.cs +++ b/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Casing.cs @@ -13,6 +13,6 @@ internal static partial class Interop internal unsafe static extern void ChangeCase(char* src, int srcLen, char* dstBuffer, int dstBufferCapacity, bool bIsUpper, bool bTurkishCasing); [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)] - internal unsafe static extern void ChangeCase(string src, int srcLen, StringBuilder dstBuffer, int dstBufferCapacity, bool bIsUpper, bool bTurkishCasing); + internal unsafe static extern void ChangeCase(string src, int srcLen, char* dstBuffer, int dstBufferCapacity, bool bIsUpper, bool bTurkishCasing); } } diff --git a/src/mscorlib/corefx/System/Globalization/TextInfo.Unix.cs b/src/mscorlib/corefx/System/Globalization/TextInfo.Unix.cs index 3f2ce0b..9a18175 100644 --- a/src/mscorlib/corefx/System/Globalization/TextInfo.Unix.cs +++ b/src/mscorlib/corefx/System/Globalization/TextInfo.Unix.cs @@ -31,11 +31,14 @@ namespace System.Globalization { Contract.Assert(s != null); - StringBuilder sb = StringBuilderCache.Acquire(s.Length); + char[] buf = new char[s.Length]; - Interop.GlobalizationInterop.ChangeCase(s, s.Length, sb, sb.Capacity, toUpper, m_needsTurkishCasing); + fixed(char* pBuf = buf) + { + Interop.GlobalizationInterop.ChangeCase(s, s.Length, pBuf, buf.Length, toUpper, m_needsTurkishCasing); + } - return StringBuilderCache.GetStringAndRelease(sb); + return new string(buf); } [System.Security.SecuritySafeCritical] @@ -63,4 +66,4 @@ namespace System.Globalization return lcName.Length >= 2 && ((lcName[0] == 't' && lcName[1] == 'r') || (lcName[0] == 'a' && lcName[1] == 'z')); } } -} \ No newline at end of file +}