Skip to content

Commit 37b9245

Browse files
committed
address code review feedback: remove unused path parameter, use stackalloc for small inputs
1 parent 83f119d commit 37b9245

File tree

1 file changed

+59
-60
lines changed

1 file changed

+59
-60
lines changed

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs

Lines changed: 59 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Diagnostics;
66
using System.Runtime.CompilerServices;
77
using System.Runtime.InteropServices;
8+
using System.Text;
89
using System.Threading;
910
using System.Threading.Tasks;
1011
using Microsoft.Win32.SafeHandles;
@@ -71,35 +72,9 @@ private static unsafe SafeFileHandle CreateFileOpenHandle(string path, FileMode
7172
{
7273
Debug.Assert(path != null);
7374

74-
uint ntStatus = 0;
75-
if (ShouldPreallocate(allocationSize, access, mode))
75+
if (TryNtCreateFile(path, mode, access, share, options, allocationSize, out uint ntStatus, out IntPtr fileHandle))
7676
{
77-
GetPathForNtCreateFile(path, out ReadOnlySpan<char> prefixedAbsolutePath, out char[]? rentedArray);
78-
79-
(ntStatus, IntPtr fileHandle) = Interop.NtDll.CreateFile(prefixedAbsolutePath, mode, access, share, options, allocationSize);
80-
81-
if (rentedArray is not null)
82-
{
83-
ArrayPool<char>.Shared.Return(rentedArray);
84-
}
85-
86-
if (ntStatus == 0)
87-
{
88-
return ValidateFileHandle(new SafeFileHandle(fileHandle, ownsHandle: true), path, (options & FileOptions.Asynchronous) != 0);
89-
}
90-
else if (ntStatus == NT_ERROR_STATUS_DISK_FULL)
91-
{
92-
throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, path, allocationSize));
93-
}
94-
else if (ntStatus == NT_ERROR_STATUS_FILE_TOO_LARGE)
95-
{
96-
throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, path, allocationSize));
97-
}
98-
99-
// NtCreateFile has failed for some other reason than a full disk or too large file.
100-
// Instead of implementing the mapping for every NS Status value (there are plenty of them: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55)
101-
// or using RtlNtStatusToDosError & GetExceptionForWin32Error
102-
// the code falls back to CreateFileW that just throws the right exception.
77+
return ValidateFileHandle(new SafeFileHandle(fileHandle, ownsHandle: true), path, (options & FileOptions.Asynchronous) != 0);
10378
}
10479

