[PATCH v2 0/5] MR871: ntdll: Fill the IOSB only inside the if (alerted) block in multiple functions.
-- v2: ntdll: Fill the IOSB in sock_transmit() only inside the "if (alerted)" block. ntdll: Fill the IOSB in sock_send() only inside the "if (alerted)" block. ntdll: Fill the IOSB in sock_recv() only inside the "if (alerted)" block. ntdll: The async handle passed to set_async_direct_result() cannot be NULL. ws2_32/tests: Add more tests for iosb contents while a recv is pending. https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/afd.c | 7 ++++--- dlls/ws2_32/tests/sock.c | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index 21e7a50aef3..3215ddaef62 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -1466,13 +1466,14 @@ static void test_recv(void) IOCTL_AFD_RECV, ¶ms, sizeof(params) - 1, NULL, 0); ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret); - memset(&io, 0, sizeof(io)); + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; memset(buffer, 0xcc, sizeof(buffer)); ret = NtDeviceIoControlFile((HANDLE)client, event, NULL, NULL, &io, IOCTL_AFD_RECV, ¶ms, sizeof(params), NULL, 0); ok(ret == STATUS_PENDING, "got %#x\n", ret); - ok(!io.Status, "got status %#lx\n", io.Status); - ok(!io.Information, "got information %#Ix\n", io.Information); + ok(io.Status == 0xdeadbeef, "got status %#lx\n", io.Status); + ok(io.Information == 0xdeadbeef, "got information %#Ix\n", io.Information); /* These structures need not remain valid. */ memset(¶ms, 0xcc, sizeof(params)); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 3e6faaffa4c..ad7b67d2a43 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -12159,11 +12159,15 @@ static void test_nonblocking_async_recv(void) memset(buffer, 0, sizeof(buffer)); WSASetLastError(0xdeadbeef); + overlapped.Internal = 0xdeadbeef; + overlapped.InternalHigh = 0xdeadbeef; ret = WSARecv(client, &wsabuf, 1, NULL, &flags, &overlapped, NULL); ok(ret == -1, "got %d\n", ret); ok(WSAGetLastError() == ERROR_IO_PENDING, "got error %u\n", WSAGetLastError()); ret = WaitForSingleObject((HANDLE)client, 0); ok(ret == WAIT_TIMEOUT, "expected timeout\n"); + ok(overlapped.Internal == STATUS_PENDING, "got status %#lx\n", (NTSTATUS)overlapped.Internal); + ok(overlapped.InternalHigh == 0xdeadbeef, "got size %Iu\n", overlapped.InternalHigh); ret = send(server, "data", 4, 0); ok(ret == 4, "got %d\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 9 +++++---- dlls/ntdll/unix/unix_private.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 1194ee514b5..821a1f08311 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2533,21 +2533,22 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG /* Notify direct completion of async and close the wait handle if it is no longer needed. * This function is a no-op (returns status as-is) if the supplied handle is NULL. */ -void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) +void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) { NTSTATUS ret; - if (!*optional_handle) return; + /* if we got STATUS_ALERTED, we must have a valid async handle */ + assert( *async_handle ); SERVER_START_REQ( set_async_direct_result ) { - req->handle = wine_server_obj_handle( *optional_handle ); + req->handle = wine_server_obj_handle( *async_handle ); req->status = status; req->information = information; req->mark_pending = mark_pending; ret = wine_server_call( req ); if (ret == STATUS_SUCCESS) - *optional_handle = wine_server_ptr_handle( reply->handle ); + *async_handle = wine_server_ptr_handle( reply->handle ); } SERVER_END_REQ; diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..73b9ed76de0 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -278,7 +278,7 @@ extern NTSTATUS get_device_info( int fd, struct _FILE_FS_DEVICE_INFORMATION *inf extern void init_files(void) DECLSPEC_HIDDEN; extern void init_cpu_info(void) DECLSPEC_HIDDEN; extern void add_completion( HANDLE handle, ULONG_PTR value, NTSTATUS status, ULONG info, BOOL async ) DECLSPEC_HIDDEN; -extern void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ); +extern void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) DECLSPEC_HIDDEN; extern void dbg_init(void) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This avoids an uninitialized variable warning with gcc 12.2. --- dlls/ntdll/unix/socket.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index f8ed9f6f854..8ff58f7566e 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -23,6 +23,7 @@ #endif #include "config.h" +#include <assert.h> #include <errno.h> #include <sys/types.h> #include <sys/socket.h> @@ -879,24 +880,25 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + alerted = status == STATUS_ALERTED; if (alerted) { status = try_recv( fd, async, &information ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; - } - - if (status != STATUS_PENDING) - { - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } + if (status != STATUS_PENDING) + release_fileio( &async->io ); + if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This matches sock_recv(). --- dlls/ntdll/unix/socket.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 8ff58f7566e..cbd9d4f56a6 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1116,6 +1116,9 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + if (!NT_ERROR(status) && is_icmp_over_dgram( fd )) sock_save_icmp_id( async ); @@ -1132,19 +1135,17 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi * and returns EWOULDBLOCK, but we have no way of doing that. */ if (status == STATUS_DEVICE_NOT_READY && async->sent_len) status = STATUS_SUCCESS; - } - if (status != STATUS_PENDING) - { information = async->sent_len; - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } - else information = 0; + + if (status != STATUS_PENDING) + release_fileio( &async->io ); if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This matches sock_recv(). --- dlls/ntdll/unix/socket.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index cbd9d4f56a6..c5e8d64f232 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1388,25 +1388,26 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + alerted = status == STATUS_ALERTED; if (alerted) { status = try_transmit( fd, file_fd, async ); if (status == STATUS_DEVICE_NOT_READY) status = STATUS_PENDING; - } - if (status != STATUS_PENDING) - { information = async->head_cursor + async->file_cursor + async->tail_cursor; - if (!NT_ERROR(status) || wait_handle) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } - else information = 0; + + if (status != STATUS_PENDING) + release_fileio( &async->io ); if (alerted) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
participants (2)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)