[PATCH 0/4] MR3073: Draft: Implement FileDispositionInformationEx and FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE

[Joel Holdsworth (＠jhol)]

This MR supercedes !1895 as a solution to bug [#50771](https://bugs.winehq.org/show_bug.cgi?id=50771). Rather than fixing the problem of Wine's inability to modify the attributes of read-only files, this patch set instead implements `FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE` which the Msys2 and Cygwin runtime libriries can use to avoid needing to modify attributes.

[PATCH 1/4] include: Defined FILE_DISPOSITION_INFORMATION_EX and friends

[Joel Holdsworth]

From: Joel Holdsworth

Signed-off-by: Joel Holdsworth 
---
 include/winternl.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/winternl.h b/include/winternl.h
index 455e199f3e1..1549f5b85f4 100644
--- a/include/winternl.h
+++ b/include/winternl.h
@@ -1255,6 +1255,7 @@ typedef enum _FILE_INFORMATION_CLASS {
     FileReplaceCompletionInformation,
     FileHardLinkFullIdInformation,
     FileIdExtdBothDirectoryInformation,
+    FileDispositionInformationEx,
     FileMaximumInformation
 } FILE_INFORMATION_CLASS, *PFILE_INFORMATION_CLASS;

@@ -1424,6 +1425,17 @@ typedef struct _FILE_DISPOSITION_INFORMATION {
     BOOLEAN DoDeleteFile;
 } FILE_DISPOSITION_INFORMATION, *PFILE_DISPOSITION_INFORMATION;

+typedef struct _FILE_DISPOSITION_INFORMATION_EX {
+    ULONG Flags;
+} FILE_DISPOSITION_INFORMATION_EX, *PFILE_DISPOSITION_INFORMATION_EX;
+
+#define FILE_DISPOSITION_DO_NOT_DELETE              0x00000000
+#define FILE_DISPOSITION_DELETE                     0x00000001
+#define FILE_DISPOSITION_POSIX_SEMANTICS            0x00000002
+#define FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK  0x00000004
+#define FILE_DISPOSITION_ON_CLOSE                   0x00000008
+#define FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE  0x00000010
+
 typedef struct _FILE_POSITION_INFORMATION {
     LARGE_INTEGER CurrentByteOffset;
 } FILE_POSITION_INFORMATION, *PFILE_POSITION_INFORMATION;

[PATCH 2/4] ntdll: Initial implementation of FileDispositionInformationEx

[Joel Holdsworth]

From: Joel Holdsworth

This is required by Msys2 when running gpg-agent.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54996
Signed-off-by: Joel Holdsworth 
---
 dlls/ntdll/unix/file.c         | 29 +++++++++++++++++++++++++++--
 include/wine/server_protocol.h | 14 +++++++-------
 server/fd.c                    | 12 ++++++------
 server/protocol.def            |  4 ++--
 server/request.h               | 10 +++++-----
 server/trace.c                 |  8 ++++----
 6 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index ff1618f31fd..da6c157ba71 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -4751,10 +4751,35 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
         {
             FILE_DISPOSITION_INFORMATION *info = ptr;

-            SERVER_START_REQ( set_fd_disp_info )
+            SERVER_START_REQ( set_fd_disp_info_ex )
             {
                 req->handle   = wine_server_obj_handle( handle );
-                req->unlink   = info->DoDeleteFile;
+                req->flags    = info->DoDeleteFile ? FILE_DISPOSITION_DELETE : FILE_DISPOSITION_DO_NOT_DELETE;
+                status = wine_server_call( req );
+            }
+            SERVER_END_REQ;
+        }
+        else status = STATUS_INVALID_PARAMETER_3;
+        break;
+
+    case FileDispositionInformationEx:
+        if (len >= sizeof(FILE_DISPOSITION_INFORMATION_EX))
+        {
+            FILE_DISPOSITION_INFORMATION_EX *info = ptr;
+
+            if (info->Flags & FILE_DISPOSITION_POSIX_SEMANTICS)
+                FIXME( "FILE_DISPOSITION_POSIX_SEMANTICS not supported\n" );
+            if (info->Flags & FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK)
+                FIXME( "FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK not supported\n" );
+            if (info->Flags & FILE_DISPOSITION_ON_CLOSE)
+                FIXME( "FILE_DISPOSITION_ON_CLOSE not supported\n" );
+            if (info->Flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE)
+                FIXME( "FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE not supported\n" );
+
+            SERVER_START_REQ( set_fd_disp_info_ex )
+            {
+                req->handle   = wine_server_obj_handle( handle );
+                req->flags    = info->Flags;
                 status = wine_server_call( req );
             }
             SERVER_END_REQ;
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h
index 1af69c90b55..4c67701344a 100644
--- a/include/wine/server_protocol.h
+++ b/include/wine/server_protocol.h
@@ -5226,14 +5226,14 @@ struct set_fd_completion_mode_reply

-struct set_fd_disp_info_request
+struct set_fd_disp_info_ex_request
 {
     struct request_header __header;
     obj_handle_t handle;
-    int          unlink;
+    unsigned int flags;
     char __pad_20[4];
 };
-struct set_fd_disp_info_reply
+struct set_fd_disp_info_ex_reply
 {
     struct reply_header __header;
 };
@@ -5819,7 +5819,7 @@ enum request
     REQ_set_completion_info,
     REQ_add_fd_completion,
     REQ_set_fd_completion_mode,
-    REQ_set_fd_disp_info,
+    REQ_set_fd_disp_info_ex,
     REQ_set_fd_name_info,
     REQ_set_fd_eof_info,
     REQ_get_window_layered_info,
@@ -6106,7 +6106,7 @@ union generic_request
     struct set_completion_info_request set_completion_info_request;
     struct add_fd_completion_request add_fd_completion_request;
     struct set_fd_completion_mode_request set_fd_completion_mode_request;
-    struct set_fd_disp_info_request set_fd_disp_info_request;
+    struct set_fd_disp_info_ex_request set_fd_disp_info_ex_request;
     struct set_fd_name_info_request set_fd_name_info_request;
     struct set_fd_eof_info_request set_fd_eof_info_request;
     struct get_window_layered_info_request get_window_layered_info_request;
@@ -6391,7 +6391,7 @@ union generic_reply
     struct set_completion_info_reply set_completion_info_reply;
     struct add_fd_completion_reply add_fd_completion_reply;
     struct set_fd_completion_mode_reply set_fd_completion_mode_reply;
-    struct set_fd_disp_info_reply set_fd_disp_info_reply;
+    struct set_fd_disp_info_ex_reply set_fd_disp_info_ex_reply;
     struct set_fd_name_info_reply set_fd_name_info_reply;
     struct set_fd_eof_info_reply set_fd_eof_info_reply;
     struct get_window_layered_info_reply get_window_layered_info_reply;
@@ -6417,7 +6417,7 @@ union generic_reply

/* ### protocol_version begin ### */

-#define SERVER_PROTOCOL_VERSION 776
+#define SERVER_PROTOCOL_VERSION 777

/* ### protocol_version end ### */

diff --git a/server/fd.c b/server/fd.c
index eaebe044f37..46d32416463 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -2463,7 +2463,7 @@ static int is_dir_empty( int fd )
 }

/* set disposition for the fd */
-static void set_fd_disposition( struct fd *fd, int unlink )
+static void set_fd_disposition_ex( struct fd *fd, unsigned int flags )
 {
     struct stat st;

@@ -2479,7 +2479,7 @@ static void set_fd_disposition( struct fd *fd, int unlink )
         return;
     }

-    if (unlink)
+    if (flags & FILE_DISPOSITION_DELETE)
     {
         struct fd *fd_ptr;

@@ -2524,7 +2524,7 @@ static void set_fd_disposition( struct fd *fd, int unlink )
         }
     }

