Replace Interop.ReadDir.cs with Interop.ReadDir.cs from System.IO.FileSystem (#21622)
authorEgor Bogatov <egorbo@gmail.com>
Fri, 28 Dec 2018 18:58:42 +0000 (21:58 +0300)
committerJan Kotas <jkotas@microsoft.com>
Fri, 28 Dec 2018 18:58:42 +0000 (08:58 -1000)
* Use Interop.ReadDir.cs from FileSystem (corefx)

* Use ArrayPool

* Remove ReadBufferSize (and static cctor)

* use NameBufferSize for 256 const

* Delete ReadDir and SafeDirectoryHandle.Unix.cs

* Introduce GetDirectoryEntryFullPath, remove uint casts

* Refactor EnumerateFilesRecursively

src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.ReadDir.cs
src/System.Private.CoreLib/shared/Microsoft/Win32/SafeHandles/SafeDirectoryHandle.Unix.cs [deleted file]
src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems
src/System.Private.CoreLib/shared/System/TimeZoneInfo.Unix.cs

index d98c428..395f5d3 100644 (file)
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Diagnostics;
 using System.Runtime.InteropServices;
-using System.Threading;
-using Microsoft.Win32.SafeHandles;
+using System.Text;
 
 internal static partial class Interop
 {
     internal static partial class Sys
     {
-        private static readonly int s_readBufferSize = GetReadDirRBufferSize();
-
         internal enum NodeType : int
         {
-            DT_UNKNOWN  =  0,
-            DT_FIFO     =  1,
-            DT_CHR      =  2,
-            DT_DIR      =  4,
-            DT_BLK      =  6,
-            DT_REG      =  8,
-            DT_LNK      = 10,
-            DT_SOCK     = 12,
-            DT_WHT      = 14
+            DT_UNKNOWN = 0,
+            DT_FIFO = 1,
+            DT_CHR = 2,
+            DT_DIR = 4,
+            DT_BLK = 6,
+            DT_REG = 8,
+            DT_LNK = 10,
+            DT_SOCK = 12,
+            DT_WHT = 14
         }
 
         [StructLayout(LayoutKind.Sequential)]
-        private unsafe struct InternalDirectoryEntry
+        internal unsafe struct DirectoryEntry
         {
-            internal IntPtr     Name;
-            internal int        NameLength;
-            internal NodeType   InodeType;
-        }
+            internal byte* Name;
+            internal int NameLength;
+            internal NodeType InodeType;
+            internal const int NameBufferSize = 256;
 
-        internal struct DirectoryEntry
-        {
-            internal NodeType   InodeType;
-            internal string     InodeName;
+            internal ReadOnlySpan<char> GetName(Span<char> buffer)
+            {
+                Debug.Assert(buffer.Length >= Encoding.UTF8.GetMaxCharCount(NameBufferSize - 1), "should have enough space for the max file name");
+                Debug.Assert(Name != null, "should not have a null name");
+
+                ReadOnlySpan<byte> nameBytes = (NameLength == -1)
+                    // In this case the struct was allocated via struct dirent *readdir(DIR *dirp);
+                    ? new ReadOnlySpan<byte>(Name, new ReadOnlySpan<byte>(Name, NameBufferSize - 1).IndexOf<byte>(0))
+                    : new ReadOnlySpan<byte>(Name, NameLength);
+
+                Debug.Assert(nameBytes.Length > 0, "we shouldn't have gotten a garbage value from the OS");
+                if (nameBytes.Length == 0)
+                    return buffer.Slice(0, 0);
+
+                int charCount = Encoding.UTF8.GetChars(nameBytes, buffer);
+                ReadOnlySpan<char> value = buffer.Slice(0, charCount);
+                Debug.Assert(NameLength != -1 || !value.Contains('\0'), "should not have embedded nulls if we parsed the end of string");
+                return value;
+            }
         }
 
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_OpenDir", SetLastError = true)]
-        internal static extern Microsoft.Win32.SafeHandles.SafeDirectoryHandle OpenDir(string path);
+        internal static extern IntPtr OpenDir(string path);
 
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetReadDirRBufferSize", SetLastError = false)]
         internal static extern int GetReadDirRBufferSize();
 
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_ReadDirR", SetLastError = false)]
-        private static extern unsafe int ReadDirR(IntPtr dir, byte* buffer, int bufferSize, out InternalDirectoryEntry outputEntry);
+        internal static extern unsafe int ReadDirR(IntPtr dir, byte* buffer, int bufferSize, out DirectoryEntry outputEntry);
 
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_CloseDir", SetLastError = true)]
         internal static extern int CloseDir(IntPtr dir);
-
-        // The calling pattern for ReadDir is described in src/Native/System.Native/pal_readdir.cpp
-        internal static int ReadDir(SafeDirectoryHandle dir, out DirectoryEntry outputEntry)
-        {
-            bool addedRef = false;
-            try
-            {
-                // We avoid a native string copy into InternalDirectoryEntry.
-                // - If the platform suppors reading into a buffer, the data is read directly into the buffer. The
-                //   data can be read as long as the buffer is valid.
-                // - If the platform does not support reading into a buffer, the information returned in
-                //   InternalDirectoryEntry points to native memory owned by the SafeDirectoryHandle. The data is only
-                //   valid until the next call to CloseDir/ReadDir. We extend the reference until we have copied all data
-                //   to ensure it does not become invalid by a CloseDir; and we copy the data so our caller does not
-                //   use the native memory held by the SafeDirectoryHandle.
-                dir.DangerousAddRef(ref addedRef);
-
-                unsafe
-                {
-                    // s_readBufferSize is zero when the native implementation does not support reading into a buffer.
-                    byte* buffer = stackalloc byte[s_readBufferSize];
-                    InternalDirectoryEntry temp;
-                    int ret = ReadDirR(dir.DangerousGetHandle(), buffer, s_readBufferSize, out temp);
-                    // We copy data into DirectoryEntry to ensure there are no dangling references.
-                    outputEntry = ret == 0 ?
-                                new DirectoryEntry() { InodeName = GetDirectoryEntryName(temp), InodeType = temp.InodeType } : 
-                                default(DirectoryEntry);
-
-                    return ret;
-                }
-            }
-            finally
-            {
-                if (addedRef)
-                {
-                    dir.DangerousRelease();
-                }
-            }
-        }
-
-        private static unsafe string GetDirectoryEntryName(InternalDirectoryEntry dirEnt)
-        {
-            if (dirEnt.NameLength == -1)
-                return Marshal.PtrToStringAnsi(dirEnt.Name);
-            else
-                return Marshal.PtrToStringAnsi(dirEnt.Name, dirEnt.NameLength);
-        }
     }
 }
diff --git a/src/System.Private.CoreLib/shared/Microsoft/Win32/SafeHandles/SafeDirectoryHandle.Unix.cs b/src/System.Private.CoreLib/shared/Microsoft/Win32/SafeHandles/SafeDirectoryHandle.Unix.cs
deleted file mode 100644 (file)
index f7435ea..0000000
+++ /dev/null
@@ -1,26 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// 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.Runtime.InteropServices;
-
-namespace Microsoft.Win32.SafeHandles
-{
-    internal sealed class SafeDirectoryHandle : SafeHandle
-    {
-        private SafeDirectoryHandle() : base(IntPtr.Zero, true)
-        {
-        }
-
-        protected override bool ReleaseHandle()
-        {
-            return Interop.Sys.CloseDir(handle) == 0;
-        }
-
-        public override bool IsInvalid
-        {
-            get { return handle == IntPtr.Zero; }
-        }
-    }
-}
index 1c26056..d9ac914 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Unlink.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Write.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Internal\IO\File.Unix.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\SafeDirectoryHandle.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\SafeFileHandle.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\DebugProvider.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Globalization\CalendarData.Unix.cs" Condition="'$(EnableDummyGlobalizationImplementation)' != 'true'" />
index 254a0cd..27b6326 100644 (file)
@@ -2,6 +2,7 @@
 // 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.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Globalization;
@@ -9,6 +10,7 @@ using System.IO;
 using System.Text;
 using System.Threading;
 using System.Security;
+using System.Runtime.InteropServices;
 
 using Internal.IO;
 
@@ -406,84 +408,116 @@ namespace System
             return id;
         }
 
