Skip to content

Commit c2f995e

Browse files
committed
address code review feedback: if NtCreateFile fails, we should immediately throw an exception and don't call CreateFile
1 parent 37b9245 commit c2f995e

File tree

3 files changed

+27
-50
lines changed

3 files changed

+27
-50
lines changed

src/libraries/Common/src/Interop/Windows/NtDll/Interop.NtCreateFile.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ private static DesiredAccess GetDesiredAccess(FileAccess access, FileMode fileMo
121121
{
122122
result |= DesiredAccess.SYNCHRONIZE; // requried by FILE_SYNCHRONOUS_IO_NONALERT
123123
}
124+
if ((options & FileOptions.DeleteOnClose) != 0 || fileMode == FileMode.Create)
125+
{
126+
result |= DesiredAccess.DELETE; // required by FILE_DELETE_ON_CLOSE and FILE_SUPERSEDE (which deletes a file if it exsits)
127+
}
124128

125129
return result;
126130
}
@@ -153,7 +157,7 @@ private static CreateOptions GetCreateOptions(FileOptions options)
153157
}
154158
if ((options & FileOptions.DeleteOnClose) != 0)
155159
{
156-
result |= CreateOptions.FILE_DELETE_ON_CLOSE;
160+
result |= CreateOptions.FILE_DELETE_ON_CLOSE; // has extra handling in GetDesiredAccess
157161
}
158162
if ((options & FileOptions.Asynchronous) == 0)
159163
{

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,9 @@
13471347
<Compile Include="$(CommonPath)Interop\Windows\NtDll\Interop.NtCreateFile.cs">
13481348
<Link>Common\Interop\Windows\NtDll\Interop.NtCreateFile.cs</Link>
13491349
</Compile>
1350+
<Compile Include="$(CommonPath)Interop\Windows\NtDll\Interop.RtlNtStatusToDosError.cs">
1351+
<Link>Common\Interop\Windows\NtDll\Interop.RtlNtStatusToDosError.cs</Link>
1352+
</Compile>
13501353
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DeleteFile.cs">
13511354
<Link>Common\Interop\Windows\Kernel32\Interop.DeleteFile.cs</Link>
13521355
</Compile>

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

Lines changed: 19 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,10 @@ private static unsafe SafeFileHandle CreateFileOpenHandle(string path, FileMode
7272
{
7373
Debug.Assert(path != null);
7474

75-
if (TryNtCreateFile(path, mode, access, share, options, allocationSize, out uint ntStatus, out IntPtr fileHandle))
75+
if (ShouldPreallocate(allocationSize, access, mode))
7676
{
77+
IntPtr fileHandle = NtCreateFile(path, mode, access, share, options, allocationSize);
78+
7779
return ValidateFileHandle(new SafeFileHandle(fileHandle, ownsHandle: true), path, (options & FileOptions.Asynchronous) != 0);
7880
}
7981

@@ -104,43 +106,14 @@ private static unsafe SafeFileHandle CreateFileOpenHandle(string path, FileMode
104106
path,
105107
(options & FileOptions.Asynchronous) != 0);
106108

107-
if (ntStatus == NT_STATUS_INVALID_PARAMETER)
108-
{
109-
// It seems that NtCreateFile has a bug and it reports STATUS_INVALID_PARAMETER for files
110-
// that are too big for the current file system. Example: creating a 4GB+1 file on a FAT32 drive.
111-
// Since Linux reports EFBIG for such cases, we are using the following workaround to get the right exception.
112-
// TrySetFileLength uses SetFileInformationByHandle which fails with a clear error if the file is too big.
113-
if (!TrySetFileLength(safeFileHandle, allocationSize, out int errorCode))
114-
{
115-
// Since we have failed to extend the file, we mimic the NtCreateFile behaviour
116-
// which does not create a file if there is not enough space on the disk.
117-
// So we close the handle, remove the file and then throw an exception.
118-
safeFileHandle.Dispose();
119-
Interop.Kernel32.DeleteFile(path);
120-
121-
if (errorCode == Interop.Errors.ERROR_FILE_TOO_LARGE // this is what we would expect in such a case
122-
|| errorCode == Interop.Errors.ERROR_DISK_FULL) // but this is what we get (verified with Windows 10.0.18363.1500)
123-
{
124-
throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, path, allocationSize));
125-
}
126-
127-
throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
128-
}
129-
}
130-
131109
return safeFileHandle;
132110
}
133111
}
134112

135-
private static bool TryNtCreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long allocationSize,
136-
out uint ntStatus, out IntPtr fileHandle)
113+
private static IntPtr NtCreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long allocationSize)
137114
{
138-
if (!ShouldPreallocate(allocationSize, access, mode))
139-
{
140-
ntStatus = 0;
141-
fileHandle = default;
142-
return false;
143-
}
115+
uint ntStatus;
116+
IntPtr fileHandle;
144117

145118
const string mandatoryNtPrefix = @"\??\";
146119
if (fullPath.StartsWith(mandatoryNtPrefix, StringComparison.Ordinal))
@@ -165,24 +138,21 @@ private static bool TryNtCreateFile(string fullPath, FileMode mode, FileAccess a
165138
vsb.Dispose();
166139
}
167140

168-
if (ntStatus == 0)
141+
switch (ntStatus)
169142
{
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));
143+
case 0:
144+
return fileHandle;
145+
case NT_ERROR_STATUS_DISK_FULL:
146+
throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, allocationSize));
147+
// NtCreateFile has a bug and it reports STATUS_INVALID_PARAMETER for files
148+
// that are too big for the current file system. Example: creating a 4GB+1 file on a FAT32 drive.
149+
case NT_STATUS_INVALID_PARAMETER:
150+
case NT_ERROR_STATUS_FILE_TOO_LARGE:
151+
throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, allocationSize));
152+
default:
153+
int error = (int)Interop.NtDll.RtlNtStatusToDosError((int)ntStatus);
154+
throw Win32Marshal.GetExceptionForWin32Error(error, fullPath);
179155
}
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;
186156
}
187157

188158
internal static bool GetDefaultIsAsync(SafeFileHandle handle, bool defaultIsAsync)

0 commit comments

Comments
 (0)