From 8ce1a96f2d6c6fefdf6cdd80a1afdbe1e9d531eb Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 18:00:51 +0100 Subject: [PATCH 01/18] Reduce Kernel32 calls --- .../src/System/IO/FileSystem.Attributes.Windows.cs | 9 +++++++++ .../System/IO/FileSystem.DirectoryCreation.Windows.cs | 10 +++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index dab4cf7a8f6c5c..5090d0d3933401 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -40,6 +40,15 @@ public static bool FileExists(string fullPath) ((data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY) == 0); } + public static (int fileAttributes, int lastError) GetFileAttributes(string fullPath) + { + Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default; + int lastError = FillAttributeInfo(fullPath, ref data, returnErrorOnNotFound: true); + int fileAttributes = data.dwFileAttributes; + + return (fileAttributes, lastError); + } + /// /// Returns 0 on success, otherwise a Win32 error code. Note that /// classes should use -1 as the uninitialized state for dataInitialized. diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index dba07a8fb41567..66fe92307fb80a 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -106,8 +106,16 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr } else { + (int fileAttributes, int lastError) = GetFileAttributes(name); + currentError = lastError; + + int directoryAttributeValue = fileAttributes & + Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY; + bool fileExists = lastError == Interop.Errors.ERROR_SUCCESS && directoryAttributeValue != 0; + bool directoryExists = directoryAttributeValue == 0; + // If there's a file in this directory's place, or if we have ERROR_ACCESS_DENIED when checking if the directory already exists throw. - if (FileExists(name) || (!DirectoryExists(name, out currentError) && currentError == Interop.Errors.ERROR_ACCESS_DENIED)) + if (fileExists || !directoryExists && currentError == Interop.Errors.ERROR_ACCESS_DENIED) { firstError = currentError; errorString = name; From de9e3a986aef16d4b8e88e4dc9a2407bd187e2c9 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 18:01:12 +0100 Subject: [PATCH 02/18] Eliminate unnessecary length check --- .../src/System/IO/FileSystem.DirectoryCreation.Windows.cs | 2 +- src/libraries/Common/src/System/IO/PathInternal.cs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 66fe92307fb80a..333958040e6f22 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -37,7 +37,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr int length = fullPath.Length; // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && PathInternal.EndsInDirectorySeparator(fullPath.AsSpan())) + if (length >= 2 && PathInternal.EndsInDirectorySeparatorUnchecked(fullPath.AsSpan())) { length--; } diff --git a/src/libraries/Common/src/System/IO/PathInternal.cs b/src/libraries/Common/src/System/IO/PathInternal.cs index 52aaa59072dd7b..47944527fd4e1b 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.cs @@ -241,6 +241,12 @@ internal static ReadOnlySpan TrimEndingDirectorySeparator(ReadOnlySpan path) => path.Length > 0 && IsDirectorySeparator(path[path.Length - 1]); + /// + /// Same as but without length check. + /// + internal static bool EndsInDirectorySeparatorUnchecked(ReadOnlySpan path) => + IsDirectorySeparator(path[^1]); + internal static string GetLinkTargetFullPath(string path, string pathToTarget) => IsPartiallyQualified(pathToTarget.AsSpan()) ? Path.Join(Path.GetDirectoryName(path.AsSpan()), pathToTarget.AsSpan()) : pathToTarget; From a0b53884284c9d89b97798debeff566e445e2c07 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 18:32:41 +0100 Subject: [PATCH 03/18] Restrict EnsureExtendedPrefix call to paths longer than `MaxShortDirectoryPath` --- .../Interop/Windows/Kernel32/Interop.CreateDirectory.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs index c679fbbb023dbd..c20a54c6e48d86 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs @@ -20,8 +20,12 @@ private static partial bool CreateDirectoryPrivate( internal static bool CreateDirectory(string path, ref SECURITY_ATTRIBUTES lpSecurityAttributes) { - // We always want to add for CreateDirectory to get around the legacy 248 character limitation - path = PathInternal.EnsureExtendedPrefix(path); + // If length is greater than `MaxShortDirectoryPath` we add a extended prefix to get around the legacy character limitation + if (path.Length > PathInternal.MaxShortDirectoryPath) + { + path = PathInternal.EnsureExtendedPrefix(path); + } + return CreateDirectoryPrivate(path, ref lpSecurityAttributes); } } From e43967ada1c2a5019becc51b9b602180c2e1b853 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 18:45:51 +0100 Subject: [PATCH 04/18] Use `EnsureExtendedPrefixIfNeeded` instead of conditional `EnsureExtendedPrefix` --- .../src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs index c20a54c6e48d86..b0a75243048b7d 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.Win32.SafeHandles; using System.IO; using System.Runtime.InteropServices; @@ -21,10 +20,7 @@ private static partial bool CreateDirectoryPrivate( internal static bool CreateDirectory(string path, ref SECURITY_ATTRIBUTES lpSecurityAttributes) { // If length is greater than `MaxShortDirectoryPath` we add a extended prefix to get around the legacy character limitation - if (path.Length > PathInternal.MaxShortDirectoryPath) - { - path = PathInternal.EnsureExtendedPrefix(path); - } + path = PathInternal.EnsureExtendedPrefixIfNeeded(path); return CreateDirectoryPrivate(path, ref lpSecurityAttributes); } From d6b06b304d2ef3302f44917ad032c20a7b9bc994 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 19:35:24 +0100 Subject: [PATCH 05/18] Slim implementation of FillAttributeInfo to reduce range checks --- .../IO/FileSystem.Attributes.Windows.cs | 93 ++++++++++++------- .../FileSystem.DirectoryCreation.Windows.cs | 39 ++++---- 2 files changed, 79 insertions(+), 53 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index 5090d0d3933401..f4db29a9392946 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -29,6 +29,17 @@ private static bool DirectoryExists(string? path, out int lastError) ((data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY) != 0); } + private static bool DirectoryExistsSlim(string? path, out int lastError) + { + Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default; + lastError = FillAttributeInfoSlim(path, ref data, returnErrorOnNotFound: true); + + return + (lastError == 0) && + (data.dwFileAttributes != -1) && + ((data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY) != 0); + } + public static bool FileExists(string fullPath) { Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default; @@ -58,53 +69,63 @@ public static (int fileAttributes, int lastError) GetFileAttributes(string fullP /// Return the error code for not found errors? internal static int FillAttributeInfo(string? path, ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data, bool returnErrorOnNotFound) { - int errorCode = Interop.Errors.ERROR_SUCCESS; - // Neither GetFileAttributes or FindFirstFile like trailing separators path = PathInternal.TrimEndingDirectorySeparator(path); using (DisableMediaInsertionPrompt.Create()) { - if (!Interop.Kernel32.GetFileAttributesEx(path, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) - { - errorCode = Marshal.GetLastWin32Error(); + return FillAttributeInfoSlim(path, ref data, returnErrorOnNotFound); + } + } - if (!IsPathUnreachableError(errorCode)) + /// + /// Same as without separator-trimming and media-insertion blocking + /// + /// The file path from which the file attribute information will be filled. + /// A struct that will contain the attribute information. + /// Return the error code for not found errors? + internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data, bool returnErrorOnNotFound) + { + int errorCode = Interop.Errors.ERROR_SUCCESS; + + if (!Interop.Kernel32.GetFileAttributesEx(path, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) + { + errorCode = Marshal.GetLastWin32Error(); + + Interop.Kernel32.WIN32_FIND_DATA findData = default; + if (!IsPathUnreachableError(errorCode)) + { + // Assert so we can track down other cases (if any) to add to our test suite + Debug.Assert(errorCode is Interop.Errors.ERROR_ACCESS_DENIED or Interop.Errors.ERROR_SHARING_VIOLATION or Interop.Errors.ERROR_SEM_TIMEOUT, + $"Unexpected error code getting attributes {errorCode} from path {path}"); + + // Files that are marked for deletion will not let you GetFileAttributes, + // ERROR_ACCESS_DENIED is given back without filling out the data struct. + // FindFirstFile, however, will. Historically we always gave back attributes + // for marked-for-deletion files. + // + // Another case where enumeration works is with special system files such as + // pagefile.sys that give back ERROR_SHARING_VIOLATION on GetAttributes. + // + // Ideally we'd only try again for known cases due to the potential performance + // hit. The last attempt to do so baked for nearly a year before we found the + // pagefile.sys case. As such we're probably stuck filtering out specific + // cases that we know we don't want to retry on. + + using SafeFindHandle handle = Interop.Kernel32.FindFirstFile(path!, ref findData); + if (handle.IsInvalid) { - // Assert so we can track down other cases (if any) to add to our test suite - Debug.Assert(errorCode == Interop.Errors.ERROR_ACCESS_DENIED || errorCode == Interop.Errors.ERROR_SHARING_VIOLATION || errorCode == Interop.Errors.ERROR_SEM_TIMEOUT, - $"Unexpected error code getting attributes {errorCode} from path {path}"); - - // Files that are marked for deletion will not let you GetFileAttributes, - // ERROR_ACCESS_DENIED is given back without filling out the data struct. - // FindFirstFile, however, will. Historically we always gave back attributes - // for marked-for-deletion files. - // - // Another case where enumeration works is with special system files such as - // pagefile.sys that give back ERROR_SHARING_VIOLATION on GetAttributes. - // - // Ideally we'd only try again for known cases due to the potential performance - // hit. The last attempt to do so baked for nearly a year before we found the - // pagefile.sys case. As such we're probably stuck filtering out specific - // cases that we know we don't want to retry on. - - Interop.Kernel32.WIN32_FIND_DATA findData = default; - using (SafeFindHandle handle = Interop.Kernel32.FindFirstFile(path!, ref findData)) - { - if (handle.IsInvalid) - { - errorCode = Marshal.GetLastWin32Error(); - } - else - { - errorCode = Interop.Errors.ERROR_SUCCESS; - data.PopulateFrom(ref findData); - } - } + errorCode = Marshal.GetLastWin32Error(); + } + else + { + errorCode = Interop.Errors.ERROR_SUCCESS; + data.PopulateFrom(ref findData); } } } + if (errorCode != Interop.Errors.ERROR_SUCCESS && !returnErrorOnNotFound) { switch (errorCode) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 333958040e6f22..4dc252e5234bdc 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -44,29 +44,34 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr int lengthRoot = PathInternal.GetRootLength(fullPath.AsSpan()); - if (length > lengthRoot) + using (DisableMediaInsertionPrompt.Create()) { - // Special case root (fullpath = X:\\) - int i = length - 1; - while (i >= lengthRoot && !somepathexists) + if (length > lengthRoot) { - string dir = fullPath.Substring(0, i + 1); - - if (!DirectoryExists(dir)) // Create only the ones missing - { - stackDir.Add(dir); - } - else + // Special case root (fullpath = X:\\) + int i = length - 1; + while (i >= lengthRoot && !somepathexists) { - somepathexists = true; - } + string dir = fullPath[..(i + 1)]; + // Neither GetFileAttributes or FindFirstFile like trailing separators + dir = PathInternal.TrimEndingDirectorySeparator(dir); + + if (!DirectoryExistsSlim(dir, out _)) // Create only the ones missing + { + stackDir.Add(dir); + } + else + { + somepathexists = true; + } + + while (i > lengthRoot && !PathInternal.IsDirectorySeparator(fullPath[i])) + { + i--; + } - while (i > lengthRoot && !PathInternal.IsDirectorySeparator(fullPath[i])) - { i--; } - - i--; } } From 31961a456d94ebcb9b6ffdee55cb3e41f5c9c0d1 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 19:44:08 +0100 Subject: [PATCH 06/18] Put `stackDir.Count - 1` into a local to improve codegen --- .../src/System/IO/FileSystem.DirectoryCreation.Windows.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 4dc252e5234bdc..cafff248c2bf0d 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -90,8 +90,9 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr while (stackDir.Count > 0) { - string name = stackDir[stackDir.Count - 1]; - stackDir.RemoveAt(stackDir.Count - 1); + int index = stackDir.Count - 1; + string name = stackDir[index]; + stackDir.RemoveAt(index); r = Interop.Kernel32.CreateDirectory(name, ref secAttrs); if (!r && (firstError == 0)) From 12d7729caaf2d41da368c6e9d4232ce59349ff8d Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 20:06:01 +0100 Subject: [PATCH 07/18] Use down-counting loop instead of List-RemoveAt construct --- .../src/System/IO/FileSystem.DirectoryCreation.Windows.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index cafff248c2bf0d..8292362a2433b7 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -88,11 +88,9 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - while (stackDir.Count > 0) + for (int i = count - 1; 0 <= i; i--) { - int index = stackDir.Count - 1; - string name = stackDir[index]; - stackDir.RemoveAt(index); + string name = stackDir[i]; r = Interop.Kernel32.CreateDirectory(name, ref secAttrs); if (!r && (firstError == 0)) From 6faa0e15b86d26e1ac912536b091d68ac3385247 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 20:23:10 +0100 Subject: [PATCH 08/18] Spannify strings --- .../FileSystem.DirectoryCreation.Windows.cs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 8292362a2433b7..be66cadf0f587c 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -34,15 +34,17 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr // isn't threadsafe. bool somepathexists = false; - int length = fullPath.Length; + + ReadOnlySpan fullPathSpan = fullPath; + int length = fullPathSpan.Length; // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && PathInternal.EndsInDirectorySeparatorUnchecked(fullPath.AsSpan())) + if (length >= 2 && PathInternal.EndsInDirectorySeparatorUnchecked(fullPathSpan)) { length--; } - int lengthRoot = PathInternal.GetRootLength(fullPath.AsSpan()); + int lengthRoot = PathInternal.GetRootLength(fullPathSpan); using (DisableMediaInsertionPrompt.Create()) { @@ -52,13 +54,16 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr int i = length - 1; while (i >= lengthRoot && !somepathexists) { - string dir = fullPath[..(i + 1)]; + ReadOnlySpan dir = fullPathSpan[..(i + 1)]; + // Neither GetFileAttributes or FindFirstFile like trailing separators dir = PathInternal.TrimEndingDirectorySeparator(dir); - if (!DirectoryExistsSlim(dir, out _)) // Create only the ones missing + string dirString = new(dir); + + if (!DirectoryExistsSlim(dirString, out _)) // Create only the ones missing { - stackDir.Add(dir); + stackDir.Add(dirString); } else { @@ -133,11 +138,12 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr // Handle CreateDirectory("X:\\") when X: doesn't exist. Similarly for n/w paths. if ((count == 0) && !somepathexists) { - string? root = Path.GetPathRoot(fullPath); + ReadOnlySpan root = Path.GetPathRoot(fullPathSpan); + string rootString = new(root); - if (!DirectoryExists(root)) + if (!DirectoryExists(rootString)) { - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, root); + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, rootString); } return; From bf0e7151d24169f9d043ad49ad9f537079337cd2 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Wed, 16 Mar 2022 20:43:19 +0100 Subject: [PATCH 09/18] Reduce range checks in interop calls --- .../Windows/Kernel32/Interop.FindFirstFileEx.cs | 5 +++++ .../Kernel32/Interop.GetFileAttributesEx.cs | 4 ++++ .../src/System/IO/FileSystem.Attributes.Windows.cs | 5 +++-- .../Common/src/System/IO/PathInternal.Windows.cs | 14 +++++++------- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs index 0095c09cdac3a4..d06dae7ea4fc76 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs @@ -20,6 +20,11 @@ internal static SafeFindHandle FindFirstFile(string fileName, ref WIN32_FIND_DAT { fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); + // use FindExInfoBasic since we don't care about short name and it has better perf + return FindFirstFilePrefixed(fileName, ref data); + } + internal static SafeFindHandle FindFirstFilePrefixed(string fileName, ref WIN32_FIND_DATA data) + { // use FindExInfoBasic since we don't care about short name and it has better perf return FindFirstFileExPrivate(fileName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0); } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs index 59f6d4992e89f0..e1cfd3d6763db9 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs @@ -21,6 +21,10 @@ private static partial bool GetFileAttributesExPrivate( internal static bool GetFileAttributesEx(string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation) { name = PathInternal.EnsureExtendedPrefixIfNeeded(name); + return GetFileAttributesExPrefixed(name, fileInfoLevel, ref lpFileInformation); + } + internal static bool GetFileAttributesExPrefixed(string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation) + { return GetFileAttributesExPrivate(name, fileInfoLevel, ref lpFileInformation); } } diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index f4db29a9392946..2c393b485fb92b 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -87,8 +87,9 @@ internal static int FillAttributeInfo(string? path, ref Interop.Kernel32.WIN32_F internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data, bool returnErrorOnNotFound) { int errorCode = Interop.Errors.ERROR_SUCCESS; + string? prefixedString = PathInternal.EnsureExtendedPrefixIfNeeded(path); - if (!Interop.Kernel32.GetFileAttributesEx(path, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) + if (!Interop.Kernel32.GetFileAttributesExPrefixed(prefixedString, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) { errorCode = Marshal.GetLastWin32Error(); @@ -112,7 +113,7 @@ internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN // pagefile.sys case. As such we're probably stuck filtering out specific // cases that we know we don't want to retry on. - using SafeFindHandle handle = Interop.Kernel32.FindFirstFile(path!, ref findData); + using SafeFindHandle handle = Interop.Kernel32.FindFirstFilePrefixed(prefixedString!, ref findData); if (handle.IsInvalid) { errorCode = Marshal.GetLastWin32Error(); diff --git a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs index 48fb35304cd7d4..40cac5c2800002 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs @@ -72,13 +72,13 @@ internal static bool IsValidDriveChar(char value) return (value >= 'A' && value <= 'Z') || (value >= 'a' && value <= 'z'); } - internal static bool EndsWithPeriodOrSpace(string? path) - { - if (string.IsNullOrEmpty(path)) - return false; + internal static bool EndsWithPeriodOrSpace(string? path) => + !string.IsNullOrEmpty(path) && EndsWithPeriodOrSpaceSlim(path); - char c = path[path.Length - 1]; - return c == ' ' || c == '.'; + internal static bool EndsWithPeriodOrSpaceSlim(string path) + { + char c = path[^1]; + return c is ' ' or '.'; } /// @@ -91,7 +91,7 @@ internal static bool EndsWithPeriodOrSpace(string? path) [return: NotNullIfNotNull("path")] internal static string? EnsureExtendedPrefixIfNeeded(string? path) { - if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpace(path))) + if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpaceSlim(path))) { return EnsureExtendedPrefix(path); } From 7fc97aa9af2695584b04d14855331bd6596ec754 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Sun, 20 Mar 2022 15:43:20 +0100 Subject: [PATCH 10/18] Replace modern language with legacy language to fix netfx builds --- src/libraries/Common/src/System/IO/PathInternal.Windows.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs index 40cac5c2800002..c355b75cfab9b9 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs @@ -77,8 +77,8 @@ internal static bool EndsWithPeriodOrSpace(string? path) => internal static bool EndsWithPeriodOrSpaceSlim(string path) { - char c = path[^1]; - return c is ' ' or '.'; + char c = path[path.Length - 1]; + return c == ' ' || c == '.'; } /// From 920bf49702f19323a8216756c1d5e3156214b483 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Sun, 20 Mar 2022 17:16:16 +0100 Subject: [PATCH 11/18] Apply suggestions --- .../Common/src/System/IO/FileSystem.Attributes.Windows.cs | 6 +++--- .../src/System/IO/FileSystem.DirectoryCreation.Windows.cs | 7 ++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index 2c393b485fb92b..9774a88efc62b9 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -51,13 +51,13 @@ public static bool FileExists(string fullPath) ((data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY) == 0); } - public static (int fileAttributes, int lastError) GetFileAttributes(string fullPath) + public static (bool fileExists, bool directoryExists, int lastError) GetFileAttributes(string fullPath) { Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default; int lastError = FillAttributeInfo(fullPath, ref data, returnErrorOnNotFound: true); - int fileAttributes = data.dwFileAttributes; + int directoryAttributeValue = data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY; - return (fileAttributes, lastError); + return (directoryAttributeValue != 0, directoryAttributeValue == 0, lastError); } /// diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index be66cadf0f587c..ad00e79158d253 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -115,13 +115,10 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr } else { - (int fileAttributes, int lastError) = GetFileAttributes(name); + (bool fileExists, bool directoryExists, int lastError) = GetFileAttributes(name); currentError = lastError; - int directoryAttributeValue = fileAttributes & - Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY; - bool fileExists = lastError == Interop.Errors.ERROR_SUCCESS && directoryAttributeValue != 0; - bool directoryExists = directoryAttributeValue == 0; + fileExists = lastError == Interop.Errors.ERROR_SUCCESS && fileExists; // If there's a file in this directory's place, or if we have ERROR_ACCESS_DENIED when checking if the directory already exists throw. if (fileExists || !directoryExists && currentError == Interop.Errors.ERROR_ACCESS_DENIED) From 8cfca8641f11d052c8c81a1cb3a415104c59dd13 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Sun, 20 Mar 2022 17:33:55 +0100 Subject: [PATCH 12/18] Revert "Reduce Kernel32 calls" because of breaking changes This reverts commit 8ce1a96f2d6c6fefdf6cdd80a1afdbe1e9d531eb. --- .../src/System/IO/FileSystem.Attributes.Windows.cs | 9 --------- .../System/IO/FileSystem.DirectoryCreation.Windows.cs | 7 +------ 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index 9774a88efc62b9..b58b918dcdb6f9 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -51,15 +51,6 @@ public static bool FileExists(string fullPath) ((data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY) == 0); } - public static (bool fileExists, bool directoryExists, int lastError) GetFileAttributes(string fullPath) - { - Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default; - int lastError = FillAttributeInfo(fullPath, ref data, returnErrorOnNotFound: true); - int directoryAttributeValue = data.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_DIRECTORY; - - return (directoryAttributeValue != 0, directoryAttributeValue == 0, lastError); - } - /// /// Returns 0 on success, otherwise a Win32 error code. Note that /// classes should use -1 as the uninitialized state for dataInitialized. diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index ad00e79158d253..7b2ea8b410b8e0 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -115,13 +115,8 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr } else { - (bool fileExists, bool directoryExists, int lastError) = GetFileAttributes(name); - currentError = lastError; - - fileExists = lastError == Interop.Errors.ERROR_SUCCESS && fileExists; - // If there's a file in this directory's place, or if we have ERROR_ACCESS_DENIED when checking if the directory already exists throw. - if (fileExists || !directoryExists && currentError == Interop.Errors.ERROR_ACCESS_DENIED) + if (FileExists(name) || (!DirectoryExists(name, out currentError) && currentError == Interop.Errors.ERROR_ACCESS_DENIED)) { firstError = currentError; errorString = name; From b9e4754af46bfcb6c2471f3210f5b427ce92e0da Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Sun, 20 Mar 2022 17:34:57 +0100 Subject: [PATCH 13/18] Remove "yoda conditions" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Günther Foidl <5755834+gfoidl@users.noreply.github.com> --- .../src/System/IO/FileSystem.DirectoryCreation.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 7b2ea8b410b8e0..fe3a65df658ade 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -93,7 +93,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - for (int i = count - 1; 0 <= i; i--) + for (int i = count - 1; i >= 0; i--) { string name = stackDir[i]; From 8fb6ecb262bc2890f2ce65748661d5cc06c9ca16 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Tue, 22 Mar 2022 20:47:41 +0100 Subject: [PATCH 14/18] Add newlines --- .../src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs | 1 + .../src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs index d06dae7ea4fc76..ef3c91dbf9cb8b 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs @@ -23,6 +23,7 @@ internal static SafeFindHandle FindFirstFile(string fileName, ref WIN32_FIND_DAT // use FindExInfoBasic since we don't care about short name and it has better perf return FindFirstFilePrefixed(fileName, ref data); } + internal static SafeFindHandle FindFirstFilePrefixed(string fileName, ref WIN32_FIND_DATA data) { // use FindExInfoBasic since we don't care about short name and it has better perf diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs index e1cfd3d6763db9..ddbdc13879e71c 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs @@ -23,6 +23,7 @@ internal static bool GetFileAttributesEx(string? name, GET_FILEEX_INFO_LEVELS fi name = PathInternal.EnsureExtendedPrefixIfNeeded(name); return GetFileAttributesExPrefixed(name, fileInfoLevel, ref lpFileInformation); } + internal static bool GetFileAttributesExPrefixed(string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation) { return GetFileAttributesExPrivate(name, fileInfoLevel, ref lpFileInformation); From 5bc0806b44c27e2a196db922c56a27abd48ccd48 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Tue, 22 Mar 2022 20:52:47 +0100 Subject: [PATCH 15/18] Inline `EndsInDirectorySeparator` /cc @danmoseley --- .../IO/FileSystem.DirectoryCreation.Windows.cs | 2 +- .../Common/src/System/IO/PathInternal.cs | 16 ++-------------- .../System.Private.CoreLib/src/System/IO/Path.cs | 2 +- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index fe3a65df658ade..95611375b41242 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -39,7 +39,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr int length = fullPathSpan.Length; // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && PathInternal.EndsInDirectorySeparatorUnchecked(fullPathSpan)) + if (length >= 2 && PathInternal.IsDirectorySeparator(fullPathSpan[^1])) { length--; } diff --git a/src/libraries/Common/src/System/IO/PathInternal.cs b/src/libraries/Common/src/System/IO/PathInternal.cs index 47944527fd4e1b..a389dde7117340 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.cs @@ -16,7 +16,7 @@ internal static partial class PathInternal internal static bool StartsWithDirectorySeparator(ReadOnlySpan path) => path.Length > 0 && IsDirectorySeparator(path[0]); internal static string EnsureTrailingSeparator(string path) - => EndsInDirectorySeparator(path.AsSpan()) ? path : path + DirectorySeparatorCharAsString; + => path.Length > 0 && IsDirectorySeparator(path[path.Length - 1]) ? path : path + DirectorySeparatorCharAsString; internal static bool IsRoot(ReadOnlySpan path) => path.Length == GetRootLength(path); @@ -231,22 +231,10 @@ internal static bool EndsInDirectorySeparator(string? path) => /// Trims one trailing directory separator beyond the root of the path. /// internal static ReadOnlySpan TrimEndingDirectorySeparator(ReadOnlySpan path) => - EndsInDirectorySeparator(path) && !IsRoot(path) ? + path.Length > 0 && IsDirectorySeparator(path[path.Length - 1]) && !IsRoot(path) ? path.Slice(0, path.Length - 1) : path; - /// - /// Returns true if the path ends in a directory separator. - /// - internal static bool EndsInDirectorySeparator(ReadOnlySpan path) => - path.Length > 0 && IsDirectorySeparator(path[path.Length - 1]); - - /// - /// Same as but without length check. - /// - internal static bool EndsInDirectorySeparatorUnchecked(ReadOnlySpan path) => - IsDirectorySeparator(path[^1]); - internal static string GetLinkTargetFullPath(string path, string pathToTarget) => IsPartiallyQualified(pathToTarget.AsSpan()) ? Path.Join(Path.GetDirectoryName(path.AsSpan()), pathToTarget.AsSpan()) : pathToTarget; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs index 2afe0d303d1b47..aa766a98d53418 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs @@ -942,7 +942,7 @@ private static string GetRelativePath(string relativeTo!!, string path!!, String /// /// Returns true if the path ends in a directory separator. /// - public static bool EndsInDirectorySeparator(ReadOnlySpan path) => PathInternal.EndsInDirectorySeparator(path); + public static bool EndsInDirectorySeparator(ReadOnlySpan path) => path.Length > 0 && PathInternal.IsDirectorySeparator(path[path.Length - 1]); /// /// Returns true if the path ends in a directory separator. From c926642522a3cc9723a7056f4f730ec2a39bf210 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Tue, 22 Mar 2022 20:57:58 +0100 Subject: [PATCH 16/18] Inline `GetFileAttributesExPrefixed` --- .../Windows/Kernel32/Interop.GetFileAttributesEx.cs | 7 +------ .../Common/src/System/IO/FileSystem.Attributes.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs index ddbdc13879e71c..914bef9040a54e 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileAttributesEx.cs @@ -13,7 +13,7 @@ internal static partial class Kernel32 /// [LibraryImport(Libraries.Kernel32, EntryPoint = "GetFileAttributesExW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool GetFileAttributesExPrivate( + internal static partial bool GetFileAttributesExPrivate( string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation); @@ -21,11 +21,6 @@ private static partial bool GetFileAttributesExPrivate( internal static bool GetFileAttributesEx(string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation) { name = PathInternal.EnsureExtendedPrefixIfNeeded(name); - return GetFileAttributesExPrefixed(name, fileInfoLevel, ref lpFileInformation); - } - - internal static bool GetFileAttributesExPrefixed(string? name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation) - { return GetFileAttributesExPrivate(name, fileInfoLevel, ref lpFileInformation); } } diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index b58b918dcdb6f9..5c2805ae78e31d 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -80,7 +80,7 @@ internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN int errorCode = Interop.Errors.ERROR_SUCCESS; string? prefixedString = PathInternal.EnsureExtendedPrefixIfNeeded(path); - if (!Interop.Kernel32.GetFileAttributesExPrefixed(prefixedString, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) + if (!Interop.Kernel32.GetFileAttributesExPrivate(prefixedString, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data)) { errorCode = Marshal.GetLastWin32Error(); From 61af4d359b2a6af647f9c36ddb592ffdf8b24a89 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Mon, 2 May 2022 20:43:44 +0200 Subject: [PATCH 17/18] Apply suggestions Co-Authored-By: Dan Moseley --- .../Windows/Kernel32/Interop.FindFirstFileEx.cs | 6 ------ .../System/IO/FileSystem.Attributes.Windows.cs | 4 ++-- .../IO/FileSystem.DirectoryCreation.Windows.cs | 17 +++++++---------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs index ef3c91dbf9cb8b..0095c09cdac3a4 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FindFirstFileEx.cs @@ -20,12 +20,6 @@ internal static SafeFindHandle FindFirstFile(string fileName, ref WIN32_FIND_DAT { fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); - // use FindExInfoBasic since we don't care about short name and it has better perf - return FindFirstFilePrefixed(fileName, ref data); - } - - internal static SafeFindHandle FindFirstFilePrefixed(string fileName, ref WIN32_FIND_DATA data) - { // use FindExInfoBasic since we don't care about short name and it has better perf return FindFirstFileExPrivate(fileName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0); } diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index 5c2805ae78e31d..ae1cf616840a19 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -88,7 +88,7 @@ internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN if (!IsPathUnreachableError(errorCode)) { // Assert so we can track down other cases (if any) to add to our test suite - Debug.Assert(errorCode is Interop.Errors.ERROR_ACCESS_DENIED or Interop.Errors.ERROR_SHARING_VIOLATION or Interop.Errors.ERROR_SEM_TIMEOUT, + Debug.Assert(errorCode == Interop.Errors.ERROR_ACCESS_DENIED || errorCode == Interop.Errors.ERROR_SHARING_VIOLATION || errorCode == Interop.Errors.ERROR_SEM_TIMEOUT, $"Unexpected error code getting attributes {errorCode} from path {path}"); // Files that are marked for deletion will not let you GetFileAttributes, @@ -104,7 +104,7 @@ internal static int FillAttributeInfoSlim(string? path, ref Interop.Kernel32.WIN // pagefile.sys case. As such we're probably stuck filtering out specific // cases that we know we don't want to retry on. - using SafeFindHandle handle = Interop.Kernel32.FindFirstFilePrefixed(prefixedString!, ref findData); + using SafeFindHandle handle = Interop.Kernel32.FindFirstFile(prefixedString!, ref findData); if (handle.IsInvalid) { errorCode = Marshal.GetLastWin32Error(); diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index 95611375b41242..9c688358330008 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -34,17 +34,15 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr // isn't threadsafe. bool somepathexists = false; - - ReadOnlySpan fullPathSpan = fullPath; - int length = fullPathSpan.Length; + int length = fullPath.Length; // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && PathInternal.IsDirectorySeparator(fullPathSpan[^1])) + if (length >= 2 && PathInternal.IsDirectorySeparator(fullPath.AsSpan()[^1])) { length--; } - int lengthRoot = PathInternal.GetRootLength(fullPathSpan); + int lengthRoot = PathInternal.GetRootLength(fullPath.AsSpan()); using (DisableMediaInsertionPrompt.Create()) { @@ -54,7 +52,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr int i = length - 1; while (i >= lengthRoot && !somepathexists) { - ReadOnlySpan dir = fullPathSpan[..(i + 1)]; + ReadOnlySpan dir = fullPath.AsSpan()[..(i + 1)]; // Neither GetFileAttributes or FindFirstFile like trailing separators dir = PathInternal.TrimEndingDirectorySeparator(dir); @@ -130,12 +128,11 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr // Handle CreateDirectory("X:\\") when X: doesn't exist. Similarly for n/w paths. if ((count == 0) && !somepathexists) { - ReadOnlySpan root = Path.GetPathRoot(fullPathSpan); - string rootString = new(root); + string? root = Path.GetPathRoot(fullPath); - if (!DirectoryExists(rootString)) + if (!DirectoryExists(root)) { - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, rootString); + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, root); } return; From bc226033ced3734d9006df4cbee5993affc15e76 Mon Sep 17 00:00:00 2001 From: Robin Lindner Date: Mon, 4 Jul 2022 19:47:00 +0200 Subject: [PATCH 18/18] Revert behavior change --- .../Common/src/System/IO/PathInternal.Windows.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs index 0af67211ea57b6..9ee11d0ab85607 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs @@ -72,11 +72,10 @@ internal static bool IsValidDriveChar(char value) return (uint)((value | 0x20) - 'a') <= (uint)('z' - 'a'); } - internal static bool EndsWithPeriodOrSpace(string? path) => - !string.IsNullOrEmpty(path) && EndsWithPeriodOrSpaceSlim(path); - - internal static bool EndsWithPeriodOrSpaceSlim(string path) + internal static bool EndsWithPeriodOrSpace(string path) { + if (string.IsNullOrEmpty(path)) + return false; char c = path[path.Length - 1]; return c == ' ' || c == '.'; } @@ -91,7 +90,7 @@ internal static bool EndsWithPeriodOrSpaceSlim(string path) [return: NotNullIfNotNull("path")] internal static string? EnsureExtendedPrefixIfNeeded(string? path) { - if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpaceSlim(path))) + if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpace(path))) { return EnsureExtendedPrefix(path); }