CA1838 Avoid 'StringBuilder' parameters for P/Invokes

eng/Common.globalconfig

@@ -319,7 +319,7 @@ dotnet_diagnostic.CA1836.severity = warning
 dotnet_diagnostic.CA1837.severity = suggestion
 
 # Avoid 'StringBuilder' parameters for P/Invokes
-dotnet_diagnostic.CA1838.severity = suggestion
+dotnet_diagnostic.CA1838.severity = warning
 
 # Dispose objects before losing scope
 dotnet_diagnostic.CA2000.severity = none

src/Framework/NativeMethods.cs

@@ -48,7 +48,6 @@ internal static class NativeMethods
     internal const int MAX_PATH = 260;
 
     private const string kernel32Dll = "kernel32.dll";
-    private const string mscoreeDLL = "mscoree.dll";
 
     private const string WINDOWS_FILE_SYSTEM_REGISTRY_KEY = @"SYSTEM\CurrentControlSet\Control\FileSystem";
     private const string WINDOWS_LONG_PATHS_ENABLED_VALUE_NAME = "LongPathsEnabled";
@@ -950,13 +949,13 @@ internal static string GetShortFilePath(string path)
 
             if (length > 0)
             {
-                StringBuilder fullPathBuffer = new StringBuilder(length);
+                char[] fullPathBuffer = new char[length];
                 length = GetShortPathName(path, fullPathBuffer, length);
                 errorCode = Marshal.GetLastWin32Error();
 
                 if (length > 0)
                 {
-                    string fullPath = fullPathBuffer.ToString();
+                    string fullPath = new(fullPathBuffer, 0, length);
                     path = fullPath;
                 }
             }
@@ -989,13 +988,13 @@ internal static string GetLongFilePath(string path)
 
             if (length > 0)
             {
-                StringBuilder fullPathBuffer = new StringBuilder(length);
+                char[] fullPathBuffer = new char[length];
                 length = GetLongPathName(path, fullPathBuffer, length);
                 errorCode = Marshal.GetLastWin32Error();
 
                 if (length > 0)
                 {
-                    string fullPath = fullPathBuffer.ToString();
+                    string fullPath = new(fullPathBuffer, 0, length);
                     path = fullPath;
                 }
             }
@@ -1084,7 +1083,7 @@ DateTime LastWriteFileUtcTime(string path)
 
                 if (success && (data.fileAttributes & NativeMethods.FILE_ATTRIBUTE_DIRECTORY) == 0)
                 {
-                    long dt = ((long) (data.ftLastWriteTimeHigh) << 32) | ((long) data.ftLastWriteTimeLow);
+                    long dt = ((long)(data.ftLastWriteTimeHigh) << 32) | ((long)data.ftLastWriteTimeLow);
                     fileModifiedTime = DateTime.FromFileTimeUtc(dt);
 
                     // If file is a symlink _and_ we're not instructed to do the wrong thing, get a more accurate timestamp.
@@ -1468,17 +1467,6 @@ internal static void VerifyThrowWin32Result(int result)
     [return: MarshalAs(UnmanagedType.Bool)]
     internal static extern bool GetFileAttributesEx(String name, int fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation);
 
-    [DllImport(kernel32Dll, SetLastError = true, CharSet = CharSet.Unicode)]
-    private static extern uint SearchPath
-    (
-        string path,
-        string fileName,
-        string extension,
-        int numBufferChars,
-        [Out] StringBuilder buffer,
-        int[] filePart
-    );
-
     [DllImport("kernel32.dll", PreserveSig = true, SetLastError = true)]
     [return: MarshalAs(UnmanagedType.Bool)]
     internal static extern bool FreeLibrary([In] IntPtr module);
@@ -1489,26 +1477,14 @@ int[] filePart
     [DllImport("kernel32.dll", CharSet = CharSet.Unicode, PreserveSig = true, SetLastError = true)]
     internal static extern IntPtr LoadLibrary(string fileName);
 
-    [DllImport(mscoreeDLL, SetLastError = true, CharSet = CharSet.Unicode)]
-    internal static extern uint GetRequestedRuntimeInfo(String pExe,
-                                            String pwszVersion,
-                                            String pConfigurationFile,
-                                            uint startupFlags,
-                                            uint runtimeInfoFlags,
-                                            [Out] StringBuilder pDirectory,
-                                            int dwDirectory,
-                                            out uint dwDirectoryLength,
-                                            [Out] StringBuilder pVersion,
-                                            int cchBuffer,
-                                            out uint dwlength);
-
     /// <summary>
-    /// Gets the fully qualified filename of the currently executing .exe
+    /// Gets the fully qualified filename of the currently executing .exe.
     /// </summary>
+    /// <param name="hModule"><see cref="HandleRef"/> of the module for which we are finding the file name.</param>
+    /// <param name="buffer">The character buffer used to return the file name.</param>
+    /// <param name="length">The length of the buffer.</param>
     [DllImport(kernel32Dll, SetLastError = true, CharSet = CharSet.Unicode)]
-    internal static extern int GetModuleFileName(
-            HandleRef hModule,
-            [Out] StringBuilder buffer, int length);
+    internal static extern int GetModuleFileName(HandleRef hModule, [Out] char[] buffer, int length);
 
     [DllImport("kernel32.dll")]
     internal static extern IntPtr GetStdHandle(int nStdHandle);
@@ -1557,10 +1533,10 @@ internal static bool SetCurrentDirectory(string path)
     private static extern bool GlobalMemoryStatusEx([In, Out] MemoryStatus lpBuffer);
 
     [DllImport("kernel32.dll", CharSet = CharSet.Unicode, BestFitMapping = false)]
-    internal static extern int GetShortPathName(string path, [Out] StringBuilder fullpath, [In] int length);
+    internal static extern int GetShortPathName(string path, [Out] char[] fullpath, [In] int length);
 
     [DllImport("kernel32.dll", CharSet = CharSet.Unicode, BestFitMapping = false)]
-    internal static extern int GetLongPathName([In] string path, [Out] StringBuilder fullpath, [In] int length);
+    internal static extern int GetLongPathName([In] string path, [Out] char[] fullpath, [In] int length);
 
     [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)]
     internal static extern bool CreatePipe(out SafeFileHandle hReadPipe, out SafeFileHandle hWritePipe, SecurityAttributes lpPipeAttributes, int nSize);
