From 7594d2c0977497c81db0c394f775832689bad492 Mon Sep 17 00:00:00 2001
From: Jan Philipp Hafer <jan.hafer@rwth-aachen.de>
Date: Mon, 1 May 2023 18:22:28 +0200
Subject: [PATCH] address review by user @squeek502

---
 lib/std/fs/test.zig    | 35 +++++++++++++++++++++++++++--------
 lib/std/os/windows.zig | 25 +++++++++++--------------
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig
index 15c8307f58..660ec0c381 100644
--- a/lib/std/fs/test.zig
+++ b/lib/std/fs/test.zig
@@ -1416,23 +1416,42 @@ test "File.PermissionsUnix" {
     try testing.expect(!permissions_unix.unixHas(.other, .execute));
 }
 
-test "delete a read-only file on windows" {
-    if (builtin.os.tag != .windows) return error.SkipZigTest;
+test "delete a read-only file on windows with file pending semantics" {
+    if (builtin.os.tag != .windows or builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1))
+        return error.SkipZigTest;
+
+    var tmp = tmpDir(.{});
+    defer tmp.cleanup();
+    {
+        const file = try tmp.dir.createFile("test_file", .{ .read = true });
+        defer file.close();
+        // Create a file and make it read-only
+        const metadata = try file.metadata();
+        var permissions = metadata.permissions();
+        permissions.setReadOnly(true);
+        try file.setPermissions(permissions);
+        try testing.expectError(error.AccessDenied, tmp.dir.deleteFile("test_file"));
+        // Now make the file not read-only
+        permissions.setReadOnly(false);
+        try file.setPermissions(permissions);
+    }
+    try tmp.dir.deleteFile("test_file");
+}
+
+test "delete a read-only file on windows with posix semantis" {
+    if (builtin.os.tag != .windows or !builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1))
+        return error.SkipZigTest;
 
     var tmp = tmpDir(.{});
     defer tmp.cleanup();
     const file = try tmp.dir.createFile("test_file", .{ .read = true });
+    defer file.close();
     // Create a file and make it read-only
     const metadata = try file.metadata();
     var permissions = metadata.permissions();
     permissions.setReadOnly(true);
     try file.setPermissions(permissions);
-    try testing.expectError(error.AccessDenied, tmp.dir.deleteFile("test_file"));
-    // Now make the file not read-only
-    permissions.setReadOnly(false);
-    try file.setPermissions(permissions);
-    file.close();
-    try tmp.dir.deleteFile("test_file");
+    try tmp.dir.deleteFile("test_file"); // file is unmapped and deleted once last handle closed
 }
 
 test "delete a setAsCwd directory on Windows" {
diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig
index e6ce95713c..ef2643792e 100644
--- a/lib/std/os/windows.zig
+++ b/lib/std/os/windows.zig
@@ -938,6 +938,7 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil
         else => return unexpectedStatus(rc),
     }
     defer CloseHandle(tmp_handle);
+
     if (comptime builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) {
         // Deletion with posix semantics.
         var info = FILE_DISPOSITION_INFORMATION_EX{
@@ -953,17 +954,13 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil
             @sizeOf(FILE_DISPOSITION_INFORMATION_EX),
             .FileDispositionInformationEx,
         );
-        switch (rc) {
-            .SUCCESS => {},
-            .CANNOT_DELETE => return error.FileBusy, // file is currently mapped
-            else => return unexpectedStatus(rc),
-        }
     } else {
         // Deletion with file pending semantics, which requires waiting or moving
         // files to get them removed (from here).
         var file_dispo = FILE_DISPOSITION_INFORMATION{
             .DeleteFile = TRUE,
         };
+
         rc = ntdll.NtSetInformationFile(
             tmp_handle,
             &io,
@@ -971,15 +968,15 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil
             @sizeOf(FILE_DISPOSITION_INFORMATION),
             .FileDispositionInformation,
         );
-        switch (rc) {
-            .SUCCESS => {},
-            .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty,
-            .INVALID_PARAMETER => unreachable,
-            .CANNOT_DELETE => return error.AccessDenied,
-            .MEDIA_WRITE_PROTECTED => return error.AccessDenied,
-            .ACCESS_DENIED => return error.AccessDenied,
-            else => return unexpectedStatus(rc),
-        }
+    }
+    switch (rc) {
+        .SUCCESS => {},
+        .DIRECTORY_NOT_EMPTY => return error.DirNotEmpty,
+        .INVALID_PARAMETER => unreachable,
+        .CANNOT_DELETE => return error.AccessDenied,
+        .MEDIA_WRITE_PROTECTED => return error.AccessDenied,
+        .ACCESS_DENIED => return error.AccessDenied,
+        else => return unexpectedStatus(rc),
     }
 }