-    fd->closed->unlink = unlink ? 1 : 0;
+    fd->closed->unlink = (flags & FILE_DISPOSITION_DELETE) ? 1 : 0;
     if (fd->options & FILE_DELETE_ON_CLOSE)
         fd->closed->unlink = -1;
 }
@@ -2949,13 +2949,13 @@ DECL_HANDLER(set_fd_completion_mode)
     }
 }

-/* set fd disposition information */
-DECL_HANDLER(set_fd_disp_info)
+/* set fd disposition extended information */
+DECL_HANDLER(set_fd_disp_info_ex)
 {
     struct fd *fd = get_handle_fd_obj( current->process, req->handle, DELETE );
     if (fd)
     {
-        set_fd_disposition( fd, req->unlink );
+        set_fd_disposition_ex( fd, req->flags );
         release_object( fd );
     }
 }
diff --git a/server/protocol.def b/server/protocol.def
index cd96858dea9..0f0732b4929 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -3639,9 +3639,9 @@ struct handle_info

/* set fd disposition information */
-@REQ(set_fd_disp_info)
+@REQ(set_fd_disp_info_ex)
     obj_handle_t handle;          /* handle to a file or directory */
-    int          unlink;          /* whether to unlink file on close */
+    unsigned int flags;           /* what actions should be taken when deleting a file */
 @END

