Fix GetTempPath (#16921)
authorJeremy Kuhne <jeremy.kuhne@microsoft.com>
Thu, 15 Mar 2018 05:16:00 +0000 (22:16 -0700)
committerGitHub <noreply@github.com>
Thu, 15 Mar 2018 05:16:00 +0000 (22:16 -0700)
* Fix GetTempPath

GetTempPath and GetTempFileName weren't updated to handle long paths.
Update to use API properly and stop using StringBuilder. Also tweak
Normalize to allow utilizing an existing Span as input.

* Assert for null, fix ProjectN wrap

* Address more feedback

* One more tweak

* Tweak Normalize to be usable in CoreFX

* Change another span Equals

* Split AsSpan() to avoid risk of losing inlining

src/mscorlib/shared/Interop/Windows/Kernel32/Interop.GetFullPathNameW.cs
src/mscorlib/shared/Interop/Windows/Kernel32/Interop.GetTempFileNameW.cs
src/mscorlib/shared/Interop/Windows/Kernel32/Interop.GetTempPathW.cs
src/mscorlib/shared/System/IO/Path.Windows.cs
src/mscorlib/shared/System/IO/PathHelper.Windows.cs
src/mscorlib/shared/System/Text/ValueStringBuilder.cs

index 0603045..e64129b 100644 (file)
@@ -14,16 +14,17 @@ internal partial class Interop
         /// </summary>
         [DllImport(Libraries.Kernel32, SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false, ExactSpelling = true)]
 #if PROJECTN
-        internal static extern unsafe uint GetFullPathNameW(string path, uint numBufferChars, char* buffer, IntPtr mustBeZero);
+        internal static extern unsafe uint GetFullPathNameW(char* lpFileName, uint nBufferLength, char* lpBuffer, IntPtr lpFilePart);
 
         // Works around https://devdiv.visualstudio.com/web/wi.aspx?pcguid=011b8bdf-6d56-4f87-be0d-0092136884d9&id=575202
-        internal static unsafe uint GetFullPathNameW(string path, uint numBufferChars, ref char buffer, IntPtr mustBeZero)
+        internal static unsafe uint GetFullPathNameW(ref char lpFileName, uint nBufferLength, ref char lpBuffer, IntPtr lpFilePart)
         {
-            fixed (char* pBuffer = &buffer)
-                return GetFullPathNameW(path, numBufferChars, pBuffer, mustBeZero);
+            fixed (char* pBuffer = &lpBuffer)
+            fixed (char* pFileName = &lpFileName)
+                return GetFullPathNameW(pFileName, nBufferLength, pBuffer, mustBeZero);
         }
 #else
-        internal static extern uint GetFullPathNameW(string path, uint numBufferChars, ref char buffer, IntPtr mustBeZero);
+        internal static extern uint GetFullPathNameW(ref char lpFileName, uint nBufferLength, ref char lpBuffer, IntPtr lpFilePart);
 #endif
     }
 }
index 3667389..97e1d82 100644 (file)
@@ -2,8 +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.Text;
 using System.Runtime.InteropServices;
 
 internal partial class Interop
@@ -11,6 +9,6 @@ internal partial class Interop
     internal partial class Kernel32
     {
         [DllImport(Libraries.Kernel32, CharSet = CharSet.Unicode, SetLastError = true, BestFitMapping = false)]
-        internal static extern uint GetTempFileNameW(string tmpPath, string prefix, uint uniqueIdOrZero, [Out]StringBuilder tmpFileName);
+        internal static extern uint GetTempFileNameW(ref char lpPathName, string lpPrefixString, uint uUnique, ref char lpTempFileName);
     }
 }
index ff2783b..7f7bb77 100644 (file)
@@ -2,8 +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.IO;
-using System.Text;
 using System.Runtime.InteropServices;
 
 internal partial class Interop
@@ -11,6 +9,6 @@ internal partial class Interop
     internal partial class Kernel32
     {
         [DllImport(Libraries.Kernel32, CharSet = CharSet.Unicode, BestFitMapping = false)]
-        internal static extern uint GetTempPathW(int bufferLen, [Out]StringBuilder buffer);
+        internal static extern uint GetTempPathW(int bufferLen, ref char buffer);
     }
 }
