Don't use StringBuilderCache for casing
authorMatt Ellis <matell@microsoft.com>
Wed, 2 Sep 2015 01:40:04 +0000 (18:40 -0700)
committerMatt Ellis <matell@microsoft.com>
Tue, 22 Sep 2015 18:50:37 +0000 (11:50 -0700)
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.

src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Casing.cs
src/mscorlib/corefx/System/Globalization/TextInfo.Unix.cs

index f0980a7..5fdcf30 100644 (file)
@@ -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);
     }
 }
index 3f2ce0b..9a18175 100644 (file)
@@ -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
+}