From 0bf9c76ae45953aae79a86ea732554316122b963 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 13 Dec 2021 11:00:00 +0100 Subject: [PATCH 1/3] Refactor FileStatus.Unix. - Moves InitiallyDirectory out of FileStatus into FileSystemInfo. In FileSystemInfo it can be a readonly field making its usage clearer. And FileStatus can then directly be used to implement some FileSystem methods without allocating an intermediate FileInfo/DirectoryInfo. - Treat not exists/exist as initialized states to avoid wrongly assuming initialized means the file cache is valid, which isn't so when the file does not exist. - Use 0 for tracking uninitialized to make default(FileStatus) uninitialized. --- .../tests/Directory/SymbolicLinks.cs | 4 +- .../tests/DirectoryInfo/SymbolicLinks.cs | 4 +- .../IO/Enumeration/FileSystemEntry.Unix.cs | 14 +- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 10 +- .../IO/FileStatus.SetTimes.OtherUnix.cs | 8 +- .../src/System/IO/FileStatus.Unix.cs | 192 ++++++++---------- .../src/System/IO/FileSystem.Unix.cs | 40 +--- .../src/System/IO/FileSystemInfo.Unix.cs | 20 +- .../src/System/IO/FileSystemInfo.cs | 1 - 9 files changed, 128 insertions(+), 165 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs b/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs index 43106e3c2a6b79..4c4da0f4847f75 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs @@ -49,8 +49,8 @@ protected override void AssertLinkExists(FileSystemInfo link) { // Unix implementation detail: // When the directory target does not exist FileStatus.GetExists returns false because: - // - We check _exists (which whould be true because the link itself exists). - // - We check InitiallyDirectory, which is the initial expected object type (which would be true). + // - We check FileExists (which whould be true because the link itself exists). + // - We check _asDirectory, which is the initial expected object type (which would be true). // - We check _directory (false because the target directory does not exist) Assert.False(link.Exists); } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs index 84658b6f275b96..511beeda1a454f 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs @@ -46,8 +46,8 @@ protected override void AssertLinkExists(FileSystemInfo link) { // Unix implementation detail: // When the directory target does not exist FileStatus.GetExists returns false because: - // - We check _exists (which whould be true because the link itself exists). - // - We check InitiallyDirectory, which is the initial expected object type (which would be true). + // - We check FileExists (which whould be true because the link itself exists). + // - We check _asDirectory, which is the initial expected object type (which would be true). // - We check _directory (false because the target directory does not exist) Assert.False(link.Exists); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 1be3942011b2fd..aa16728f8546fa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -11,6 +11,7 @@ namespace System.IO.Enumeration public unsafe ref partial struct FileSystemEntry { private Interop.Sys.DirectoryEntry _directoryEntry; + private bool _isDirectory; private FileStatus _status; private Span _pathBuffer; private ReadOnlySpan _fullPath; @@ -32,8 +33,8 @@ internal static FileAttributes Initialize( entry._pathBuffer = pathBuffer; entry._fullPath = ReadOnlySpan.Empty; entry._fileName = ReadOnlySpan.Empty; + entry._isDirectory = false; entry._status.InvalidateCaches(); - entry._status.InitiallyDirectory = false; bool isDirectory = directoryEntry.InodeType == Interop.Sys.NodeType.DT_DIR; bool isSymlink = directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK; @@ -41,15 +42,15 @@ internal static FileAttributes Initialize( if (isDirectory) { - entry._status.InitiallyDirectory = true; + entry._isDirectory = true; } else if (isSymlink) { - entry._status.InitiallyDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + entry._isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); } else if (isUnknown) { - entry._status.InitiallyDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + entry._isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); if (entry._status.IsSymbolicLink(entry.FullPath, continueOnError: true)) { entry._directoryEntry.InodeType = Interop.Sys.NodeType.DT_LNK; @@ -145,16 +146,17 @@ public FileAttributes Attributes public DateTimeOffset CreationTimeUtc => _status.GetCreationTime(FullPath, continueOnError: true); public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true); public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); + public bool IsHidden => _status.IsHidden(FullPath, FileName, continueOnError: true); internal bool IsReadOnly => _status.IsReadOnly(FullPath, continueOnError: true); - public bool IsDirectory => _status.InitiallyDirectory; + public bool IsDirectory => _isDirectory; internal bool IsSymbolicLink => _directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK; public FileSystemInfo ToFileSystemInfo() { string fullPath = ToFullPath(); - return FileSystemInfo.Create(fullPath, new string(FileName), ref _status); + return FileSystemInfo.Create(fullPath, new string(FileName), _isDirectory, ref _status); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 23c46bae4a0125..40a887e7210be8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -7,7 +7,7 @@ namespace System.IO { internal partial struct FileStatus { - internal void SetCreationTime(string path, DateTimeOffset time) + internal void SetCreationTime(string path, DateTimeOffset time, bool asDirectory) { // Try to set the attribute on the file system entry using setattrlist, // if we get ENOTSUP then it means that "The volume does not support @@ -27,11 +27,11 @@ internal void SetCreationTime(string path, DateTimeOffset time) } else if (error == Interop.Error.ENOTSUP) { - SetAccessOrWriteTimeCore(path, time, isAccessTime: false, checkCreationTime: false); + SetAccessOrWriteTimeCore(path, time, isAccessTime: false, checkCreationTime: false, asDirectory); } else { - Interop.CheckIo(error, path, InitiallyDirectory); + Interop.CheckIo(error, path, asDirectory); } } @@ -54,7 +54,7 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long return error; } - private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) => - SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: true); + private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime, bool asDirectory) => + SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: true, asDirectory); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs index d9d9b1eeb7c920..85b01fde17d7c1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs @@ -7,11 +7,11 @@ namespace System.IO { internal partial struct FileStatus { - internal void SetCreationTime(string path, DateTimeOffset time) => - SetLastWriteTime(path, time); + internal void SetCreationTime(string path, DateTimeOffset time, bool asDirectory) => + SetLastWriteTime(path, time, asDirectory); - private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) => - SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: false); + private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime, bool asDirectory) => + SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: false, asDirectory); // This is not used on these platforms, but is needed for source compat private Interop.Error SetCreationTimeCore(string path, long seconds, long nanoseconds) => 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 05b4bbcff6a2aa..cd8d67ecdbb29c 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,44 +10,46 @@ internal partial struct FileStatus { private const int NanosecondsPerTick = 100; - // The last cached lstat information about the file + private const int InitializedExists = -2; // entry exists. + private const int InitializedNotExists = -1; // no entry exists. + private const int Uninitialized = 0; // uninitialized, '0' to make default(FileStatus) uninitialized. + + // Tracks the initialization state. + // < 0 : initialized succesfully. Value is InitializedExists or InitializedNotExists. + // 0 : uninitialized. + // > 0 : initialized with error. Value is raw errno. + private int _initialized; + + // The last cached lstat information about the file. + // Must only be used after calling EnsureCachesInitialized and checking FileExists is true. private Interop.Sys.FileStatus _fileCache; - // -1: if the file cache isn't initialized - Refresh should always change this value - // 0: if the file cache was initialized with no errors - // Positive number: the error code returned by the lstat call - private int _initializedFileCache; - - // We track intent of creation to know whether or not we want to (1) create a - // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. - // Set to true during initialization when the DirectoryEntry's INodeType describes a directory - internal bool InitiallyDirectory { get; set; } - - // Is a directory as of the last refresh - // Its value can come from either the main path or the symbolic link path + // Is a directory as of the last refresh. + // Its value can come from either the main path or the symbolic link target. + // Must only be used after calling EnsureCachesInitialized. private bool _isDirectory; - // Exists as of the last refresh - private bool _exists; + private bool FileExists => _initialized == InitializedExists; - private bool IsFileCacheInitialized => _initializedFileCache == 0; + // Check if the main path (without following symlinks) has the hidden attribute set. + private bool HasHiddenFlag + { + get + { + Debug.Assert(_initialized != Uninitialized); // Use this after EnsureCachesInitialized has been called. - // Check if the main path (without following symlinks) has the hidden attribute set - // Ideally, use this if Refresh has been successfully called at least once. - // But since it is also used for soft retrieval during FileSystemEntry initialization, - // we return false early if the cache has not been initialized - internal bool HasHiddenFlag => IsFileCacheInitialized && - (_fileCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN; + return FileExists && (_fileCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN; + } + } - // Checks if the main path (without following symlinks) has the read-only attribute set - // Ideally, use this if Refresh has been successfully called at least once. - // But since it is also used for soft retrieval during FileSystemEntry initialization, - // we return false early if the cache has not been initialized - internal bool HasReadOnlyFlag + // Checks if the main path (without following symlinks) has the read-only attribute set. + private bool HasReadOnlyFlag { get { - if (!IsFileCacheInitialized) + Debug.Assert(_initialized != Uninitialized); // Use this after EnsureCachesInitialized has been called. + + if (!FileExists) { return false; } @@ -78,6 +80,7 @@ internal bool HasReadOnlyFlag #if !TARGET_BROWSER // HasReadOnlyFlag cache. + // Must only be used after calling EnsureCachesInitialized. private int _isReadOnlyCache; private bool IsModeReadOnlyCore() @@ -123,20 +126,20 @@ private bool IsModeReadOnlyCore() #endif // Checks if the main path is a symbolic link - // Only call if Refresh has been successfully called at least once private bool HasSymbolicLinkFlag { get { - Debug.Assert(IsFileCacheInitialized); - return (_fileCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; + Debug.Assert(_initialized != Uninitialized); // Use this after EnsureCachesInitialized has been called. + + return FileExists && (_fileCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; } } - // Sets the cache initialization flags to -1, which means the caches are now uninitialized + // Sets the cache initialization flags to 0, which means the caches are now uninitialized internal void InvalidateCaches() { - _initializedFileCache = -1; + _initialized = Uninitialized; } internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) @@ -176,7 +179,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan path, ReadOnlySpan