index 1934e8d..8f1bed0 100644 (file)
@@ -128,24 +128,61 @@ namespace System.IO
 
         public static string GetTempPath()
         {
-            StringBuilder sb = StringBuilderCache.Acquire(Interop.Kernel32.MAX_PATH);
-            uint r = Interop.Kernel32.GetTempPathW(Interop.Kernel32.MAX_PATH, sb);
-            if (r == 0)
+            Span<char> initialBuffer = stackalloc char[PathInternal.MaxShortPath];
+            var builder = new ValueStringBuilder(initialBuffer);
+
+            GetTempPath(ref builder);
+
+            string path = PathHelper.Normalize(ref builder);
+            builder.Dispose();
+            return path;
+        }
+
+        private static void GetTempPath(ref ValueStringBuilder builder)
+        {
+            uint result = 0;
+            while ((result = Interop.Kernel32.GetTempPathW(builder.Capacity, ref builder.GetPinnableReference())) > builder.Capacity)
+            {
+                // Reported size is greater than the buffer size. Increase the capacity.
+                builder.EnsureCapacity(checked((int)result));
+            }
+
+            if (result == 0)
                 throw Win32Marshal.GetExceptionForLastWin32Error();
-            return GetFullPath(StringBuilderCache.GetStringAndRelease(sb));
+
+            builder.Length = (int)result;
         }
 
         // Returns a unique temporary file name, and creates a 0-byte file by that
         // name on disk.
         public static string GetTempFileName()
         {
-            string path = GetTempPath();
+            Span<char> initialTempPathBuffer = stackalloc char[PathInternal.MaxShortPath];
+            ValueStringBuilder tempPathBuilder = new ValueStringBuilder(initialTempPathBuffer);
 
-            StringBuilder sb = StringBuilderCache.Acquire(Interop.Kernel32.MAX_PATH);
-            uint r = Interop.Kernel32.GetTempFileNameW(path, "tmp", 0, sb);
-            if (r == 0)
+            GetTempPath(ref tempPathBuilder);
+
+            Span<char> initialBuffer = stackalloc char[PathInternal.MaxShortPath];
+            var builder = new ValueStringBuilder(initialBuffer);
+
+            uint result = 0;
+            while ((result = Interop.Kernel32.GetTempFileNameW(
+                ref tempPathBuilder.GetPinnableReference(), "tmp", 0, ref builder.GetPinnableReference())) > builder.Capacity)
+            {
+                // Reported size is greater than the buffer size. Increase the capacity.
+                builder.EnsureCapacity(checked((int)result));
+            }
+
+            tempPathBuilder.Dispose();
+
+            if (result == 0)
                 throw Win32Marshal.GetExceptionForLastWin32Error();
-            return StringBuilderCache.GetStringAndRelease(sb);
+
+            builder.Length = (int)result;
+
+            string path = PathHelper.Normalize(ref builder);
+            builder.Dispose();
+            return path;
         }
 
         // Tests if the given path contains a root. A path is considered rooted
index df773da..2b17fff 100644 (file)
@@ -26,30 +26,57 @@ namespace System.IO
         internal static string Normalize(string path)
         {
             Span<char> initialBuffer = stackalloc char[PathInternal.MaxShortPath];
-            ValueStringBuilder builder = new ValueStringBuilder(initialBuffer);
+            var builder = new ValueStringBuilder(initialBuffer);
 
             // Get the full path
-            GetFullPathName(path, ref builder);
+            GetFullPathName(path.AsSpan(), ref builder);
 
             // If we have the exact same string we were passed in, don't allocate another string.
             // TryExpandShortName does this input identity check.
-            string result = builder.AsSpan().Contains('~')
+            string result = builder.AsSpan().IndexOf('~') >= 0
                 ? TryExpandShortFileName(ref builder, originalPath: path)
-                : builder.AsSpan().EqualsOrdinal(path.AsSpan()) ? path : builder.ToString();
+                : builder.AsSpan().Equals(path.AsSpan(), StringComparison.Ordinal) ? path : builder.ToString();
+
+            // Clear the buffer
+            builder.Dispose();
+            return result;
+        }
+
+        /// <summary>
+        /// Normalize the given path.
+        /// </summary>
+        /// <remarks>
+        /// Exceptions are the same as the string overload.
+        /// </remarks>
+        internal static string Normalize(ref ValueStringBuilder path)
+        {
+            Span<char> initialBuffer = stackalloc char[PathInternal.MaxShortPath];
+            var builder = new ValueStringBuilder(initialBuffer);
+
+            // Get the full path
+            GetFullPathName(path.AsSpan(terminate: true), ref builder);
+
+            string result = builder.AsSpan().Contains('~')
+                ? TryExpandShortFileName(ref builder, originalPath: null)
+                : builder.ToString();
 
             // Clear the buffer
             builder.Dispose();
             return result;
         }
 
