Unix: Remove MaxPath, MaxName (#15229)
authorTom Deseyn <tom.deseyn@gmail.com>
Fri, 1 Dec 2017 11:38:13 +0000 (12:38 +0100)
committerStephen Toub <stoub@microsoft.com>
Fri, 1 Dec 2017 11:38:13 +0000 (06:38 -0500)
* Unix: Remove MaxPath, MaxName

PATH_MAX is not an upper bound on path lengths on Unix. system function return
values should be used to determine behavior, instead of limiting the path length.

NAME_MAX depends on the file system used. Its value depends on the path, it is
not constant for the entire system.

* PR feedback

* Sync with corefx Interop.ReadLink

* Deal with /etc/localtime link containing relative path

src/corefx/System.Globalization.Native/timeZoneInfo.cpp
src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.TimeZoneInfo.cs
src/mscorlib/shared/Interop/Unix/System.Native/Interop.GetCwd.cs
src/mscorlib/shared/Interop/Unix/System.Native/Interop.PathConf.cs
src/mscorlib/shared/Interop/Unix/System.Native/Interop.ReadLink.cs [new file with mode: 0644]
src/mscorlib/shared/System.Private.CoreLib.Shared.projitems
src/mscorlib/shared/System/IO/Path.Unix.cs
src/mscorlib/src/System/Threading/Mutex.cs
src/mscorlib/src/System/TimeZoneInfo.Unix.cs

index 0dd28b4..d5ab7e0 100644 (file)
 #include "errors.h"
 
 /*
-Gets the symlink value for the path.
-*/
-extern "C" int32_t GlobalizationNative_ReadLink(const char* path, char* result, size_t resultCapacity)
-{
-    ssize_t r = readlink(path, result, resultCapacity - 1); // subtract one to make room for the NULL character
-
-    if (r < 1 || r >= resultCapacity)
-        return false;
-
-    result[r] = '\0';
-    return true;
-}
-
-/*
 These values should be kept in sync with the managed Interop.GlobalizationInterop.TimeZoneDisplayNameType enum.
 */
 enum TimeZoneDisplayNameType : int32_t
index 26a9fe0..271ec3f 100644 (file)
@@ -9,9 +9,6 @@ internal static partial class Interop
 {
     internal static partial class GlobalizationInterop
     {
-        [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Ansi, EntryPoint = "GlobalizationNative_ReadLink")] // readlink requires char*
-        internal static extern bool ReadLink(string filePath, [Out] StringBuilder result, uint resultCapacity);
-
         // needs to be kept in sync with TimeZoneDisplayNameType in System.Globalization.Native
         internal enum TimeZoneDisplayNameType
         {
index a27a35c..a8bef6b 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Buffers;
 using System.Runtime.InteropServices;
 
 internal static partial class Interop
@@ -10,7 +11,7 @@ internal static partial class Interop
     internal static partial class Sys
     {
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetCwd", SetLastError = true)]
-        private static unsafe extern byte* GetCwd(byte* buffer, int bufferLength);
+        private static extern unsafe byte* GetCwd(byte* buffer, int bufferLength);
 
         internal static unsafe string GetCwd()
         {      
@@ -25,15 +26,13 @@ internal static partial class Interop
             }
 
             // If that was too small, try increasing large buffer sizes
-            // until we get one that works or until we hit MaxPath.
-            int maxPath = Interop.Sys.MaxPath;
-            if (StackLimit < maxPath)
+            int bufferSize = StackLimit;
+            do
             {
-                int bufferSize = StackLimit;
-                do
+                checked { bufferSize *= 2; }
+                byte[] buf = ArrayPool<byte>.Shared.Rent(bufferSize);
+                try
                 {
-                    checked { bufferSize *= 2; }
-                    var buf = new byte[Math.Min(bufferSize, maxPath)];
                     fixed (byte* ptr = &buf[0])
                     {
                         result = GetCwdHelper(ptr, buf.Length);
@@ -43,11 +42,12 @@ internal static partial class Interop
                         }
                     }
                 }
-                while (bufferSize < maxPath);
+                finally
+                {
+                    ArrayPool<byte>.Shared.Return(buf);
+                }
             }
-
-            // If we couldn't get the cwd with a MaxPath-sized buffer, something's wrong.
-            throw Interop.GetExceptionForIoErrno(new ErrorInfo(Interop.Error.ENAMETOOLONG));
+            while (true);
         }
 
         private static unsafe string GetCwdHelper(byte* ptr, int bufferSize)
index 4a1fcf6..eb9e32d 100644 (file)
@@ -24,50 +24,7 @@ internal static partial class Interop
             PC_VDISABLE         = 9,
         }
 
-        /// <summary>The maximum path length for the system.  -1 if it hasn't yet been initialized.</summary>
-        private static int s_maxPath = -1;
-
-        /// <summary>The maximum name length for the system.  -1 if it hasn't yet been initialized.</summary>
-        private static int s_maxName = -1;
-
-        internal static int MaxPath
-        {
-            get
-            {
-                // Benign race condition on cached value
-                if (s_maxPath < 0) 
-                {
-                    // GetMaximumPath returns a long from PathConf
-                    // but our callers expect an int so we need to convert.
-                    long temp = GetMaximumPath();
-                    if (temp > int.MaxValue)
-                        s_maxPath = int.MaxValue;
-                    else
-                        s_maxPath = Convert.ToInt32(temp);
-                }
-                return s_maxPath; 
-            }
-        }
-
-        internal static int MaxName
-        {
-            get
-            {
-                // Benign race condition on cached value
-                if (s_maxName < 0)
-                {
-                    int result = PathConf("/", PathConfName.PC_NAME_MAX);
-                    s_maxName = result >= 0 ? result : DEFAULT_PC_NAME_MAX;
-                }
-                
-                return s_maxName;
-            }
-        }
-
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PathConf", SetLastError = true)]
         private static extern int PathConf(string path, PathConfName name);
-
-        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetMaximumPath")]
-        private static extern long GetMaximumPath();
     }
 }