@@ -1648,7 +1624,7 @@ internal static bool MsgWaitOne(this WaitHandle handle, int timeout)
 
         if (!(returnValue == 0 || ((uint)returnValue == RPC_S_CALLPENDING && timeout != Timeout.Infinite)))
         {
-           throw new InternalErrorException($"Received {returnValue} from CoWaitForMultipleHandles, but expected 0 (S_OK)");
+            throw new InternalErrorException($"Received {returnValue} from CoWaitForMultipleHandles, but expected 0 (S_OK)");
         }
 
         return returnValue == 0;

src/Tasks/AssemblyDependency/AssemblyInformation.cs

@@ -569,30 +569,36 @@ internal static string GetRuntimeVersion(string path)
 #if FEATURE_MSCOREE
             if (NativeMethodsShared.IsWindows)
             {
-                StringBuilder runtimeVersion;
-                uint hresult;
 #if DEBUG
-                // Just to make sure and exercise the code that doubles the size
-                // every time GetRequestedRuntimeInfo fails due to insufficient buffer size.
+                // Just to make sure and exercise the code that uses dwLength to allocate the buffer
+                // when GetRequestedRuntimeInfo fails due to insufficient buffer size.
                 int bufferLength = 1;
 #else
                 int bufferLength = 11; // 11 is the length of a runtime version and null terminator v2.0.50727/0
 #endif
-                do
-                {
-                    runtimeVersion = new StringBuilder(bufferLength);
-                    hresult = NativeMethods.GetFileVersion(path, runtimeVersion, bufferLength, out _);
-                    bufferLength *= 2;
-                } while (hresult == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER);
 
-                if (hresult == NativeMethodsShared.S_OK)
-                {
-                    return runtimeVersion.ToString();
-                }
-                else
+                unsafe
                 {
-                    return String.Empty;
-                }
+                    // Allocate an initial buffer 
+                    char* runtimeVersionInitial = stackalloc char[bufferLength];
+
+                    // Run GetFileVersion, this should succeed using the initial buffer.
+                    // It also returns the dwLength which is used if there is insufficient buffer.
+                    uint hresult = NativeMethods.GetFileVersion(path, runtimeVersionInitial, bufferLength, out int dwLength);
+
+                    if (hresult == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER)
+                    {
+                        // Allocate new buffer based on the returned length.
+                        char* runtimeVersion = stackalloc char[dwLength];
+
+                        // Get the RuntimeVersion in this second call.
+                        bufferLength = dwLength;
+                        hresult = NativeMethods.GetFileVersion(path, runtimeVersion, bufferLength, out dwLength);
+                        return hresult == NativeMethodsShared.S_OK ? new string(runtimeVersion, 0, dwLength - 1) : string.Empty;
+                    }
+
+                    return hresult == NativeMethodsShared.S_OK ? new string(runtimeVersionInitial, 0, dwLength - 1) : string.Empty;
+                }                
             }
             else
             {
@@ -601,14 +607,14 @@ internal static string GetRuntimeVersion(string path)
 #else
                 return ManagedRuntimeVersionReader.GetRuntimeVersion(path);
 #endif
-        }
+                }
 
 
-        /// <summary>
-        /// Import assembly dependencies.
-        /// </summary>
-        /// <returns>The array of assembly dependencies.</returns>
-        private AssemblyNameExtension[] ImportAssemblyDependencies()
+                /// <summary>
+                /// Import assembly dependencies.
+                /// </summary>
+                /// <returns>The array of assembly dependencies.</returns>
+                private AssemblyNameExtension[] ImportAssemblyDependencies()
         {
 #if !FEATURE_ASSEMBLYLOADCONTEXT
             var asmRefs = new List<AssemblyNameExtension>();
@@ -790,7 +796,7 @@ private static AssemblyNameExtension ConstructAssemblyName(IntPtr asmMetaPtr, ch
             // Construct the assembly name. (Note asmNameLength should/must be > 0.)
             var assemblyName = new AssemblyName
             {
-                Name = new string(asmNameBuf, 0, (int) asmNameLength - 1),
+                Name = new string(asmNameBuf, 0, (int)asmNameLength - 1),
                 Version = new Version(
                     asmMeta.usMajorVersion,
                     asmMeta.usMinorVersion,
@@ -911,7 +917,7 @@ public static string GetRuntimeVersion(string path)
                     // Read the PE header signature
 
                     sr.BaseStream.Position = peHeaderOffset;
-                    if (!ReadBytes(sr, (byte) 'P', (byte) 'E', 0, 0))
+                    if (!ReadBytes(sr, (byte)'P', (byte)'E', 0, 0))
                     {
                         return string.Empty;
                     }

src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs

@@ -367,16 +367,18 @@ bool specificVersion
         }
 
         /// <summary>
-        /// Return the root path of the GAC
+        /// Return the root path of the GAC.
         /// </summary>
         internal static string GetGacPath()
         {
             int gacPathLength = 0;
-            NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, null, ref gacPathLength);
-            StringBuilder gacPath = new StringBuilder(gacPathLength);
-            NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, gacPath, ref gacPathLength);
-
-            return gacPath.ToString();
+            unsafe
+            {
+                NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, null, ref gacPathLength);
+                char* gacPath = stackalloc char[gacPathLength];
+                NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, gacPath, ref gacPathLength);
+                return new string(gacPath, 0, gacPathLength - 1);
+            }
         }
     }
 }