-        private static void GetFullPathName(string path, ref ValueStringBuilder builder)
+        /// <summary>
+        /// Calls GetFullPathName on the given path.
+        /// </summary>
+        /// <param name="path">The path name. MUST be null terminated after the span.</param>
+        private static void GetFullPathName(ReadOnlySpan<char> path, ref ValueStringBuilder builder)
         {
             // If the string starts with an extended prefix we would need to remove it from the path before we call GetFullPathName as
             // it doesn't root extended paths correctly. We don't currently resolve extended paths, so we'll just assert here.
             Debug.Assert(PathInternal.IsPartiallyQualified(path) || !PathInternal.IsExtended(path));
 
             uint result = 0;
-            while ((result = Interop.Kernel32.GetFullPathNameW(path, (uint)builder.Capacity, ref builder.GetPinnableReference(), IntPtr.Zero)) > builder.Capacity)
+            while ((result = Interop.Kernel32.GetFullPathNameW(ref MemoryMarshal.GetReference(path), (uint)builder.Capacity, ref builder.GetPinnableReference(), IntPtr.Zero)) > builder.Capacity)
             {
                 // Reported size is greater than the buffer size. Increase the capacity.
                 builder.EnsureCapacity(checked((int)result));
@@ -61,7 +88,7 @@ namespace System.IO
                 int errorCode = Marshal.GetLastWin32Error();
                 if (errorCode == 0)
                     errorCode = Interop.Errors.ERROR_BAD_PATHNAME;
-                throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
+                throw Win32Marshal.GetExceptionForWin32Error(errorCode, path.ToString());
             }
 
             builder.Length = (int)result;
@@ -119,7 +146,7 @@ namespace System.IO
             bool isDevice = PathInternal.IsDevice(outputBuilder.AsSpan());
 
             // As this is a corner case we're not going to add a stackalloc here to keep the stack pressure down.
-            ValueStringBuilder inputBuilder = new ValueStringBuilder();
+            var inputBuilder = new ValueStringBuilder();
 
             bool isDosUnc = false;
             int rootDifference = 0;
@@ -149,12 +176,10 @@ namespace System.IO
             bool success = false;
             int foundIndex = inputBuilder.Length - 1;
 
-            // Need to null terminate the input builder
-            inputBuilder.Append('\0');
-
             while (!success)
             {
-                uint result = Interop.Kernel32.GetLongPathNameW(ref inputBuilder.GetPinnableReference(), ref outputBuilder.GetPinnableReference(), (uint)outputBuilder.Capacity);
+                uint result = Interop.Kernel32.GetLongPathNameW(
+                    ref inputBuilder.GetPinnableReference(terminate: true), ref outputBuilder.GetPinnableReference(), (uint)outputBuilder.Capacity);
 
                 // Replace any temporary null we added
                 if (inputBuilder[foundIndex] == '\0') inputBuilder[foundIndex] = '\\';
@@ -197,15 +222,12 @@ namespace System.IO
                     outputBuilder.Length = checked((int)result);
                     if (foundIndex < inputLength - 1)
                     {
-                        // It was a partial find, put the non-existent part of the path back (minus the added null)
-                        outputBuilder.Append(inputBuilder.AsSpan(foundIndex, inputBuilder.Length - foundIndex - 1));
+                        // It was a partial find, put the non-existent part of the path back
+                        outputBuilder.Append(inputBuilder.AsSpan(foundIndex, inputBuilder.Length - foundIndex));
                     }
                 }
             }
 
-            // Need to trim out the trailing null in the input builder
-            inputBuilder.Length = inputBuilder.Length - 1;
-
             // If we were able to expand the path, use it, otherwise use the original full path result
             ref ValueStringBuilder builderToUse = ref (success ? ref outputBuilder : ref inputBuilder);
 
@@ -220,7 +242,7 @@ namespace System.IO
             // Strip out any added characters at the front of the string
             ReadOnlySpan<char> output = builderToUse.AsSpan(rootDifference);
 
-            string returnValue = output.EqualsOrdinal(originalPath.AsSpan())
+            string returnValue = ((originalPath != null) && output.Equals(originalPath.AsSpan(), StringComparison.Ordinal))
                 ? originalPath : new string(output);
 
             inputBuilder.Dispose();
index d43e8f9..b1abdd5 100644 (file)
@@ -40,7 +40,19 @@ namespace System.Text
                 Grow(capacity - _chars.Length);
         }
 
-        public ref char GetPinnableReference() => ref MemoryMarshal.GetReference(_chars);
+        /// <summary>
+        /// Get a pinnable reference to the builder.
+        /// </summary>
+        /// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
+        public ref char GetPinnableReference(bool terminate = false)
+        {
+            if (terminate)
+            {
+                EnsureCapacity(Length + 1);
+                _chars[Length] = '\0';
+            }
+            return ref MemoryMarshal.GetReference(_chars);
+        }
 
         public ref char this[int index]
         {
@@ -58,6 +70,20 @@ namespace System.Text
             return s;
         }
 
+        /// <summary>
+        /// Returns a span around the contents of the builder.
+        /// </summary>
+        /// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
+        public ReadOnlySpan<char> AsSpan(bool terminate)
+        {
+            if (terminate)
+            {
+                EnsureCapacity(Length + 1);
+                _chars[Length] = '\0';
+            }
+            return _chars.Slice(0, _pos);
+        }
+
         public ReadOnlySpan<char> AsSpan() => _chars.Slice(0, _pos);
         public ReadOnlySpan<char> AsSpan(int start) => _chars.Slice(start, _pos - start);
         public ReadOnlySpan<char> AsSpan(int start, int length) => _chars.Slice(start, length);