diff --git a/src/mscorlib/shared/Interop/Unix/System.Native/Interop.ReadLink.cs b/src/mscorlib/shared/Interop/Unix/System.Native/Interop.ReadLink.cs
new file mode 100644 (file)
index 0000000..50f1ae5
--- /dev/null
@@ -0,0 +1,63 @@
+// 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.Runtime.InteropServices;
+using System.Buffers;
+using System.Text;
+
+internal static partial class Interop
+{
+    internal static partial class Sys
+    {
+        /// <summary>
+        /// Takes a path to a symbolic link and attempts to place the link target path into the buffer. If the buffer is too
+        /// small, the path will be truncated. No matter what, the buffer will not be null terminated. 
+        /// </summary>
+        /// <param name="path">The path to the symlink</param>
+        /// <param name="buffer">The buffer to hold the output path</param>
+        /// <param name="bufferSize">The size of the buffer</param>
+        /// <returns>
+        /// Returns the number of bytes placed into the buffer on success; bufferSize if the buffer is too small; and -1 on error.
+        /// </returns>
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_ReadLink", SetLastError = true)]
+        private static extern unsafe int ReadLink(string path, byte[] buffer, int bufferSize);
+
+        /// <summary>
+        /// Takes a path to a symbolic link and returns the link target path.
+        /// </summary>
+        /// <param name="path">The path to the symlink</param>
+        /// <returns>
+        /// Returns the link to the target path on success; and null otherwise.
+        /// </returns>
+        public static string ReadLink(string path)
+        {
+            int bufferSize = 256;
+            do
+            {
+                byte[] buffer = ArrayPool<byte>.Shared.Rent(bufferSize);
+                try
+                {
+                    int resultLength = Interop.Sys.ReadLink(path, buffer, buffer.Length);
+                    if (resultLength < 0)
+                    {
+                        // error
+                        return null;
+                    }
+                    else if (resultLength < buffer.Length)
+                    {
+                        // success
+                        return Encoding.UTF8.GetString(buffer, 0, resultLength);
+                    }
+                }
+                finally
+                {
+                    ArrayPool<byte>.Shared.Return(buffer);
+                }
+
+                // buffer was too small, loop around again and try with a larger buffer.
+                bufferSize *= 2;
+            } while (true);
+        }
+    }
+}
\ No newline at end of file
index 9dc07cb..20bf435 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Permissions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.PosixFAdvise.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Read.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.ReadLink.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Stat.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.SysLog.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Unix\System.Native\Interop.Unlink.cs" />
index 61a8314..e5be579 100644 (file)
@@ -14,8 +14,6 @@ namespace System.IO
 
         public static char[] GetInvalidPathChars() => new char[] { '\0' };
 
