From f750b174dd62c4f7ab1edfa529d283fc90bfc7fe Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 13 Jun 2022 13:55:31 +0200 Subject: [PATCH 1/5] Unix: Move 'FileNotFound to DirectoryNotFound' handling to GetExceptionForIoErrno. On Windows, APIs throw DirectoryNotFoundException instead of FileNotFoundException when the base components of a path don't exist. SafeFileHandle, and FileSystemInfo have some specific code to detect that case and to the same on Unix. This change moves that mapping to Interop.IOErrors so it applies to all users of GetExceptionForIoErrno. --- .../src/Interop/Unix/Interop.IOErrors.cs | 34 ++++++++++++---- .../src/System.IO.FileSystem.DriveInfo.csproj | 2 + .../src/System.IO.Ports.csproj | 2 + .../src/System.Net.Http.csproj | 2 + .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 24 ----------- .../src/System/IO/FileStatus.Unix.cs | 40 ++++++++++++------- .../src/System/IO/FileSystem.Unix.cs | 2 +- .../src/System/IO/FileSystemInfo.Unix.cs | 14 ------- 8 files changed, 59 insertions(+), 61 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 6c348f9b164637..71c9a814bcdd61 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -114,18 +114,21 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa switch (errorInfo.Error) { case Error.ENOENT: - if (isDirectory) - { - return !string.IsNullOrEmpty(path) ? - new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, path)) : - new DirectoryNotFoundException(SR.IO_PathNotFound_NoPathName); - } - else + // For Windows compatibility, throw DirectoryNotFoundException instead of FileNotFoundException + // when the parent folder does not exist. + if (!isDirectory && (path is null || + DirectoryExists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path))))) { return !string.IsNullOrEmpty(path) ? new FileNotFoundException(SR.Format(SR.IO_FileNotFound_FileName, path), path) : new FileNotFoundException(SR.IO_FileNotFound); } + goto case Error.ENOTDIR; + + case Error.ENOTDIR: + return !string.IsNullOrEmpty(path) ? + new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, path)) : + new DirectoryNotFoundException(SR.IO_PathNotFound_NoPathName); case Error.EACCES: case Error.EBADF: @@ -160,6 +163,23 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa default: return GetIOException(errorInfo, path); + + static bool DirectoryExists(string? fullPath) + { + if (fullPath is null) + { + return false; + } + + Interop.Sys.FileStatus fileinfo; + + if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) + { + return false; + } + + return ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR); + } } } diff --git a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj index 690d1aeb7a94b7..8488c6947f30be 100644 --- a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj +++ b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj @@ -63,6 +63,8 @@ Link="Common\Interop\Unix\Interop.UnixFileSystemTypes.cs" /> + Link="Common\Interop\Unix\Interop.Errors.cs" /> + + (e.Error == Interop.Error.EISDIR) ? Interop.Error.EACCES.Info() : e); } return handle; } - private static bool DirectoryExists(string fullPath) - { - Interop.Sys.FileStatus fileinfo; - - if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) - { - return false; - } - - return ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR); - } - // Each thread will have its own copy. This prevents race conditions if the handle had the last error. [ThreadStatic] internal static Interop.ErrorInfo? t_lastCloseErrorInfo; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index e90408a7481e28..8a688bfc6935e2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -10,13 +10,14 @@ internal partial struct FileStatus { private const int NanosecondsPerTick = 100; - private const int InitializedExistsDir = -3; // target is directory. - private const int InitializedExistsFile = -2; // target is file. - private const int InitializedNotExists = -1; // entry does not exist. - private const int Uninitialized = 0; // uninitialized, '0' to make default(FileStatus) uninitialized. + private const int InitializedExistsDir = -4; // target is directory. + private const int InitializedExistsFile = -3; // target is file. + private const int InitializedNotExistsNotADir = -2; // entry parent path is not a dir. + private const int InitializedNotExists = -1; // entry does not exist. + private const int Uninitialized = 0; // uninitialized, '0' to make default(FileStatus) uninitialized. // Tracks the initialization state. - // < 0 : initialized succesfully. Value is InitializedNotExists, InitializedExistsFile or InitializedExistsDir. + // < 0 : initialized succesfully. Value is InitializedNotExists, InitializedNotExistsNotADir, InitializedExistsFile or InitializedExistsDir. // 0 : uninitialized. // > 0 : initialized with error. Value is raw errno. private int _state; @@ -223,7 +224,7 @@ internal void SetAttributes(string path, FileAttributes attributes, bool asDirec EnsureCachesInitialized(path); if (!EntryExists) - FileSystemInfo.ThrowNotFound(path); + ThrowNotFound(path); if (Interop.Sys.CanSetHiddenFlag) { @@ -336,7 +337,7 @@ private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, b EnsureCachesInitialized(path); if (!EntryExists) - FileSystemInfo.ThrowNotFound(path); + ThrowNotFound(path); // we use utimes()/utimensat() to set the accessTime and writeTime Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; @@ -414,15 +415,18 @@ internal void RefreshCaches(ReadOnlySpan path) { Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ENOENT || // A component of the path does not exist, or path is an empty string - errorInfo.Error == Interop.Error.ENOTDIR) // A component of the path prefix of path is not a directory + switch (errorInfo.Error) { - _state = InitializedNotExists; - } - else - { - Debug.Assert(errorInfo.RawErrno > 0); // Expect a positive integer - _state = errorInfo.RawErrno; // Initialized with error. + case Interop.Error.ENOENT: + _state = InitializedNotExists; + break; + case Interop.Error.ENOTDIR: + _state = InitializedNotExistsNotADir; + break; + default: + Debug.Assert(errorInfo.RawErrno > 0); // Expect a positive integer + _state = errorInfo.RawErrno; // Initialized with error. + break; } return; @@ -471,5 +475,11 @@ private static long UnixTimeSecondsToNanoseconds(DateTimeOffset time, long secon const long TicksPerSecond = TicksPerMillisecond * 1000; return (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick; } + + private void ThrowNotFound(string? path) + { + Interop.Error error = _state == InitializedNotExistsNotADir ? Interop.Error.ENOTDIR : Interop.Error.ENOENT; + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(error), path, isDirectory: false); + } } } 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 751a59f9eab949..d15ac0d5f0f023 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 @@ -580,7 +580,7 @@ public static FileAttributes GetAttributes(string fullPath) FileAttributes attributes = new FileInfo(fullPath, null).Attributes; if (attributes == (FileAttributes)(-1)) - FileSystemInfo.ThrowNotFound(fullPath); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.ENOENT), fullPath, isDirectory: false); return attributes; } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystemInfo.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystemInfo.Unix.cs index f74e3c411dd6f8..a3b33116bec710 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystemInfo.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystemInfo.Unix.cs @@ -69,20 +69,6 @@ public void Refresh() _fileStatus.RefreshCaches(FullPath); } - internal static void ThrowNotFound(string path) - { - // Windows distinguishes between whether the directory or the file isn't found, - // and throws a different exception in these cases. We attempt to approximate that - // here; there is a race condition here, where something could change between - // when the error occurs and our checks, but it's the best we can do, and the - // worst case in such a race condition (which could occur if the file system is - // being manipulated concurrently with these checks) is that we throw a - // FileNotFoundException instead of DirectoryNotFoundException. - - bool directoryError = !Directory.Exists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path))); - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.ENOENT), path, directoryError); - } - // There is no special handling for Unix- see Windows code for the reason we do this internal string NormalizedPath => FullPath; } From 08eeb305e9e0306685ac7efe34e5ed8322d714aa Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 13 Jun 2022 16:36:00 +0200 Subject: [PATCH 2/5] Fix OSX test failure. --- .../System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 d15ac0d5f0f023..bd23715dd03e7b 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 @@ -290,7 +290,10 @@ public static void CreateDirectory(string fullPath) return; // fullPath is '/'. } - int result = Interop.Sys.MkDir(fullPath, (int)Interop.Sys.Permissions.Mask); + // macOS returns ENOTDIR when the path refers to a file and ends with '/'. + // Trim the separator so we get EEXIST instead. + ReadOnlySpan path = PathInternal.TrimEndingDirectorySeparator(fullPath.AsSpan()); + int result = Interop.Sys.MkDir(path, (int)Interop.Sys.Permissions.Mask); if (result == 0) { return; // Created directory. From a0a09137c6152aaba4922bfd7b58071ae7907c31 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 13 Jun 2022 16:36:12 +0200 Subject: [PATCH 3/5] Fix wasm build. --- src/libraries/System.Console/src/System.Console.csproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Console/src/System.Console.csproj b/src/libraries/System.Console/src/System.Console.csproj index 540cedba4f3d42..307d213038f2ef 100644 --- a/src/libraries/System.Console/src/System.Console.csproj +++ b/src/libraries/System.Console/src/System.Console.csproj @@ -62,6 +62,8 @@ Link="Common\Interop\Unix\Interop.FileDescriptors.cs" /> + From 5cef5a632843ced04dbafb9e75405a818e668f36 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 24 Jun 2022 13:11:00 +0200 Subject: [PATCH 4/5] Review usage of CheckIo/GetExceptionForIoErrno. --- .../src/Interop/Unix/Interop.IOErrors.cs | 34 +++++------ .../src/System/IO/DriveInfo.Unix.cs | 2 +- .../src/System/IO/FileSystemWatcher.Linux.cs | 3 +- .../src/System/IO/FileSystemWatcher.OSX.cs | 2 +- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 11 +++- .../src/System/Environment.UnixOrBrowser.cs | 2 +- .../Enumeration/FileSystemEnumerator.Unix.cs | 4 +- .../src/System/IO/FileStatus.Unix.cs | 11 +--- .../src/System/IO/FileSystem.Unix.cs | 56 ++++++------------- .../src/System/IO/FileSystem.Windows.cs | 2 +- .../IO/Strategies/FileStreamHelpers.Unix.cs | 4 +- .../System/TimeZoneInfo.Unix.NonAndroid.cs | 2 +- 12 files changed, 52 insertions(+), 81 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index add32932348c7d..3cbcd513047f52 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -8,19 +8,19 @@ internal static partial class Interop { - private static void ThrowExceptionForIoErrno(ErrorInfo errorInfo, string? path, bool isDirectory) + private static void ThrowExceptionForIoErrno(ErrorInfo errorInfo, string? path, bool isDirError) { Debug.Assert(errorInfo.Error != Error.SUCCESS); Debug.Assert(errorInfo.Error != Error.EINTR, "EINTR errors should be handled by the native shim and never bubble up to managed code"); - throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory); + throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirError); } - internal static void CheckIo(Error error, string? path = null, bool isDirectory = false) + internal static void CheckIo(Error error, string? path = null, bool isDirError = false) { if (error != Interop.Error.SUCCESS) { - ThrowExceptionForIoErrno(error.Info(), path, isDirectory); + ThrowExceptionForIoErrno(error.Info(), path, isDirError); } } @@ -31,15 +31,15 @@ internal static void CheckIo(Error error, string? path = null, bool isDirectory /// /// The result of the system call. /// The path with which this error is associated. This may be null. - /// true if the is known to be a directory; otherwise, false. + /// true if error is caused by a directory issue. /// /// On success, returns the non-negative result long that was validated. /// - internal static long CheckIo(long result, string? path = null, bool isDirectory = false) + internal static long CheckIo(long result, string? path = null, bool isDirError = false) { if (result < 0) { - ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirectory); + ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirError); } return result; @@ -53,9 +53,9 @@ internal static long CheckIo(long result, string? path = null, bool isDirectory /// /// On success, returns the non-negative result int that was validated. /// - internal static int CheckIo(int result, string? path = null, bool isDirectory = false) + internal static int CheckIo(int result, string? path = null, bool isDirError = false) { - CheckIo((long)result, path, isDirectory); + CheckIo((long)result, path, isDirError); return result; } @@ -68,9 +68,9 @@ internal static int CheckIo(int result, string? path = null, bool isDirectory = /// /// On success, returns the non-negative result IntPtr that was validated. /// - internal static IntPtr CheckIo(IntPtr result, string? path = null, bool isDirectory = false) + internal static IntPtr CheckIo(IntPtr result, string? path = null, bool isDirError = false) { - CheckIo((long)result, path, isDirectory); + CheckIo((long)result, path, isDirError); return result; } @@ -83,12 +83,12 @@ internal static IntPtr CheckIo(IntPtr result, string? path = null, bool isDirect /// /// On success, returns the valid SafeFileHandle that was validated. /// - internal static TSafeHandle CheckIo(TSafeHandle handle, string? path = null, bool isDirectory = false) + internal static TSafeHandle CheckIo(TSafeHandle handle, string? path = null, bool isDirError = false) where TSafeHandle : SafeHandle { if (handle.IsInvalid) { - ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirectory); + ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirError); } return handle; @@ -99,9 +99,9 @@ internal static TSafeHandle CheckIo(TSafeHandle handle, string? pat /// /// The error info /// The path with which this error is associated. This may be null. - /// true if the is known to be a directory; otherwise, false. + /// true if error is caused by a directory issue. /// - internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? path = null, bool isDirectory = false) + internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? path = null, bool isDirError = false) { // Translate the errno into a known set of exception types. For cases where multiple errnos map // to the same exception type, include an inner exception with the details. @@ -110,8 +110,8 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa case Error.ENOENT: // For Windows compatibility, throw DirectoryNotFoundException instead of FileNotFoundException // when the parent folder does not exist. - if (!isDirectory && (path is null || - DirectoryExists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path))))) + if ((!isDirError && (path is null || + DirectoryExists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path)))))) { return !string.IsNullOrEmpty(path) ? new FileNotFoundException(SR.Format(SR.IO_FileNotFound_FileName, path), path) : diff --git a/src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs b/src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs index e1eb1b53e0ee43..dc42bde044b0c6 100644 --- a/src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs +++ b/src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs @@ -90,7 +90,7 @@ private void CheckStatfsResultAndThrowIfNecessary(int result) } else { - throw Interop.GetExceptionForIoErrno(errorInfo, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo); } } } diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs index 718bdd8608b2d6..80e5be15eb96cf 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs @@ -786,8 +786,7 @@ private bool TryReadEvent(out NotifyEvent notifyEvent) { fixed (byte* buf = &_buffer[0]) { - _bufferAvailable = Interop.CheckIo(Interop.Sys.Read(_inotifyHandle, buf, this._buffer.Length), - isDirectory: true); + _bufferAvailable = Interop.CheckIo(Interop.Sys.Read(_inotifyHandle, buf, this._buffer.Length)); Debug.Assert(_bufferAvailable <= this._buffer.Length); } } diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs index 7938fe9b92b89b..37202f31b7e8c0 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs @@ -160,7 +160,7 @@ internal unsafe RunningInstance( _fullDirectory = Interop.Sys.RealPath(_fullDirectory); if (_fullDirectory is null) { - throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), _fullDirectory, isDirectory: true); + throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), _fullDirectory, isDirError: true); } // Also ensure it has a trailing slash. diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index f9b12053f0f188..82e1fd90bc0f5b 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -105,6 +105,11 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int throw ex; } + if (error.Error == Interop.Error.EISDIR) + { + error = Interop.Error.EACCES.Info(); + } + Interop.CheckIo(error.Error, path); } @@ -305,7 +310,7 @@ private bool Init(string path, FileMode mode, FileAccess access, FileShare share if ((status.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) { - throw Interop.GetExceptionForIoErrno(Interop.Error.EACCES.Info(), path, isDirectory: true); + throw Interop.GetExceptionForIoErrno(Interop.Error.EACCES.Info(), path); } if ((status.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFREG) @@ -336,7 +341,7 @@ private bool Init(string path, FileMode mode, FileAccess access, FileShare share Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); if (errorInfo.Error == Interop.Error.EWOULDBLOCK) { - throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory: false); + throw Interop.GetExceptionForIoErrno(errorInfo, path); } } @@ -403,7 +408,7 @@ private bool Init(string path, FileMode mode, FileAccess access, FileShare share // We know the file descriptor is valid and we know the size argument to FTruncate is correct, // so if EBADF or EINVAL is returned, it means we're dealing with a special file that can't be // truncated. Ignore the error in such cases; in all others, throw. - throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory: false); + throw Interop.GetExceptionForIoErrno(errorInfo, path); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs index 846147ec3a7bc6..3f04333e6b3932 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs @@ -17,7 +17,7 @@ public static partial class Environment private static string CurrentDirectoryCore { get => Interop.Sys.GetCwd(); - set => Interop.CheckIo(Interop.Sys.ChDir(value), value, isDirectory: true); + set => Interop.CheckIo(Interop.Sys.ChDir(value), value, isDirError: true); } private static string ExpandEnvironmentVariablesCore(string name) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs index 430088be866d9a..10ad2d0226962a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs @@ -76,7 +76,7 @@ private IntPtr CreateDirectoryHandle(string path, bool ignoreNotFound = false) { return IntPtr.Zero; } - throw Interop.GetExceptionForIoErrno(info, path, isDirectory: true); + throw Interop.GetExceptionForIoErrno(info, path, isDirError: true); } return handle; } @@ -199,7 +199,7 @@ private unsafe void FindNextEntry(byte* entryBufferPtr, int bufferLength) } else { - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(result), _currentPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(result), _currentPath, isDirError: true); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index 28ac94b073c84f..6a8fc6411b27ec 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -434,15 +434,6 @@ private void SetUnixFileMode(SafeFileHandle? handle, string? path, UnixFileMode throw new ArgumentException(SR.Arg_InvalidUnixFileMode, nameof(UnixFileMode)); } - // Use ThrowNotFound to throw the appropriate exception when the file doesn't exist. - if (handle is null && path is not null) - { - EnsureCachesInitialized(path); - - if (!EntryExists || IsBrokenLink) - FileSystemInfo.ThrowNotFound(path); - } - // Linux does not support link permissions. // To have consistent cross-platform behavior we operate on the link target. int rv = handle is not null ? Interop.Sys.FChMod(handle, (int)mode) @@ -552,7 +543,7 @@ private static long UnixTimeSecondsToNanoseconds(DateTimeOffset time, long secon private void ThrowNotFound(string? path) { Interop.Error error = _state == InitializedNotExistsNotADir ? Interop.Error.ENOTDIR : Interop.Error.ENOENT; - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(error), path, isDirectory: false); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(error), path); } } } 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 3b33cd6152e0e4..5decf99b250f7a 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 @@ -89,34 +89,23 @@ private static void LinkOrCopyFile (string sourceFullPath, string destFullPath) { // The operation failed. Within reason, try to determine which path caused the problem // so we can throw a detailed exception. - string? path = null; - bool isDirectory = false; if (errorInfo.Error == Interop.Error.ENOENT) { if (!Directory.Exists(Path.GetDirectoryName(destFullPath))) { - // The parent directory of destFile can't be found. - // Windows distinguishes between whether the directory or the file isn't found, - // and throws a different exception in these cases. We attempt to approximate that - // here; there is a race condition here, where something could change between - // when the error occurs and our checks, but it's the best we can do, and the - // worst case in such a race condition (which could occur if the file system is - // being manipulated concurrently with these checks) is that we throw a - // FileNotFoundException instead of DirectoryNotFoundexception. - path = destFullPath; - isDirectory = true; + throw Interop.GetExceptionForIoErrno(errorInfo, destFullPath, isDirError: true); } else { - path = sourceFullPath; + throw Interop.GetExceptionForIoErrno(errorInfo, sourceFullPath); } } else if (errorInfo.Error == Interop.Error.EEXIST) { - path = destFullPath; + throw Interop.GetExceptionForIoErrno(errorInfo, destFullPath); } - throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory); + throw Interop.GetExceptionForIoErrno(errorInfo); } } @@ -125,12 +114,8 @@ public static void ReplaceFile(string sourceFullPath, string destFullPath, strin { // Unix rename works in more cases, we limit to what is allowed by Windows File.Replace. // These checks are not atomic, the file could change after a check was performed and before it is renamed. - Interop.Sys.FileStatus sourceStat; - if (Interop.Sys.LStat(sourceFullPath, out sourceStat) != 0) - { - Interop.ErrorInfo errno = Interop.Sys.GetLastErrorInfo(); - throw Interop.GetExceptionForIoErrno(errno, sourceFullPath); - } + Interop.CheckIo(Interop.Sys.LStat(sourceFullPath, out Interop.Sys.FileStatus sourceStat), sourceFullPath); + // Check source is not a directory. if ((sourceStat.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) { @@ -208,16 +193,7 @@ public static void MoveFile(string sourceFullPath, string destFullPath, bool ove } else { - // Windows distinguishes between whether the directory or the file isn't found, - // and throws a different exception in these cases. We attempt to approximate that - // here; there is a race condition here, where something could change between - // when the error occurs and our checks, but it's the best we can do, and the - // worst case in such a race condition (which could occur if the file system is - // being manipulated concurrently with these checks) is that we throw a - // FileNotFoundException instead of DirectoryNotFoundException. - throw Interop.GetExceptionForIoErrno(errorInfo, destFullPath, - isDirectory: errorInfo.Error == Interop.Error.ENOENT && !Directory.Exists(Path.GetDirectoryName(destFullPath)) // The parent directory of destFile can't be found - ); + throw Interop.GetExceptionForIoErrno(errorInfo, destFullPath); } } @@ -308,7 +284,7 @@ public static void CreateDirectory(string fullPath, UnixFileMode unixCreateMode) // macOS returns ENOTDIR when the path refers to a file and ends with '/'. // Trim the separator so we get EEXIST instead. ReadOnlySpan path = PathInternal.TrimEndingDirectorySeparator(fullPath.AsSpan()); - int result = Interop.Sys.MkDir(fullPath, (int)unixCreateMode); + int result = Interop.Sys.MkDir(path, (int)unixCreateMode); if (result == 0) { return; // Created directory. @@ -325,7 +301,7 @@ public static void CreateDirectory(string fullPath, UnixFileMode unixCreateMode) } else { - throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, fullPath); } } @@ -375,7 +351,7 @@ private static void CreateParentsAndDirectory(string fullPath, UnixFileMode unix } else { - throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString(), isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString()); } i--; } while (i > 0); @@ -403,7 +379,7 @@ private static void CreateParentsAndDirectory(string fullPath, UnixFileMode unix } } - throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString(), isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath.ToString()); } } } @@ -472,7 +448,7 @@ private static void MoveDirectory(string sourceFullPath, string destFullPath, bo case Interop.Error.ENOTDIR: // sourceFullPath exists and it's not a directory throw new IOException(SR.Format(SR.IO_PathNotFound_Path, sourceFullPath)); default: - throw Interop.GetExceptionForIoErrno(errorInfo, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo); } } } @@ -579,7 +555,7 @@ private static bool RemoveEmptyDirectory(string fullPath, bool topLevel = false, throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, fullPath)); } - throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirError: true); } return true; @@ -598,7 +574,7 @@ public static FileAttributes GetAttributes(string fullPath) FileAttributes attributes = new FileInfo(fullPath, null).Attributes; if (attributes == (FileAttributes)(-1)) - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.ENOENT), fullPath, isDirectory: false); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.ENOENT), fullPath); return attributes; } @@ -611,7 +587,7 @@ public static UnixFileMode GetUnixFileMode(string fullPath) UnixFileMode mode = default(FileStatus).GetUnixFileMode(fullPath); if (mode == (UnixFileMode)(-1)) - FileSystemInfo.ThrowNotFound(fullPath); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.ENOENT), fullPath); return mode; } @@ -654,7 +630,7 @@ public static string[] GetLogicalDrives() internal static void CreateSymbolicLink(string path, string pathToTarget, bool isDirectory) { - Interop.CheckIo(Interop.Sys.SymLink(pathToTarget, path), path, isDirectory); + Interop.CheckIo(Interop.Sys.SymLink(pathToTarget, path), path); } internal static FileSystemInfo? ResolveLinkTarget(string linkPath, bool returnFinalTarget, bool isDirectory) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index c9ecc55ca59c54..11f70114c9d53b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -262,7 +262,7 @@ private static void GetFindData(string fullPath, bool isDirectory, bool ignoreAc // File not found doesn't make much sense coming from a directory. if (isDirectory && errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND) errorCode = Interop.Errors.ERROR_PATH_NOT_FOUND; - if (isDirectory && errorCode == Interop.Errors.ERROR_ACCESS_DENIED && ignoreAccessDenied) + if (ignoreAccessDenied && errorCode == Interop.Errors.ERROR_ACCESS_DENIED) return; throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs index 534a49217408cb..9e2f06d2d015ff 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs @@ -23,7 +23,7 @@ internal static long CheckFileCall(long result, string? path, bool ignoreNotSupp Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); if (!(ignoreNotSupported && errorInfo.Error == Interop.Error.ENOTSUP)) { - throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory: false); + throw Interop.GetExceptionForIoErrno(errorInfo, path); } } @@ -51,7 +51,7 @@ internal static void FlushToDisk(SafeFileHandle handle) // In such cases there's nothing to flush. break; default: - throw Interop.GetExceptionForIoErrno(errorInfo, handle.Path, isDirectory: false); + throw Interop.GetExceptionForIoErrno(errorInfo, handle.Path); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs index 3426ac0ce25d8f..24debdff36dfa0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs @@ -194,7 +194,7 @@ private static unsafe void EnumerateFilesRecursively(string path, Predicate Date: Fri, 29 Jul 2022 06:22:31 +0200 Subject: [PATCH 5/5] Call Directory.Exists instead of using Stat. --- .../src/Interop/Unix/Interop.IOErrors.cs | 19 ++++--------------- .../System.Console/src/System.Console.csproj | 2 -- .../src/System.IO.FileSystem.DriveInfo.csproj | 2 -- .../src/System.IO.Ports.csproj | 2 -- .../src/System.Net.Http.csproj | 2 -- 5 files changed, 4 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 3cbcd513047f52..88ad235ca360a2 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -110,8 +110,7 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa case Error.ENOENT: // For Windows compatibility, throw DirectoryNotFoundException instead of FileNotFoundException // when the parent folder does not exist. - if ((!isDirError && (path is null || - DirectoryExists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path)))))) + if (!isDirError && (path is null || ParentDirectoryExists(path))) { return !string.IsNullOrEmpty(path) ? new FileNotFoundException(SR.Format(SR.IO_FileNotFound_FileName, path), path) : @@ -158,21 +157,11 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa default: return GetIOException(errorInfo, path); - static bool DirectoryExists(string? fullPath) + static bool ParentDirectoryExists(string fullPath) { - if (fullPath is null) - { - return false; - } - - Interop.Sys.FileStatus fileinfo; - - if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) - { - return false; - } + string? parentPath = Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(fullPath)); - return ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR); + return Directory.Exists(parentPath); } } } diff --git a/src/libraries/System.Console/src/System.Console.csproj b/src/libraries/System.Console/src/System.Console.csproj index fbda8d188edf11..f9303bfe1dbaab 100644 --- a/src/libraries/System.Console/src/System.Console.csproj +++ b/src/libraries/System.Console/src/System.Console.csproj @@ -62,8 +62,6 @@ Link="Common\Interop\Unix\Interop.FileDescriptors.cs" /> - diff --git a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj index 8488c6947f30be..690d1aeb7a94b7 100644 --- a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj +++ b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj @@ -63,8 +63,6 @@ Link="Common\Interop\Unix\Interop.UnixFileSystemTypes.cs" /> - Link="Common\Interop\Unix\Interop.Errors.cs" /> - -