[PATCH v4 0/4] MR3073: Implement FileDispositionInformationEx and FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
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. -- v4: ntdll: Implemented FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE ntdll/tests: Added tests for FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE ntdll: Initial implementation of FileDispositionInformationEx include: Defined FILE_DISPOSITION_INFORMATION_EX and friends https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> 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 <joel(a)airwebreathe.org.uk> --- dlls/ntdll/unix/file.c | 27 ++++++++++++++++++++++++++- include/wine/server_protocol.h | 4 ++-- server/fd.c | 8 ++++---- server/protocol.def | 2 +- server/request.h | 2 +- server/trace.c | 2 +- 6 files changed, 35 insertions(+), 10 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index ff1618f31fd..3e930f1ac2f 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4754,7 +4754,32 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, SERVER_START_REQ( set_fd_disp_info ) { 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 ) + { + 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 d765697277b..1701a3e0420 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -5230,7 +5230,7 @@ struct set_fd_disp_info_request { struct request_header __header; obj_handle_t handle; - int unlink; + unsigned int flags; char __pad_20[4]; }; struct set_fd_disp_info_reply @@ -6418,7 +6418,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 777 +#define SERVER_PROTOCOL_VERSION 778 /* ### protocol_version end ### */ diff --git a/server/fd.c b/server/fd.c index eaebe044f37..9388d36dcfd 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( 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; } @@ -2955,7 +2955,7 @@ DECL_HANDLER(set_fd_disp_info) struct fd *fd = get_handle_fd_obj( current->process, req->handle, DELETE ); if (fd) { - set_fd_disposition( fd, req->unlink ); + set_fd_disposition( fd, req->flags ); release_object( fd ); } } diff --git a/server/protocol.def b/server/protocol.def index 54059b6b580..b690817ef1d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3641,7 +3641,7 @@ struct handle_info /* set fd disposition information */ @REQ(set_fd_disp_info) 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..0e54dc30b85 100644 --- a/server/request.h +++ b/server/request.h @@ -2207,7 +2207,7 @@ 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( FIELD_OFFSET(struct set_fd_disp_info_request, flags) == 16 ); C_ASSERT( sizeof(struct set_fd_disp_info_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 ); diff --git a/server/trace.c b/server/trace.c index 4dbc5de352f..4242e70a835 100644 --- a/server/trace.c +++ b/server/trace.c @@ -4360,7 +4360,7 @@ static void dump_set_fd_completion_mode_request( const struct set_fd_completion_ static void dump_set_fd_disp_info_request( const struct set_fd_disp_info_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 ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- dlls/ntdll/tests/file.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index b96b2e5b072..96d6ab0a88a 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,26 @@ static void test_file_disposition_information(void) SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL ); DeleteFileA( buffer ); + /* set disposition on readonly file ignoring readonly attribute */ + /* FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE is only supported in Windows 10 build 1809 and later */ + 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_INVALID_INFO_CLASS || res == STATUS_SUCCESS, + "unexpected FileDispositionInformation result (expected STATUS_SUCCES or SSTATUS_INVALID_INFO_CLASS, got %lx)\n", res ); + CloseHandle( handle ); + if ( res == STATUS_SUCCESS ) { + 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> 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 <joel(a)airwebreathe.org.uk> --- 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 96d6ab0a88a..34c00a94fee 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -3098,13 +3098,11 @@ 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_INVALID_INFO_CLASS || res == STATUS_SUCCESS, "unexpected FileDispositionInformation result (expected STATUS_SUCCES or SSTATUS_INVALID_INFO_CLASS, got %lx)\n", res ); CloseHandle( handle ); if ( res == STATUS_SUCCESS ) { 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 ); diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 3e930f1ac2f..8352cd6722b 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 ) { diff --git a/server/fd.c b/server/fd.c index 9388d36dcfd..942585b06ae 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2499,7 +2499,8 @@ static void set_fd_disposition( 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
On Mon Jun 19 21:17:10 2023 +0000, Zebediah Figura wrote:
I wouldn't rename the server call, it adds noise and isn't particularly meaningful anyway if we only have the one. Thanks for the feedback. I set it back to the original name.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3073#note_36186
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
This merge request was closed by Joel Holdsworth. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
I just notice there is a problem combining the two server calls into one, because `FILE_DISPOSITION_INFORMATION` has no effect on `FILE_FLAG_DELETE_ON_CLOSE`, wheras `FILE_DISPOSITION_INFORMATION_EX` always sets it or clears it. I will come back later with a version that has the two server calls separated. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3073#note_36246
participants (3)
-
Joel Holdsworth -
Joel Holdsworth (＠jhol)
-
Zebediah Figura (＠zfigura)