diff --git a/server/request.h b/server/request.h
index 20e27ebdce1..447f93212de 100644
--- a/server/request.h
+++ b/server/request.h
@@ -377,7 +377,7 @@ DECL_HANDLER(query_completion);
 DECL_HANDLER(set_completion_info);
 DECL_HANDLER(add_fd_completion);
 DECL_HANDLER(set_fd_completion_mode);
-DECL_HANDLER(set_fd_disp_info);
+DECL_HANDLER(set_fd_disp_info_ex);
 DECL_HANDLER(set_fd_name_info);
 DECL_HANDLER(set_fd_eof_info);
 DECL_HANDLER(get_window_layered_info);
@@ -663,7 +663,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] =
     (req_handler)req_set_completion_info,
     (req_handler)req_add_fd_completion,
     (req_handler)req_set_fd_completion_mode,
-    (req_handler)req_set_fd_disp_info,
+    (req_handler)req_set_fd_disp_info_ex,
     (req_handler)req_set_fd_name_info,
     (req_handler)req_set_fd_eof_info,
     (req_handler)req_get_window_layered_info,
@@ -2206,9 +2206,9 @@ C_ASSERT( sizeof(struct add_fd_completion_request) == 40 );
 C_ASSERT( FIELD_OFFSET(struct set_fd_completion_mode_request, handle) == 12 );
 C_ASSERT( FIELD_OFFSET(struct set_fd_completion_mode_request, flags) == 16 );
 C_ASSERT( sizeof(struct set_fd_completion_mode_request) == 24 );
-C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_request, handle) == 12 );
-C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_request, unlink) == 16 );
-C_ASSERT( sizeof(struct set_fd_disp_info_request) == 24 );
+C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_ex_request, handle) == 12 );
+C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_ex_request, flags) == 16 );
+C_ASSERT( sizeof(struct set_fd_disp_info_ex_request) == 24 );
 C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, handle) == 12 );
 C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, rootdir) == 16 );
 C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, namelen) == 20 );
diff --git a/server/trace.c b/server/trace.c
index 4dbc5de352f..a064a9118ea 100644
--- a/server/trace.c
+++ b/server/trace.c
@@ -4357,10 +4357,10 @@ static void dump_set_fd_completion_mode_request( const struct set_fd_completion_
     fprintf( stderr, ", flags=%08x", req->flags );
 }

-static void dump_set_fd_disp_info_request( const struct set_fd_disp_info_request *req )
+static void dump_set_fd_disp_info_ex_request( const struct set_fd_disp_info_ex_request *req )
 {
     fprintf( stderr, " handle=%04x", req->handle );
-    fprintf( stderr, ", unlink=%d", req->unlink );
+    fprintf( stderr, ", flags=%08x", req->flags );
 }

static void dump_set_fd_name_info_request( const struct set_fd_name_info_request *req )
@@ -4813,7 +4813,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
     (dump_func)dump_set_completion_info_request,
     (dump_func)dump_add_fd_completion_request,
     (dump_func)dump_set_fd_completion_mode_request,
-    (dump_func)dump_set_fd_disp_info_request,
+    (dump_func)dump_set_fd_disp_info_ex_request,
     (dump_func)dump_set_fd_name_info_request,
     (dump_func)dump_set_fd_eof_info_request,
     (dump_func)dump_get_window_layered_info_request,
@@ -5379,7 +5379,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = {
     "set_completion_info",
     "add_fd_completion",
     "set_fd_completion_mode",
-    "set_fd_disp_info",
+    "set_fd_disp_info_ex",
     "set_fd_name_info",
     "set_fd_eof_info",
     "get_window_layered_info",

[PATCH 3/4] ntdll/tests: Added tests for FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE

[Joel Holdsworth]

From: Joel Holdsworth

Signed-off-by: Joel Holdsworth 
---
 dlls/ntdll/tests/file.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index b96b2e5b072..c2a095d2e52 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -2957,6 +2957,7 @@ static void test_file_disposition_information(void)
     NTSTATUS res;
     IO_STATUS_BLOCK io;
     FILE_DISPOSITION_INFORMATION fdi;
+    FILE_DISPOSITION_INFORMATION_EX fdie;
     FILE_STANDARD_INFORMATION fsi;
     BOOL fileDeleted;
     DWORD fdi2, size;
@@ -3089,6 +3090,22 @@ static void test_file_disposition_information(void)
     SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL );
     DeleteFileA( buffer );