src/Tasks/ComReference.cs

@@ -407,21 +407,32 @@ internal static string StripTypeLibNumberFromPath(string typeLibPath, FileExists
         private static string GetModuleFileName(IntPtr handle)
         {
             bool success = false;
-            var buffer = new StringBuilder();
+            char[] buffer = null;
+            string output = string.Empty;
 
             // Try increased buffer sizes if on longpath-enabled Windows
             for (int bufferSize = NativeMethodsShared.MAX_PATH; !success && bufferSize <= NativeMethodsShared.MaxPath; bufferSize *= 2)
             {
-                buffer.EnsureCapacity(bufferSize);
-
-                var handleRef = new System.Runtime.InteropServices.HandleRef(buffer, handle);
-                int pathLength = NativeMethodsShared.GetModuleFileName(handleRef, buffer, buffer.Capacity);
+                buffer = System.Buffers.ArrayPool<char>.Shared.Rent(bufferSize);
+                try
+                {
+                    var handleRef = new System.Runtime.InteropServices.HandleRef(buffer, handle);
+                    int pathLength = NativeMethodsShared.GetModuleFileName(handleRef, buffer, bufferSize);
 
-                bool isBufferTooSmall = ((uint)Marshal.GetLastWin32Error() == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER);
-                success = pathLength != 0 && !isBufferTooSmall;
+                    bool isBufferTooSmall = (uint)Marshal.GetLastWin32Error() == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER;
+                    success = pathLength != 0 && !isBufferTooSmall;
+                    if (success)
+                    {
+                        output = new string(buffer, 0, pathLength);
+                    }
+                }
+                finally
+                {
+                    System.Buffers.ArrayPool<char>.Shared.Return(buffer);
+                }
             }
 
-            return success ? buffer.ToString() : string.Empty;
+            return output;
         }
 
         /// <summary>

src/Tasks/LockCheck.cs

@@ -56,8 +56,10 @@ internal enum ApplicationType
             string[] rgsServiceNames);
 
         [DllImport(RestartManagerDll, CharSet = CharSet.Unicode)]