+        private static string GetDirectoryEntryFullPath(ref Interop.Sys.DirectoryEntry dirent, string currentPath)
+        {
+            Span<char> nameBuffer = stackalloc char[Interop.Sys.DirectoryEntry.NameBufferSize];
+            ReadOnlySpan<char> direntName = dirent.GetName(nameBuffer);
+
+            if ((direntName.Length == 1 && direntName[0] == '.') ||
+                (direntName.Length == 2 && direntName[0] == '.' && direntName[1] == '.'))
+                return null;
+
+            return Path.Join(currentPath.AsSpan(), direntName);
+        }
+
         /// <summary>
         /// Enumerate files
         /// </summary>
-        private static IEnumerable<string> EnumerateFilesRecursively(string path)
+        private static unsafe void EnumerateFilesRecursively(string path, Predicate<string> condition)
         {
             List<string> toExplore = null; // List used as a stack
 
-            string currentPath = path;
-            for(;;)
+            int bufferSize = Interop.Sys.GetReadDirRBufferSize();
+            byte[] dirBuffer = null;
+            try
             {
-                using (Microsoft.Win32.SafeHandles.SafeDirectoryHandle dirHandle = Interop.Sys.OpenDir(currentPath))
-                {
-                    if (dirHandle.IsInvalid)
-                    {
-                        throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), currentPath, isDirectory: true);
-                    }
+                dirBuffer = ArrayPool<byte>.Shared.Rent(bufferSize);
+                string currentPath = path;
 
-                    // Read each entry from the enumerator
-                    Interop.Sys.DirectoryEntry dirent;
-                    while (Interop.Sys.ReadDir(dirHandle, out dirent) == 0)
+                fixed (byte* dirBufferPtr = dirBuffer)
+                {
+                    for(;;)
                     {
-                        if (dirent.InodeName == "." || dirent.InodeName == "..")
-                            continue;
-
-                        string fullPath = Path.Combine(currentPath, dirent.InodeName);
-
-                        // Get from the dir entry whether the entry is a file or directory.
-                        // We classify everything as a file unless we know it to be a directory.
-                        bool isDir;
-                        if (dirent.InodeType == Interop.Sys.NodeType.DT_DIR)
+                        IntPtr dirHandle = Interop.Sys.OpenDir(currentPath);
+                        if (dirHandle == IntPtr.Zero)
                         {
-                            // We know it's a directory.
-                            isDir = true;
+                            throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), currentPath, isDirectory: true);
                         }
-                        else if (dirent.InodeType == Interop.Sys.NodeType.DT_LNK || dirent.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)
-                        {
-                            // It's a symlink or unknown: stat to it to see if we can resolve it to a directory.
-                            // If we can't (e.g. symlink to a file, broken symlink, etc.), we'll just treat it as a file.
 
-                            Interop.Sys.FileStatus fileinfo;
-                            if (Interop.Sys.Stat(fullPath, out fileinfo) >= 0)
-                            {
-                                isDir = (fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
-                            }
-                            else
-                            {
-                                isDir = false;
-                            }
-                        }
-                        else
+                        try
                         {
-                            // Otherwise, treat it as a file.  This includes regular files, FIFOs, etc.
-                            isDir = false;
-                        }
-
-                        // Yield the result if the user has asked for it.  In the case of directories,
-                        // always explore it by pushing it onto the stack, regardless of whether
-                        // we're returning directories.
-                        if (isDir)
-                        {
-                            if (toExplore == null)
+                            // Read each entry from the enumerator
+                            Interop.Sys.DirectoryEntry dirent;
+                            while (Interop.Sys.ReadDirR(dirHandle, dirBufferPtr, bufferSize, out dirent) == 0)
                             {
-                                toExplore = new List<string>();
+                                string fullPath = GetDirectoryEntryFullPath(ref dirent, currentPath);
+                                if (fullPath == null)
+                                    continue;
+
+                                // Get from the dir entry whether the entry is a file or directory.
+                                // We classify everything as a file unless we know it to be a directory.
+                                bool isDir;
+                                if (dirent.InodeType == Interop.Sys.NodeType.DT_DIR)
+                                {
+                                    // We know it's a directory.
+                                    isDir = true;
+                                }
+                                else if (dirent.InodeType == Interop.Sys.NodeType.DT_LNK || dirent.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)
+                                {
+                                    // It's a symlink or unknown: stat to it to see if we can resolve it to a directory.
+                                    // If we can't (e.g. symlink to a file, broken symlink, etc.), we'll just treat it as a file.
+
+                                    Interop.Sys.FileStatus fileinfo;
+                                    if (Interop.Sys.Stat(fullPath, out fileinfo) >= 0)
+                                    {
+                                        isDir = (fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
+                                    }
+                                    else
+                                    {
+                                        isDir = false;
+                                    }
+                                }
+                                else
+                                {
+                                    // Otherwise, treat it as a file.  This includes regular files, FIFOs, etc.
+                                    isDir = false;
+                                }
+
+                                // Yield the result if the user has asked for it.  In the case of directories,
+                                // always explore it by pushing it onto the stack, regardless of whether
+                                // we're returning directories.
+                                if (isDir)
+                                {
+                                    if (toExplore == null)
+                                    {
+                                        toExplore = new List<string>();
+                                    }
+                                    toExplore.Add(fullPath);
+                                }
+                                else if (condition(fullPath))
+                                {
+                                    return;
+                                }
                             }
-                            toExplore.Add(fullPath);
                         }
-                        else
+                        finally
                         {
-                            yield return fullPath;
+                            if (dirHandle != IntPtr.Zero)
+                                Interop.Sys.CloseDir(dirHandle);
                         }
-                    }
-                }
 
-                if (toExplore == null || toExplore.Count == 0)
-                    break;
+                        if (toExplore == null || toExplore.Count == 0)
+                            break;
 
-                currentPath = toExplore[toExplore.Count - 1];
-                toExplore.RemoveAt(toExplore.Count - 1);
+                        currentPath = toExplore[toExplore.Count - 1];
+                        toExplore.RemoveAt(toExplore.Count - 1);
+                    }
+                }
+            }
+            finally
+            {
+                if (dirBuffer != null)
+                    ArrayPool<byte>.Shared.Return(dirBuffer);
             }
         }
 
@@ -502,8 +536,8 @@ namespace System
 
             try
             {
-                foreach (string filePath in EnumerateFilesRecursively(timeZoneDirectory))
-                {
+                EnumerateFilesRecursively(timeZoneDirectory, (string filePath) =>
+                {                
                     // skip the localtime and posixrules file, since they won't give us the correct id
                     if (!string.Equals(filePath, localtimeFilePath, StringComparison.OrdinalIgnoreCase)
                         && !string.Equals(filePath, posixrulesFilePath, StringComparison.OrdinalIgnoreCase))
@@ -518,10 +552,11 @@ namespace System
                             {
                                 id = id.Substring(timeZoneDirectory.Length);
                             }
-                            break;
+                            return true;
                         }
                     }
-                }
+                    return false;
+                });
             }
             catch (IOException) { }
             catch (SecurityException) { }