-        internal static int MaxPath => Interop.Sys.MaxPath;
-
         // Expands the given path to a fully qualified path. 
         public static string GetFullPath(string path)
         {
index 991fd4d..c0216d4 100644 (file)
@@ -19,6 +19,11 @@ namespace System.Threading
         private const uint AccessRights =
             (uint)Win32Native.MAXIMUM_ALLOWED | Win32Native.SYNCHRONIZE | Win32Native.MUTEX_MODIFY_STATE;
 
+#if PLATFORM_UNIX
+        // Maximum file name length on tmpfs file system.
+        private const int WaitHandleNameMax = 255;
+#endif
+
         public Mutex(bool initiallyOwned, string name, out bool createdNew)
         {
 #if !PLATFORM_UNIX
@@ -94,7 +99,11 @@ namespace System.Threading
 
         private void CreateMutexCore(bool initiallyOwned, string name, out bool createdNew)
         {
+#if !PLATFORM_UNIX
             Debug.Assert(name == null || name.Length <= Path.MaxPath);
+#else
+            Debug.Assert(name == null);
+#endif
 
             uint mutexFlags = initiallyOwned ? Win32Native.CREATE_MUTEX_INITIAL_OWNER : 0;
 
@@ -107,7 +116,7 @@ namespace System.Threading
 #if PLATFORM_UNIX
                 if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE)
                     // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8
-                    throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, Interop.Sys.MaxName), nameof(name));
+                    throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, WaitHandleNameMax), nameof(name));
 #endif
                 if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE)
                     throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name));
@@ -149,7 +158,7 @@ namespace System.Threading
                 if (name != null && errorCode == Win32Native.ERROR_FILENAME_EXCED_RANGE)
                 {
                     // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8
-                    throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, Interop.Sys.MaxName), nameof(name));
+                    throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, WaitHandleNameMax), nameof(name));
                 }
 #endif
                 if (Win32Native.ERROR_FILE_NOT_FOUND == errorCode || Win32Native.ERROR_INVALID_NAME == errorCode)
index 6158309..5d97fe8 100644 (file)
@@ -383,22 +383,18 @@ namespace System
         {
             string id = null;
 
-            StringBuilder symlinkPathBuilder = StringBuilderCache.Acquire(Path.MaxPath);
-            bool result = Interop.GlobalizationInterop.ReadLink(tzFilePath, symlinkPathBuilder, (uint)symlinkPathBuilder.Capacity);
-            if (result)
+            string symlinkPath = Interop.Sys.ReadLink(tzFilePath);
+            if (symlinkPath != null)
             {
-                string symlinkPath = StringBuilderCache.GetStringAndRelease(symlinkPathBuilder);
-                // time zone Ids have to point under the time zone directory
+                // Use Path.Combine to resolve links that contain a relative path (e.g. /etc/localtime).
+                symlinkPath = Path.Combine(tzFilePath, symlinkPath);
+
                 string timeZoneDirectory = GetTimeZoneDirectory();
                 if (symlinkPath.StartsWith(timeZoneDirectory))
                 {
                     id = symlinkPath.Substring(timeZoneDirectory.Length);
                 }
             }
-            else
-            {
-                StringBuilderCache.Release(symlinkPathBuilder);
-            }
 
             return id;
         }