-        private static extern int RmStartSession(out uint pSessionHandle,
-            int dwSessionFlags, StringBuilder strSessionKey);
+        private static extern unsafe int RmStartSession(
+            out uint pSessionHandle,
+            int dwSessionFlags,
+            char* strSessionKey);
 
         [DllImport(RestartManagerDll)]
         private static extern int RmEndSession(uint pSessionHandle);
@@ -207,11 +209,16 @@ internal static IEnumerable<ProcessInfo> GetLockingProcessInfos(params string[]
             }
 
             const int maxRetries = 6;
+            uint handle;
+            int res;
 
-            // See http://blogs.msdn.com/b/oldnewthing/archive/2012/02/17/10268840.aspx.
-            var key = new StringBuilder(new string('\0', CCH_RM_SESSION_KEY + 1));
+            unsafe
+            {
+                // See http://blogs.msdn.com/b/oldnewthing/archive/2012/02/17/10268840.aspx.
+                char* key = stackalloc char[CCH_RM_SESSION_KEY + 1];
+                res = RmStartSession(out handle, 0, key);
+            }
 
-            int res = RmStartSession(out uint handle, 0, key);
             if (res != 0)
             {
                 throw GetException(res, "RmStartSession", "Failed to begin restart manager session.");

src/Tasks/NativeMethods.cs

@@ -1058,8 +1058,12 @@ internal static bool AllDrivesMapped()
         /// and then again to pass the client-allocated character buffer. StringBuilder is the most straightforward way
         /// to allocate a mutable buffer of characters and pass it around.
         /// </summary>
+        /// <param name="cacheFlags">Value that indicates the source of the cached assembly.</param>
+        /// <param name="cachePath">The returned pointer to the path.</param>
+        /// <param name="pcchPath">The requested maximum length of CachePath, and upon return, the actual length of CachePath.</param>
+        /// 
         [DllImport("fusion.dll", CharSet = CharSet.Unicode)]
-        internal static extern int GetCachePath(AssemblyCacheFlags cacheFlags, StringBuilder cachePath, ref int pcchPath);
+        internal static extern unsafe int GetCachePath(AssemblyCacheFlags cacheFlags, [Out] char* cachePath, ref int pcchPath);
 #endif
 
         //------------------------------------------------------------------------------
@@ -1125,15 +1129,15 @@ internal static bool AllDrivesMapped()
 
 #if FEATURE_MSCOREE
         /// <summary>
-        /// Get the runtime version for a given file
+        /// Get the runtime version for a given file.
         /// </summary>
-        /// <param name="szFullPath">The path of the file to be examined</param>
+        /// <param name="szFileName">The path of the file to be examined.</param>
         /// <param name="szBuffer">The buffer allocated for the version information that is returned.</param>
-        /// <param name="cchBuffer">The size, in wide characters, of szBuffer</param>
+        /// <param name="cchBuffer">The size, in wide characters, of szBuffer.</param>
         /// <param name="dwLength">The size, in bytes, of the returned szBuffer.</param>
-        /// <returns>HResult</returns>
+        /// <returns>HResult.</returns>
         [DllImport(MscoreeDLL, SetLastError = true, CharSet = CharSet.Unicode)]
-        internal static extern uint GetFileVersion(String szFullPath, StringBuilder szBuffer, int cchBuffer, out uint dwLength);
+        internal static extern unsafe uint GetFileVersion([MarshalAs(UnmanagedType.LPWStr)] string szFileName, [Out] char* szBuffer, int cchBuffer, out int dwLength);
 #endif
 #endregion