From 212777a925c75a2386e6784a0b21d51e5ae720bb Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 15:06:46 +0100 Subject: [PATCH 1/7] introduce an overload that accepts ROS instead of a string --- .../src/Interop/Unix/System.Native/Interop.MkDir.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs index 499b1e74597e9..063f57c64185a 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs @@ -3,12 +3,21 @@ using System; using System.Runtime.InteropServices; +using System.Text; internal static partial class Interop { internal static partial class Sys { - [GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_MkDir", CharSet = CharSet.Ansi, SetLastError = true)] - internal static partial int MkDir(string path, int mode); + [GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_MkDir", SetLastError = true)] + private static partial int MkDir(ref byte path, int mode); + + internal static int MkDir(ReadOnlySpan path, int mode) + { + var converter = new ValueUtf8Converter(stackalloc byte[DefaultPathBufferSize]); + int result = MkDir(ref MemoryMarshal.GetReference(converter.ConvertAndTerminateString(path)), mode); + converter.Dispose(); + return result; + } } } From 76bdae5f999a193dd846fa887c28f080f409c5d5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 15:21:46 +0100 Subject: [PATCH 2/7] remove dead code --- .../System.IO.Pipes/src/System.IO.Pipes.csproj | 4 +--- .../src/System/IO/Pipes/PipeStream.Unix.cs | 17 ----------------- 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj b/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj index ae48db64440d4..d597398efb25a 100644 --- a/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj +++ b/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj @@ -137,8 +137,6 @@ Link="Common\Interop\Unix\Interop.GetHostName.cs" /> - - + diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs index 2fa979b222a81..f4f0caca1c562 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs @@ -434,23 +434,6 @@ private SemaphoreSlim EnsureAsyncActiveSemaphoreInitialized() return LazyInitializer.EnsureInitialized(ref _asyncActiveSemaphore, () => new SemaphoreSlim(1, 1)); } - private static void CreateDirectory(string directoryPath) - { - int result = Interop.Sys.MkDir(directoryPath, (int)Interop.Sys.Permissions.Mask); - - // If successful created, we're done. - if (result >= 0) - return; - - // If the directory already exists, consider it a success. - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.EEXIST) - return; - - // Otherwise, fail. - throw Interop.GetExceptionForIoErrno(errorInfo, directoryPath, isDirectory: true); - } - /// Creates an anonymous pipe. /// The resulting reader end of the pipe. /// The resulting writer end of the pipe. From aad24ddade2f14259fbf1a0b84418d601cd5ccf6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 15:22:42 +0100 Subject: [PATCH 3/7] avoid string allocations --- .../src/System/IO/FileSystem.Unix.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index e23ae9e9e28a8..5dd9497e28ce8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -313,9 +313,9 @@ public static void CreateParentsAndDirectory(string fullPath) { // Try create parents bottom to top and track those that could not // be created due to missing parents. Then create them top to bottom. - List stackDir = new List(); + List stackDir = new(); - stackDir.Add(fullPath); + stackDir.Add(fullPath.Length); int i = fullPath.Length - 1; // Trim trailing separator. @@ -333,7 +333,7 @@ public static void CreateParentsAndDirectory(string fullPath) } // Try create it. - string mkdirPath = fullPath.Substring(0, i); + ReadOnlySpan mkdirPath = fullPath.AsSpan(0, i); int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) { @@ -348,7 +348,7 @@ public static void CreateParentsAndDirectory(string fullPath) // We'll try to create its parent on the next iteration. // Track this path for later creation. - stackDir.Add(mkdirPath); + stackDir.Add(mkdirPath.Length); } else if (errorInfo.Error == Interop.Error.EEXIST) { @@ -358,7 +358,7 @@ public static void CreateParentsAndDirectory(string fullPath) } else { - throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString(), isDirectory: true); } i--; } while (i > 0); @@ -366,7 +366,7 @@ public static void CreateParentsAndDirectory(string fullPath) // Create directories that had missing parents. for (i = stackDir.Count - 1; i >= 0; i--) { - string mkdirPath = stackDir[i]; + ReadOnlySpan mkdirPath = fullPath.AsSpan(0, stackDir[i]); int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result < 0) { @@ -386,7 +386,7 @@ public static void CreateParentsAndDirectory(string fullPath) } } - throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString(), isDirectory: true); } } } From 432f276d740b8215a2f8ac62494bcb30cd50921e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 15:31:11 +0100 Subject: [PATCH 4/7] remove List allocation --- .../src/System/IO/FileSystem.Unix.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 5dd9497e28ce8..584e7e3381f5e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -298,10 +298,19 @@ public static void CreateDirectory(string fullPath) { return; // Path already exists and it's a directory. } - else if (errorInfo.Error == Interop.Error.ENOENT) + else if (errorInfo.Error == Interop.Error.ENOENT) // Some parts of the path don't exist yet. { - // Some parts of the path don't exist yet. - CreateParentsAndDirectory(fullPath); + // Try create parents bottom to top and track those that could not + // be created due to missing parents. Then create them top to bottom. + ValueListBuilder stackDir = new(stackalloc int[32]); // 32 arbitrarily chosen + try + { + CreateParentsAndDirectory(fullPath, stackDir); + } + finally + { + stackDir.Dispose(); + } } else { @@ -309,13 +318,9 @@ public static void CreateDirectory(string fullPath) } } - public static void CreateParentsAndDirectory(string fullPath) + public static void CreateParentsAndDirectory(string fullPath, ValueListBuilder stackDir) { - // Try create parents bottom to top and track those that could not - // be created due to missing parents. Then create them top to bottom. - List stackDir = new(); - - stackDir.Add(fullPath.Length); + stackDir.Append(fullPath.Length); int i = fullPath.Length - 1; // Trim trailing separator. @@ -348,7 +353,7 @@ public static void CreateParentsAndDirectory(string fullPath) // We'll try to create its parent on the next iteration. // Track this path for later creation. - stackDir.Add(mkdirPath.Length); + stackDir.Append(mkdirPath.Length); } else if (errorInfo.Error == Interop.Error.EEXIST) { @@ -364,7 +369,7 @@ public static void CreateParentsAndDirectory(string fullPath) } while (i > 0); // Create directories that had missing parents. - for (i = stackDir.Count - 1; i >= 0; i--) + for (i = stackDir.Length - 1; i >= 0; i--) { ReadOnlySpan mkdirPath = fullPath.AsSpan(0, stackDir[i]); int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); From 07559c7dfc20caf0800aa6f495722047ab680e53 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 15:46:27 +0100 Subject: [PATCH 5/7] minor pedantic refactor --- .../src/System/IO/FileSystem.Unix.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 584e7e3381f5e..c94a416dee4a2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -318,16 +318,16 @@ public static void CreateDirectory(string fullPath) } } - public static void CreateParentsAndDirectory(string fullPath, ValueListBuilder stackDir) + private static void CreateParentsAndDirectory(string fullPath, ValueListBuilder stackDir) { stackDir.Append(fullPath.Length); int i = fullPath.Length - 1; - // Trim trailing separator. if (PathInternal.IsDirectorySeparator(fullPath[i])) { - i--; + i--; // Trim trailing separator. } + do { // Find the end of the parent directory. @@ -337,13 +337,11 @@ public static void CreateParentsAndDirectory(string fullPath, ValueListBuilder mkdirPath = fullPath.AsSpan(0, i); int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) { - // Created parent. - break; + break; // Created parent. } Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); From 0183954ad53215f366e411afddefd45f9ae6b2ee Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 18 Nov 2021 16:42:11 +0100 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index c94a416dee4a2..c1aebca178af8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -305,7 +305,7 @@ public static void CreateDirectory(string fullPath) ValueListBuilder stackDir = new(stackalloc int[32]); // 32 arbitrarily chosen try { - CreateParentsAndDirectory(fullPath, stackDir); + CreateParentsAndDirectory(fullPath, ref stackDir); } finally { @@ -318,7 +318,7 @@ public static void CreateDirectory(string fullPath) } } - private static void CreateParentsAndDirectory(string fullPath, ValueListBuilder stackDir) + private static void CreateParentsAndDirectory(string fullPath, ref ValueListBuilder stackDir) { stackDir.Append(fullPath.Length); From ef470259723892e06eea5f3982e948f9ee3b3e87 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 19 Nov 2021 08:46:52 +0100 Subject: [PATCH 7/7] Address code review feedback --- .../Interop/Unix/System.Native/Interop.MkDir.cs | 3 +-- .../src/System/IO/FileSystem.Unix.cs | 17 +++++------------ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs index 063f57c64185a..09a4ae1e3e79a 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkDir.cs @@ -14,9 +14,8 @@ internal static partial class Sys internal static int MkDir(ReadOnlySpan path, int mode) { - var converter = new ValueUtf8Converter(stackalloc byte[DefaultPathBufferSize]); + using ValueUtf8Converter converter = new(stackalloc byte[DefaultPathBufferSize]); int result = MkDir(ref MemoryMarshal.GetReference(converter.ConvertAndTerminateString(path)), mode); - converter.Dispose(); return result; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index c1aebca178af8..e5fd8574f9679 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -300,17 +300,7 @@ public static void CreateDirectory(string fullPath) } else if (errorInfo.Error == Interop.Error.ENOENT) // Some parts of the path don't exist yet. { - // Try create parents bottom to top and track those that could not - // be created due to missing parents. Then create them top to bottom. - ValueListBuilder stackDir = new(stackalloc int[32]); // 32 arbitrarily chosen - try - { - CreateParentsAndDirectory(fullPath, ref stackDir); - } - finally - { - stackDir.Dispose(); - } + CreateParentsAndDirectory(fullPath); } else { @@ -318,8 +308,11 @@ public static void CreateDirectory(string fullPath) } } - private static void CreateParentsAndDirectory(string fullPath, ref ValueListBuilder stackDir) + private static void CreateParentsAndDirectory(string fullPath) { + // Try create parents bottom to top and track those that could not + // be created due to missing parents. Then create them top to bottom. + using ValueListBuilder stackDir = new(stackalloc int[32]); // 32 arbitrarily chosen stackDir.Append(fullPath.Length); int i = fullPath.Length - 1;