+    /* set disposition on readonly file ignoring readonly attribute */
+    GetTempFileNameA( tmp_path, "dis", 0, buffer );
+    DeleteFileA( buffer );
+    handle = CreateFileA(buffer, GENERIC_WRITE | DELETE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_READONLY, 0);
+    ok( handle != INVALID_HANDLE_VALUE, "failed to create temp file\n" );
+    fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE;
+    res = pNtSetInformationFile( handle, &io, &fdie, sizeof fdie, FileDispositionInformationEx );
+    todo_wine
+    ok( res == STATUS_SUCCESS, "unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got %lx)\n", res );
+    CloseHandle( handle );
+    fileDeleted = GetFileAttributesA( buffer ) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND;
+    todo_wine
+    ok( fileDeleted, "File should have been deleted\n" );
+    SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL );
+    DeleteFileA( buffer );
+
     /* can set disposition on file and then reset it */
     GetTempFileNameA( tmp_path, "dis", 0, buffer );
     handle = CreateFileA(buffer, GENERIC_WRITE | DELETE, 0, NULL, CREATE_ALWAYS, 0, 0);

[PATCH 4/4] ntdll: Implemented FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE

[Joel Holdsworth]

From: Joel Holdsworth

Both the Msys2 and Cygwin runtimes make use of FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE in their
implementations of the unlink() system call. This enables these routines to delete a read-only file
without first modifying the attributes.

https://github.com/msys2/msys2-runtime/blob/msys2-3.4.3/winsup/cygwin/syscal...
https://www.cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/sysca...

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50771
Signed-off-by: Joel Holdsworth 
---
 dlls/ntdll/tests/file.c | 2 --
 dlls/ntdll/unix/file.c  | 2 --
 server/fd.c             | 3 ++-
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index c2a095d2e52..cbc48006f76 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -3097,11 +3097,9 @@ static void test_file_disposition_information(void)
     ok( handle != INVALID_HANDLE_VALUE, "failed to create temp file\n" );
     fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE;
     res = pNtSetInformationFile( handle, &io, &fdie, sizeof fdie, FileDispositionInformationEx );
-    todo_wine
     ok( res == STATUS_SUCCESS, "unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got %lx)\n", res );
     CloseHandle( handle );
     fileDeleted = GetFileAttributesA( buffer ) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND;
-    todo_wine
     ok( fileDeleted, "File should have been deleted\n" );
     SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL );
     DeleteFileA( buffer );
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index da6c157ba71..cb07388a82d 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -4773,8 +4773,6 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
                 FIXME( "FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK not supported\n" );
             if (info->Flags & FILE_DISPOSITION_ON_CLOSE)
                 FIXME( "FILE_DISPOSITION_ON_CLOSE not supported\n" );
-            if (info->Flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE)
-                FIXME( "FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE not supported\n" );

SERVER_START_REQ( set_fd_disp_info_ex )
             {
diff --git a/server/fd.c b/server/fd.c
index 46d32416463..363a25ec925 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -2499,7 +2499,8 @@ static void set_fd_disposition_ex( struct fd *fd, unsigned int flags )
         }
         if (S_ISREG( st.st_mode ))  /* can't unlink files we don't have permission to write */
         {
-            if (!(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)))
+            if (!(flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) &&
+                !(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)))
             {
                 set_error( STATUS_CANNOT_DELETE );
                 return;

[PATCH 4/4] ntdll: Implemented FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=133816

Your paranoid android.

=== w7u_2qxl (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w7u_adm (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w7u_el (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w8 (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w8adm (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w864 (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w1064v1507 (32 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w7pro64 (64 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w864 (64 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted

=== w1064v1507 (64 bit report) ===

ntdll:
file.c:3100: Test failed: unexpected FileDispositionInformation result (expected STATUS_SUCCESS, got c0000003)
file.c:3103: Test failed: File should have been deleted