10580
Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(share);
@@ -135,7 +110,7 @@ private static unsafe SafeFileHandle CreateFileOpenHandle(string path, FileMode
135110
// that are too big for the current file system. Example: creating a 4GB+1 file on a FAT32 drive.
136111
// Since Linux reports EFBIG for such cases, we are using the following workaround to get the right exception.
137112
// TrySetFileLength uses SetFileInformationByHandle which fails with a clear error if the file is too big.
138-
if (!TrySetFileLength(safeFileHandle, path, allocationSize, out int errorCode))
113+
if (!TrySetFileLength(safeFileHandle, allocationSize, out int errorCode))
139114
{
140115
// Since we have failed to extend the file, we mimic the NtCreateFile behaviour
141116
// which does not create a file if there is not enough space on the disk.
@@ -157,6 +132,59 @@ private static unsafe SafeFileHandle CreateFileOpenHandle(string path, FileMode
157132
}
158133
}
159134

135+
private static bool TryNtCreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long allocationSize,
136+
out uint ntStatus, out IntPtr fileHandle)
137+
{
138+
if (!ShouldPreallocate(allocationSize, access, mode))
139+
{
140+
ntStatus = 0;
141+
fileHandle = default;
142+
return false;
143+
}
144+
145+
const string mandatoryNtPrefix = @"\??\";
146+
if (fullPath.StartsWith(mandatoryNtPrefix, StringComparison.Ordinal))
147+
{
148+
(ntStatus, fileHandle) = Interop.NtDll.CreateFile(fullPath, mode, access, share, options, allocationSize);
149+
}
150+
else
151+
{
152+
var vsb = new ValueStringBuilder(stackalloc char[1024]);
153+
vsb.Append(mandatoryNtPrefix);
154+
155+
if (fullPath.StartsWith(@"\\?\", StringComparison.Ordinal)) // NtCreateFile does not support "\\?\" prefix, only "\??\"
156+
{
157+
vsb.Append(fullPath.AsSpan(4));
158+
}
159+
else
160+
{
161+
vsb.Append(fullPath);
162+
}
163+
164+
(ntStatus, fileHandle) = Interop.NtDll.CreateFile(vsb.AsSpan(), mode, access, share, options, allocationSize);
165+
vsb.Dispose();
166+
}
167+
168+
if (ntStatus == 0)
169+
{
170+
return true;
171+
}
172+
else if (ntStatus == NT_ERROR_STATUS_DISK_FULL)
173+
{
174+
throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, allocationSize));
175+
}
176+
else if (ntStatus == NT_ERROR_STATUS_FILE_TOO_LARGE)
177+
{
178+
throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, allocationSize));
179+
}
180+
181+
// NtCreateFile has failed for some other reason than a full disk or too large file.
182+
// Instead of implementing the mapping for every NS Status value (there are plenty of them: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55)
183+
// or using RtlNtStatusToDosError & GetExceptionForWin32Error that end up throwing a different exception compared to when NtCreateFile is not involved,
184+
// the code falls back to CreateFileW that just throws the right exception.
185+
return false;
186+
}
187+
160188
internal static bool GetDefaultIsAsync(SafeFileHandle handle, bool defaultIsAsync)
161189
{
162190
return handle.IsAsync ?? !IsHandleSynchronous(handle, ignoreInvalid: true) ?? defaultIsAsync;
@@ -383,7 +411,7 @@ internal static void GetFileTypeSpecificInformation(SafeFileHandle handle, out b
383411

384412
internal static unsafe void SetFileLength(SafeFileHandle handle, string? path, long length)
385413
{
386-
if (!TrySetFileLength(handle, path, length, out int errorCode))
414+
if (!TrySetFileLength(handle, length, out int errorCode))
387415
{
388416
if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER)
389417
{
@@ -394,7 +422,7 @@ internal static unsafe void SetFileLength(SafeFileHandle handle, string? path, l
394422
}
395423
}
396424

397-
private static unsafe bool TrySetFileLength(SafeFileHandle handle, string? path, long length, out int errorCode)
425+
private static unsafe bool TrySetFileLength(SafeFileHandle handle, long length, out int errorCode)
398426
{
399427
var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO
400428
{
@@ -480,35 +508,6 @@ internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<b
480508
}
481509
}
482510

483-
private static void GetPathForNtCreateFile(string fullPath, out ReadOnlySpan<char> prefixedAbsolutePath, out char[]? rentedArray)
484-
{
485-
const string mandatoryNtPrefix = @"\??\";
486-
487-
if (fullPath.StartsWith(mandatoryNtPrefix, StringComparison.Ordinal))
488-
{
489-
prefixedAbsolutePath = fullPath;
490-
rentedArray = null;
491-
}
492-
else if (fullPath.StartsWith(@"\\?\", StringComparison.Ordinal)) // NtCreateFile does not support "\\?\" prefix, only "\??\"
493-
{
494-
rentedArray = ArrayPool<char>.Shared.Rent(fullPath.Length);
495-
496-
fullPath.CopyTo(rentedArray);
497-
rentedArray[1] = '?';
498-
499-
prefixedAbsolutePath = new ReadOnlySpan<char>(rentedArray, 0, fullPath.Length);
500-
}
501-
else
502-
{
503-
rentedArray = ArrayPool<char>.Shared.Rent(mandatoryNtPrefix.Length + fullPath.Length);
504-
505-
mandatoryNtPrefix.CopyTo(rentedArray);
506-
fullPath.CopyTo(rentedArray.AsSpan(mandatoryNtPrefix.Length));
507-
508-
prefixedAbsolutePath = new ReadOnlySpan<char>(rentedArray, 0, mandatoryNtPrefix.Length + fullPath.Length);
509-
}
510-
}
511-
512511
internal static async Task AsyncModeCopyToAsync(SafeFileHandle handle, string? path, bool canSeek, long filePosition, Stream destination, int bufferSize, CancellationToken cancellationToken)
513512
{
514513
// For efficiency, we avoid creating a new task and associated state for each asynchronous read.

0 commit comments

Comments
 (0)